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

[Juergen Gross <jgross@suse.com>]

On 29/03/16 16:24, Konrad Rzeszutek Wilk wrote:
>>>> +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.

Which is weird in this case. :-)

>>
>>>
>>>> +    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?

Have a look in V4 of the patches. :-)


Juergen

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