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

[Wei Liu <liuw@liuw.name>]

On Thu, 2011-06-09 at 11:23 +0100, Ian Campbell wrote:
> On Thu, 2011-06-09 at 09:31 +0100, Wei Liu wrote:
> > On Thu, 2011-06-09 at 08:55 +0100, Ian Campbell wrote:
> > > On Thu, 2011-06-09 at 06:03 +0100, Wei Liu wrote:
> > > > The uninitialized domid may cause libxl__domain_make to fail.
> > > > 
> > > > In libxl__domain_make:
> > > > assert(!libxl_domid_valid_guest(*domid)).
> > > > 
> > > > Signed-off-by: Wei Liu <liuw@liuw.name>
> > > 
> > > That check seems pretty odd to me at first but the commit message of
> > > 22842:ccfa0527893e does a good job of explaining why so: 
> > > 
> > > Acked-by: Ian Campbell <ian.campbell@citrix.com>
> > > 
> > > although it's not clear why libxl__domain_make doesn't just set an
> > > invalid value as it's first act and save the callers the effort, the net
> > > result would still be the correct semantics for libxl_domid_valid_guest
> > > when the function exits.
> > > 
> > 
> > I think the commit message of 22842:ccfa0527893e says pretty clear that
> > it is caller's responsibility to initialize domid to a invalid value.
> 
> Only because that's how 22842 causes libxl__make_domain to be
> implemented, we are free to change it.
> 

I'm not against changing it. But I won't do this until I completely
understand what it does and why it does.

My patch is based on similar use case in
libxc_create.c:do_domain_create, which initializes domid to 0 before
calling libxl__domain_make. That's safer (not likely to misunderstand
its usage and introduce new bug) and solve my problem.

> > However, libxl__make_domain sets domid=-1 a few lines after the check.
> > This confuses me.
> 
> Yeah, me too, that line could just be hoisted up to the first thing the
> function does with no loss of safety AFAICT.
> 
> Ian.
>