[PATCH 2/4] node_device_udev: Make udevGetStringProperty() return void

Peter Krempa pkrempa at redhat.com
Wed Jun 2 07:48:46 UTC 2021


On Wed, Jun 02, 2021 at 09:37:41 +0200, Michal Privoznik wrote:
> This function can't fail really as it's returning 0 no matter
> what. This is probably a residue from old days when we cared
> about propagating OOM errors. Now we just abort. Make its return
> type void then.
> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/node_device/node_device_udev.c | 46 ++++++++++--------------------
>  1 file changed, 15 insertions(+), 31 deletions(-)
> 
> diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
> index d5f3beb389..f48789d98f 100644
> --- a/src/node_device/node_device_udev.c
> +++ b/src/node_device/node_device_udev.c

[...]

> @@ -517,10 +515,9 @@ udevProcessUSBDevice(struct udev_device *device,
>      if (udevGetUintProperty(device, "ID_VENDOR_ID", &usb_dev->vendor, 16) < 0)
>          return -1;
>  
> -    if (udevGetStringProperty(device,
> -                              "ID_VENDOR_FROM_DATABASE",
> -                              &usb_dev->vendor_name) < 0)
> -        return -1;
> +    udevGetStringProperty(device,
> +                          "ID_VENDOR_FROM_DATABASE",
> +                          &usb_dev->vendor_name);
>  
>      if (!usb_dev->vendor_name &&

Here you've kept the NULL check that should be now impossible to trigger
...

>          udevGetStringSysfsAttr(device, "manufacturer",
> @@ -530,10 +527,9 @@ udevProcessUSBDevice(struct udev_device *device,
>      if (udevGetUintProperty(device, "ID_MODEL_ID", &usb_dev->product, 16) < 0)
>          return -1;
>  
> -    if (udevGetStringProperty(device,
> -                              "ID_MODEL_FROM_DATABASE",
> -                              &usb_dev->product_name) < 0)
> -        return -1;
> +    udevGetStringProperty(device,
> +                          "ID_MODEL_FROM_DATABASE",
> +                          &usb_dev->product_name);
>  
>      if (!usb_dev->product_name &&

(same here)

>          udevGetStringSysfsAttr(device, "product",

[...]

> @@ -965,8 +954,7 @@ udevProcessStorage(struct udev_device *device,
>       * expected, so I don't see a problem with not having a property
>       * for it. */
>  
> -    if (udevGetStringProperty(device, "ID_TYPE", &storage->drive_type) < 0)
> -        goto cleanup;
> +    udevGetStringProperty(device, "ID_TYPE", &storage->drive_type);
>  
>      if (!storage->drive_type ||

(and here, but with different logic)

>          STREQ(def->caps->data.storage.drive_type, "generic")) {
> @@ -1010,9 +998,7 @@ static int
>  udevProcessSCSIGeneric(struct udev_device *dev,
>                         virNodeDeviceDef *def)
>  {
> -    if (udevGetStringProperty(dev, "DEVNAME", &def->caps->data.sg.path) < 0 ||
> -        !def->caps->data.sg.path)

... but here you've removed it.

> -        return -1;
> +    udevGetStringProperty(dev, "DEVNAME", &def->caps->data.sg.path);
>  
>      udevGenerateDeviceName(dev, def, NULL);
>  




More information about the libvir-list mailing list