Re: [Xen-devel] [PATCH v2 2/3] libxl: add new pvusb backend "qusb" provided by qemu

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

> >> +static int usbback_is_loaded(libxl__gc *gc)
> >> +{
> >> +    int r;
> >> +    struct stat st;
> >> +
> >> +    r = lstat(SYSFS_USBBACK_DRIVER, &st);
> >> +
> >> +    if (r == 0)
> >> +        return 1;
> >> +    if (r < 0 && errno == ENOENT)
> >> +        return 0;
> > 
> > I believe the CODING STYLE in libxl asks for you to use { } for these
> > ones.
> 
> No, it doesn't:
> 
> Quote from tools/libxl/CODING_STYLE:
> 
> 5. Block structure
> 
> Every indented statement is braced apart from blocks that contain just
> one statement.

You are right. I am so used to 'if (..) else'!
> 
> > 
> >> +    LOGE(ERROR, "Accessing %s", SYSFS_USBBACK_DRIVER);
> > 
> > Why is this an error?
> 
> What else? We can't determine whether the driver is loaded or not.
> ENOENT is tested above, so it must be something weird.

Or it could be EPERM.
> 
> > 
> >> +    return -1;

Which results in the code assuming (the caller of this function)
it is an kernel driver. Is that OK? Or should we bail out completly?

> >> +}
> >> +
> > 
> > Otherwise looks OK to me.
> 
> Thanks,
> 
> Juergen
> 

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