On Thu, Oct 22, 2015 at 05:48:58AM -0400, Frediano Ziglio wrote: > > > > I am not sure if I understand your point here. Frediano. > > > > For a cleaner code, red_dispatcher_new() must just create a dispatcher > > given the QXLInstance object, but I would prefer to set > > qxl->st->dispatcher out of this function. > > I mean, having something like: qxl->st->dispatcher = red_dispatcher_new(qxl); > > > > Yes, new patch add a line like this. Actually there is this line and also > qxl->st->dispatcher is set inside red_dispatcher_new. > I think that if the function is called red_dispatcher_new is a caller responsibility > to check that qxl->st->dispatcher is NULL before calling it to avoid the leak and > also to set qxl->st->dispatcher with the value returned by red_dispatcher_new. > > So in the end I would: > - remove qxl->st->dispatcher == NULL check inside red_dispatcher_new; For what it's worth, if your API contract is "it's the caller responsibility to check that qxl->st->dispatcher is NULL before calling red_dispatcher_new", then g_return_val_if_fail(qxl->st->dispatcher == NULL, NULL); means exactly that (ie it's a programming error to call this method if dispatcher is not NULL). The added benefit is that it's explicitly written, and if you in one year from now you rework that code/write new code forgetting this contract, then you'll get reminded with a loud warning. Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel