Re: [Xen-devel] [PATCH 1/4] a fix in libxl_device_usbdev_list

["Chun Yan Liu" <cyliu@xxxxxxxx>]

>>> On 4/8/2016 at 12:45 AM, in message
<22278.36492.245114.295391@xxxxxxxxxxxxxxxxxxxxxxxx>, Ian Jackson
<Ian.Jackson@xxxxxxxxxxxxx> wrote: 
> Chunyan Liu writes ("[PATCH 1/4] a fix in libxl_device_usbdev_list"): 
> > In testing with libvirt pvusb functionality, found a rc check 
> > error in libxl_device_usbdev_list. Correct it. 
>  
> Thanks.  But now that I look at this code I'm not sure your fix is 
> complete. 
>  
> > diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c 
> > index 5f92628..04e41b4 100644 
> > --- a/tools/libxl/libxl_pvusb.c 
> > +++ b/tools/libxl/libxl_pvusb.c 
> > @@ -701,13 +701,13 @@ libxl_device_usbdev_list(libxl_ctx *ctx, uint32_t  
> domid, int *num) 
> >      usbctrls = libxl__xs_directory(gc, XBT_NULL, path, &nc); 
> >   
> >      for (i = 0; i < nc; i++) { 
> > -        int r, nd = 0; 
> > +        int rc, nd = 0; 
> >          libxl_device_usbdev *tmp = NULL; 
> >   
> > -        r = libxl__device_usbdev_list_for_usbctrl(gc, domid, 
> > +        rc = libxl__device_usbdev_list_for_usbctrl(gc, domid, 
> >                                                    atoi(usbctrls[i]), 
> >                                                    &tmp, &nd); 
> > -        if (!r || !nd) continue; 
> > +        if (rc || !nd) continue; 
>  
> If libxl__device_usbdev_list_for_usbctrl fails, shouldn't 
> libxl_device_usbdev_list fail too ? 

Following the similar definitions of other device types, the return value of
this function is "libxl_device_usbdev *", to treat the above case as failure,
it cannot be reflected through return value, we can only set return value
to NULL, but that will be confusing with a real no-device case.

- Chunyan
>  
> Ian. 
>  
>  



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