1. Patchew
  2. Libvirt
  3. View series

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

Alexander Kuznetsov posted 1 patch 1 year 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 1 year 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 11 months, 1 week 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 11 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 11 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 11 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 1 year ago 
Kindly reminding about this small cosmetic fix

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.