[libvirt PATCH 06/11] udevGetIntSysfsAttr: Return -1 for missing attributes
Michal Privoznik
mprivozn at redhat.com
Thu Jan 28 12:18:07 UTC 2021
On 1/28/21 11:44 AM, Peter Krempa wrote:
> On Thu, Jan 28, 2021 at 11:24:36 +0100, Tim Wiederhake wrote:
>> If "udevGetDeviceSysfsAttr()" returns NULL, "udevGetIntSysfsAttr"
>> would return "0", indicating success, without writing to "value".
>>
>> This was found by clang-tidy's
>> "clang-analyzer-core.UndefinedBinaryOperatorResult" check in
>> function "udevProcessCCW", flagging a read on the potentially
>> uninitialized variable "online".
>>
>> Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
>> ---
>> src/node_device/node_device_udev.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>> index 55a2731681..d5a12bab0e 100644
>> --- a/src/node_device/node_device_udev.c
>> +++ b/src/node_device/node_device_udev.c
>> @@ -254,7 +254,10 @@ udevGetIntSysfsAttr(struct udev_device *udev_device,
>>
>> str = udevGetDeviceSysfsAttr(udev_device, attr_name);
>>
>> - if (str && virStrToLong_i(str, NULL, base, value) < 0) {
>> + if (!str)
>> + return -1;
>
> In this case an error wouldn't be reported any more.
I think it's quite the opposite actually. Previously, if str == NULL
then a zero was returned (without any error) from this function. Now you
get -1.
I think we want to keep return 0 in case of !str. Callers use the
following pattern:
var = -1; /* default */
udevGetIntSysfsAttr(device, "attribute", &var, 10);
If "attribute" exists, @var is updated; if it doesn't it's left
untouched with the default value (-1 in this case).
Michal
More information about the libvir-list
mailing list