Re: [Xen-devel] Possible bug in tools/libxl/libxl.c -- Variable passed by reference not set in one possible case

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Oct 10, 2014 at 10:17:15AM +0100, Ian Campbell wrote:
> On Fri, 2014-10-10 at 10:06 +0100, Wei Liu wrote:
> > Please send plain text email in the future. Some (if not all) developers
> > only have very shabby text editor to read / reply to your email. ;-)
> > 
> > On Thu, Oct 09, 2014 at 08:28:53PM -0500, ayush ruia wrote:
> > > What I am trying to say is that shouldn't the code block highlighted in
> > > yellow should come before the block marked in green. Then it would update
> > > the value of *target_memkb and *max_memkb in all possible situations.
> > > 
> > 
> > I don't think so. This function means "if there's no such values in
> > xenstore then retrieve them from hypervisor and fill them in xenstore,
> > optionally return those values to the caller".
> > 
> > So if those values already are present in xenstore this function doesn't
> > need to do anything.
> 
> If I call this function and receive a return code of zero how can I tell
> if the target_memkb pointer I passed has been initialised or not?
> 
> If all of target, staticmax and freememslack are already set the
> function returns 0 without updating those pointers, so I don't think we
> can tell.
> 

The way the callers use it prevents the issue you described from
happening -- they only call this function when they can't read those
values from xenstore -- if those values are already in xenstore this
function won't get called.

> It does seem like the 
>  if (target && staticmax && freememslack) {
>         rc = 0;
>         goto out;
>     }
> belongs after the code which writes back the two variables.
> 

I agree this is not nice and there's room for refactoring.

Wei.

> Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel