1. Patchew
  2. Libvirt
  3. View series

[PATCH] util: netdevvlan: Change return type of virNetDevVlanCopy to void

Alexander Kuznetsov posted 1 patch 7 months, 2 weeks ago
Failed in applying to current master (apply log)
There is a newer version of this series
src/conf/domain_conf.c      | 9 +++------
src/network/bridge_driver.c | 4 +++-
src/util/virnetdevvlan.c    | 6 +++---
src/util/virnetdevvlan.h    | 2 +-
4 files changed, 10 insertions(+), 11 deletions(-)
[PATCH] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Alexander Kuznetsov 7 months, 2 weeks ago 
This function return value is invariant since 1022e0ee, so change
its type and remove all dependent checks.

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

Reported-by: Alexander Rudyuk <a.rudyuk@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/conf/domain_conf.c      | 9 +++------
 src/network/bridge_driver.c | 4 +++-
 src/util/virnetdevvlan.c    | 6 +++---
 src/util/virnetdevvlan.h    | 2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 3f88a77a8f..6f49ee507a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30533,8 +30533,7 @@ virDomainNetDefToNetworkPort(virDomainDef *dom,
     if (virNetDevBandwidthCopy(&port->bandwidth, iface->bandwidth) < 0)
         return NULL;
 
-    if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0)
-        return NULL;
+    virNetDevVlanCopy(&port->vlan, &iface->vlan);
 
     port->isolatedPort = iface->isolatedPort;
     port->trustGuestRxFilters = iface->trustGuestRxFilters;
@@ -30611,8 +30610,7 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDef *iface,
     if (virNetDevBandwidthCopy(&actual->bandwidth, port->bandwidth) < 0)
         goto error;
 
-    if (virNetDevVlanCopy(&actual->vlan, &port->vlan) < 0)
-        goto error;
+    virNetDevVlanCopy(&actual->vlan, &port->vlan);
 
     actual->isolatedPort = port->isolatedPort;
     actual->class_id = port->class_id;
@@ -30729,8 +30727,7 @@ virDomainNetDefActualToNetworkPort(virDomainDef *dom,
     if (virNetDevBandwidthCopy(&port->bandwidth, actual->bandwidth) < 0)
         return NULL;
 
-    if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0)
-        return NULL;
+    virNetDevVlanCopy(&port->vlan, &actual->vlan);
 
     port->isolatedPort = actual->isolatedPort;
     port->class_id = actual->class_id;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8f47ef2574..668870a9ee 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3973,8 +3973,10 @@ networkAllocatePort(virNetworkObj *obj,
         else if (netdef->vlan.nTags > 0)
             vlan = &netdef->vlan;
 
-        if (vlan && virNetDevVlanCopy(&port->vlan, vlan) < 0)
+        if (vlan) {
+            virNetDevVlanCopy(&port->vlan, vlan);
             return -1;
+        }
     }
 
     if (!port->trustGuestRxFilters) {
diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c
index 67daa5d3b4..e8f572efd2 100644
--- a/src/util/virnetdevvlan.c
+++ b/src/util/virnetdevvlan.c
@@ -76,11 +76,11 @@ virNetDevVlanEqual(const virNetDevVlan *a, const virNetDevVlan *b)
  *                     If src is NULL, dst will have nTags set to 0.
  *                     dst is assumed to be empty on entry.
  */
-int
+void
 virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
 {
     if (!src || src->nTags == 0)
-        return 0;
+        return;
 
     dst->tag = g_new0(unsigned int, src->nTags);
     dst->trunk = src->trunk;
@@ -88,5 +88,5 @@ virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
     dst->nativeMode = src->nativeMode;
     dst->nativeTag = src->nativeTag;
     memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag));
-    return 0;
+    return;
 }
diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
index 228d270869..fd2f8023f5 100644
--- a/src/util/virnetdevvlan.h
+++ b/src/util/virnetdevvlan.h
@@ -42,6 +42,6 @@ struct _virNetDevVlan {
 void virNetDevVlanClear(virNetDevVlan *vlan);
 void virNetDevVlanFree(virNetDevVlan *vlan);
 int virNetDevVlanEqual(const virNetDevVlan *a, const virNetDevVlan *b);
-int virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src);
+void virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevVlan, virNetDevVlanFree);
-- 
2.42.4
Re: [PATCH] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Martin Kletzander 6 months ago 
On Wed, Jan 22, 2025 at 05:37:31PM +0300, Alexander Kuznetsov wrote:
>This function return value is invariant since 1022e0ee, so change
>its type and remove all dependent checks.
>
>Found by Linux Verification Center (linuxtesting.org) with Svace.
>
>Reported-by: Alexander Rudyuk <a.rudyuk@fobos-nt.ru>
>Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
>---
> src/conf/domain_conf.c      | 9 +++------
> src/network/bridge_driver.c | 4 +++-
> src/util/virnetdevvlan.c    | 6 +++---
> src/util/virnetdevvlan.h    | 2 +-
> 4 files changed, 10 insertions(+), 11 deletions(-)
>
>diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>index 8f47ef2574..668870a9ee 100644
>--- a/src/network/bridge_driver.c
>+++ b/src/network/bridge_driver.c
>@@ -3973,8 +3973,10 @@ networkAllocatePort(virNetworkObj *obj,
>         else if (netdef->vlan.nTags > 0)
>             vlan = &netdef->vlan;
>
>-        if (vlan && virNetDevVlanCopy(&port->vlan, vlan) < 0)
>+        if (vlan) {
>+            virNetDevVlanCopy(&port->vlan, vlan);
>             return -1;
>+        }

This is *definitely* not semantically the same as before, beware of such
changes!

>diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c
>index 67daa5d3b4..e8f572efd2 100644
>--- a/src/util/virnetdevvlan.c
>+++ b/src/util/virnetdevvlan.c
>@@ -76,11 +76,11 @@ virNetDevVlanEqual(const virNetDevVlan *a, const virNetDevVlan *b)
>  *                     If src is NULL, dst will have nTags set to 0.
>  *                     dst is assumed to be empty on entry.
>  */
>-int
>+void
> virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
> {
>     if (!src || src->nTags == 0)
>-        return 0;
>+        return;
>

This means you don't even need to check if the source vlan is not NULL
before calling this function.

>     dst->tag = g_new0(unsigned int, src->nTags);
>     dst->trunk = src->trunk;
>@@ -88,5 +88,5 @@ virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
>     dst->nativeMode = src->nativeMode;
>     dst->nativeTag = src->nativeTag;
>     memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag));
>-    return 0;
>+    return;

Pointless return at the end of a function

> }
[PATCH v2 0/1] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Alexander Kuznetsov 6 months ago 
Thanks for the review!

v2:
- remove check for source vlan being not NULL and fix wrongly changed statement
- remove pointless return at the end of the function

Alexander Kuznetsov (1):
  util: netdevvlan: Change return type of virNetDevVlanCopy to void

 src/conf/domain_conf.c      | 9 +++------
 src/network/bridge_driver.c | 3 +--
 src/util/virnetdevvlan.c    | 5 ++---
 src/util/virnetdevvlan.h    | 2 +-
 4 files changed, 7 insertions(+), 12 deletions(-)

-- 
2.42.4
[PATCH v2 1/1] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Alexander Kuznetsov 6 months ago 
This function return value is invariant since 1022e0ee, so change
its type and remove all dependent checks.

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

Reported-by: Alexander Rudyuk <a.rudyuk@fobos-nt.ru>
Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
---
 src/conf/domain_conf.c      | 9 +++------
 src/network/bridge_driver.c | 3 +--
 src/util/virnetdevvlan.c    | 5 ++---
 src/util/virnetdevvlan.h    | 2 +-
 4 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f42b7075ad..032e2f9a6d 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -30501,8 +30501,7 @@ virDomainNetDefToNetworkPort(virDomainDef *dom,
     if (virNetDevBandwidthCopy(&port->bandwidth, iface->bandwidth) < 0)
         return NULL;
 
-    if (virNetDevVlanCopy(&port->vlan, &iface->vlan) < 0)
-        return NULL;
+    virNetDevVlanCopy(&port->vlan, &iface->vlan);
 
     port->isolatedPort = iface->isolatedPort;
     port->trustGuestRxFilters = iface->trustGuestRxFilters;
@@ -30579,8 +30578,7 @@ virDomainNetDefActualFromNetworkPort(virDomainNetDef *iface,
     if (virNetDevBandwidthCopy(&actual->bandwidth, port->bandwidth) < 0)
         goto error;
 
-    if (virNetDevVlanCopy(&actual->vlan, &port->vlan) < 0)
-        goto error;
+    virNetDevVlanCopy(&actual->vlan, &port->vlan);
 
     actual->isolatedPort = port->isolatedPort;
     actual->class_id = port->class_id;
@@ -30697,8 +30695,7 @@ virDomainNetDefActualToNetworkPort(virDomainDef *dom,
     if (virNetDevBandwidthCopy(&port->bandwidth, actual->bandwidth) < 0)
         return NULL;
 
-    if (virNetDevVlanCopy(&port->vlan, &actual->vlan) < 0)
-        return NULL;
+    virNetDevVlanCopy(&port->vlan, &actual->vlan);
 
     port->isolatedPort = actual->isolatedPort;
     port->class_id = actual->class_id;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 8f47ef2574..80d2c3a1d5 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -3973,8 +3973,7 @@ networkAllocatePort(virNetworkObj *obj,
         else if (netdef->vlan.nTags > 0)
             vlan = &netdef->vlan;
 
-        if (vlan && virNetDevVlanCopy(&port->vlan, vlan) < 0)
-            return -1;
+        virNetDevVlanCopy(&port->vlan, vlan);
     }
 
     if (!port->trustGuestRxFilters) {
diff --git a/src/util/virnetdevvlan.c b/src/util/virnetdevvlan.c
index 67daa5d3b4..b0e05d8ffe 100644
--- a/src/util/virnetdevvlan.c
+++ b/src/util/virnetdevvlan.c
@@ -76,11 +76,11 @@ virNetDevVlanEqual(const virNetDevVlan *a, const virNetDevVlan *b)
  *                     If src is NULL, dst will have nTags set to 0.
  *                     dst is assumed to be empty on entry.
  */
-int
+void
 virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
 {
     if (!src || src->nTags == 0)
-        return 0;
+        return;
 
     dst->tag = g_new0(unsigned int, src->nTags);
     dst->trunk = src->trunk;
@@ -88,5 +88,4 @@ virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src)
     dst->nativeMode = src->nativeMode;
     dst->nativeTag = src->nativeTag;
     memcpy(dst->tag, src->tag, src->nTags * sizeof(*src->tag));
-    return 0;
 }
diff --git a/src/util/virnetdevvlan.h b/src/util/virnetdevvlan.h
index 228d270869..fd2f8023f5 100644
--- a/src/util/virnetdevvlan.h
+++ b/src/util/virnetdevvlan.h
@@ -42,6 +42,6 @@ struct _virNetDevVlan {
 void virNetDevVlanClear(virNetDevVlan *vlan);
 void virNetDevVlanFree(virNetDevVlan *vlan);
 int virNetDevVlanEqual(const virNetDevVlan *a, const virNetDevVlan *b);
-int virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src);
+void virNetDevVlanCopy(virNetDevVlan *dst, const virNetDevVlan *src);
 
 G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetDevVlan, virNetDevVlanFree);
-- 
2.42.4
Re: [PATCH v2 1/1] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Martin Kletzander 6 months ago 
On Fri, Mar 07, 2025 at 12:08:03PM +0300, Alexander Kuznetsov wrote:
>This function return value is invariant since 1022e0ee, so change
>its type and remove all dependent checks.
>
>Found by Linux Verification Center (linuxtesting.org) with Svace.
>
>Reported-by: Alexander Rudyuk <a.rudyuk@fobos-nt.ru>
>Signed-off-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>

Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
Re: [PATCH] util: netdevvlan: Change return type of virNetDevVlanCopy to void
Posted by Alexander Kuznetsov 7 months ago 
Kindly reminding about this small cosmetic fix

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.