[PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void

Alexander Kuznetsov posted 4 patches 4 weeks ago
There is a newer version of this series
[PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void
Posted by Alexander Kuznetsov 4 weeks ago
This function return value is invariant since 18f3771, so change
its type and remove all dependent checks.

Found by Linux Verification Center (linuxtesting.org) with Svace.

Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/hypervisor/virhostdev.c | 3 +--
 src/util/virscsivhost.c     | 6 ++----
 src/util/virscsivhost.h     | 2 +-
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/hypervisor/virhostdev.c b/src/hypervisor/virhostdev.c
index 30a51507da..037842c8a1 100644
--- a/src/hypervisor/virhostdev.c
+++ b/src/hypervisor/virhostdev.c
@@ -1666,8 +1666,7 @@ virHostdevPrepareSCSIVHostDevices(virHostdevManager *mgr,
         if (!(host = virSCSIVHostDeviceNew(hostsrc->wwpn)))
             return -1;
 
-        if (virSCSIVHostDeviceSetUsedBy(host, drv_name, dom_name) < 0)
-            return -1;
+        virSCSIVHostDeviceSetUsedBy(host, drv_name, dom_name);
 
         if (virSCSIVHostDeviceListAdd(list, host) < 0)
             return -1;
diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
index 15024d7106..05b89e58d6 100644
--- a/src/util/virscsivhost.c
+++ b/src/util/virscsivhost.c
@@ -193,7 +193,7 @@ virSCSIVHostDeviceListNew(void)
 }
 
 
-int
+void
 virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
                             const char *drvname,
                             const char *domname)
@@ -202,8 +202,6 @@ virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
     VIR_FREE(dev->used_by_domname);
     dev->used_by_drvname = g_strdup(drvname);
     dev->used_by_domname = g_strdup(domname);
-
-    return 0;
 }
 
 
@@ -214,7 +212,7 @@ virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevice *dev,
 {
     *drv_name = dev->used_by_drvname;
     *dom_name = dev->used_by_domname;
- }
+}
 
 
 int
diff --git a/src/util/virscsivhost.h b/src/util/virscsivhost.h
index a7299382db..caaac26328 100644
--- a/src/util/virscsivhost.h
+++ b/src/util/virscsivhost.h
@@ -50,7 +50,7 @@ void virSCSIVHostDeviceListDel(virSCSIVHostDeviceList *list,
                                virSCSIVHostDevice *dev);
 virSCSIVHostDeviceList *virSCSIVHostDeviceListNew(void);
 virSCSIVHostDevice *virSCSIVHostDeviceNew(const char *name);
-int virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
+void virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
                                 const char *drvname,
                                 const char *domname);
 void virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevice *dev,
-- 
2.42.2
Re: [PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void
Posted by Jiri Denemark 1 week, 6 days ago
On Thu, Nov 28, 2024 at 16:28:09 +0300, Alexander Kuznetsov wrote:
> This function return value is invariant since 18f3771, so change
> its type and remove all dependent checks.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/hypervisor/virhostdev.c | 3 +--
>  src/util/virscsivhost.c     | 6 ++----
>  src/util/virscsivhost.h     | 2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)
...
> diff --git a/src/util/virscsivhost.h b/src/util/virscsivhost.h
> index a7299382db..caaac26328 100644
> --- a/src/util/virscsivhost.h
> +++ b/src/util/virscsivhost.h
> @@ -50,7 +50,7 @@ void virSCSIVHostDeviceListDel(virSCSIVHostDeviceList *list,
>                                 virSCSIVHostDevice *dev);
>  virSCSIVHostDeviceList *virSCSIVHostDeviceListNew(void);
>  virSCSIVHostDevice *virSCSIVHostDeviceNew(const char *name);
> -int virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
> +void virSCSIVHostDeviceSetUsedBy(virSCSIVHostDevice *dev,
>                                  const char *drvname,
>                                  const char *domname);

Indentation error.

Jirka
Re: [PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void
Posted by Jiri Denemark 1 week, 6 days ago
On Thu, Nov 28, 2024 at 16:28:09 +0300, Alexander Kuznetsov wrote:
> This function return value is invariant since 18f3771, so change
> its type and remove all dependent checks.
> 
> Found by Linux Verification Center (linuxtesting.org) with Svace.
> 
> Reported-by: Pavel Nekrasov <p.nekrasov@fobos-nt.ru>
> Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
> ---
>  src/hypervisor/virhostdev.c | 3 +--
>  src/util/virscsivhost.c     | 6 ++----
>  src/util/virscsivhost.h     | 2 +-
>  3 files changed, 4 insertions(+), 7 deletions(-)
> 
...
> diff --git a/src/util/virscsivhost.c b/src/util/virscsivhost.c
> index 15024d7106..05b89e58d6 100644
> --- a/src/util/virscsivhost.c
> +++ b/src/util/virscsivhost.c
...
> @@ -214,7 +212,7 @@ virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevice *dev,
>  {
>      *drv_name = dev->used_by_drvname;
>      *dom_name = dev->used_by_domname;
> - }
> +}

This is completely unrelated to what this patch is doing.

Jirka
Re: [PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void
Posted by Alexander Kuznetsov 1 week, 6 days ago
13.12.2024 16:32, Jiri Denemark wrote:
>> @@ -214,7 +212,7 @@ virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevice *dev,
>>   {
>>       *drv_name = dev->used_by_drvname;
>>       *dom_name = dev->used_by_domname;
>> - }
>> +}
> This is completely unrelated to what this patch is doing.
Yes, I just wanted to fix this small cosmetic thing, but I thought it was too
small for a separate patch. Should it be taken out separately,
or left until possible changes in the affected function?

-- 
Best regards,
Alexander Kuznetsov
Re: [PATCH v2 3/4] util: Change return type of virSCSIVHostDeviceSetUsedBy to void
Posted by Jiri Denemark 1 week, 6 days ago
On Fri, Dec 13, 2024 at 16:41:25 +0300, Alexander Kuznetsov wrote:
> 13.12.2024 16:32, Jiri Denemark wrote:
> >> @@ -214,7 +212,7 @@ virSCSIVHostDeviceGetUsedBy(virSCSIVHostDevice *dev,
> >>   {
> >>       *drv_name = dev->used_by_drvname;
> >>       *dom_name = dev->used_by_domname;
> >> - }
> >> +}
> > This is completely unrelated to what this patch is doing.
> Yes, I just wanted to fix this small cosmetic thing, but I thought it was too
> small for a separate patch. Should it be taken out separately,
> or left until possible changes in the affected function?

I think I would just leave it for future changes in this function.

Jirka