Re: [Xen-devel] Re: [PATCH] libxl: initialize domid to 0 in libxl__create_stubdom

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 21 Jun 2011, Ian Jackson wrote:
> Stefano Stabellini writes ("Re: [Xen-devel] Re: [PATCH] libxl: initialize 
> domid to 0 in libxl__create_stubdom"):
> > On Fri, 17 Jun 2011, Ian Jackson wrote:
> > >    char *something = NULL;
> > >    uint32_t domid = -1;
> > > 
> > >    ...
> > >    something = allocate();
> > >    if (!something) goto error_exit;
> > >    ...
> > >    ret = libxl__domain_make(&domid);
> > >    if (ret) goto error_exit;
> > >    ...
> > > 
> > >    return successfully somehow;
> > > 
> > >   error_exit:
> > >    free(something);
> > >    if (libxl_domid_valid_guest(domid))
> > >        libxl_domain_destroy(domid);
> ...
> > If we decide to make domid an output parameter only, then
> > 
> > uint32_t domid;
> > 
> > isn't a bug anymore.
> 
> Look at the code above.  Without the initialisation, if allocate()
> returns NULL, it calls:
>   libxl_domid_valid_guest(UNDEFINED)
> If it's an output parameter only then neither valgrind nor the
> compiler will ever spot this bug unless allocate() actually fails.
> 
> The arrangement with the caller initialising and the check in
> libxl__domain_make is there to support this error-handling paradim
> (which is a good paradigm because it means you don't have to carefully
> match up allocations with frees on every error exit path), and which
> tries to give us a chance of spotting missing initialisation bugs.

I understand what you mean, but in that case I would rather have the
check right before allocate:

assert(!libxl_domid_valid_guest(*domid));
something = allocate();

in the outer function.
Because libxl__domain_make doesn't have any business in checking for the
validity of an output parameter, it is a layering violation.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel