[PATCH v2 1/4] conf: virNetDevVPortProfileParse refactor

Kirill Shchetiniuk via Devel posted 4 patches 1 month, 2 weeks ago
[PATCH v2 1/4] conf: virNetDevVPortProfileParse refactor
Posted by Kirill Shchetiniuk via Devel 1 month, 2 weeks ago
From: Kirill Shchetiniuk <kshcheti@redhat.com>

Refactored the virNetDevVPortProfileParse function to use the appropriate
virXMLProp* functions to parse input configuration XML.

Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
---
 src/conf/netdev_vport_profile_conf.c | 112 +++++++++++----------------
 1 file changed, 44 insertions(+), 68 deletions(-)

diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c
index 032a3147d7..92819c2f34 100644
--- a/src/conf/netdev_vport_profile_conf.c
+++ b/src/conf/netdev_vport_profile_conf.c
@@ -29,12 +29,6 @@ virNetDevVPortProfile *
 virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags)
 {
     g_autofree char *virtPortType = NULL;
-    g_autofree char *virtPortManagerID = NULL;
-    g_autofree char *virtPortTypeID = NULL;
-    g_autofree char *virtPortTypeIDVersion = NULL;
-    g_autofree char *virtPortInstanceID = NULL;
-    g_autofree char *virtPortProfileID = NULL;
-    g_autofree char *virtPortInterfaceID = NULL;
     g_autofree virNetDevVPortProfile *virtPort = NULL;
     xmlNodePtr parameters;
 
@@ -55,88 +49,70 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags)
     }
 
     if ((parameters = virXMLNodeGetSubelement(node, "parameters"))) {
-        virtPortManagerID = virXMLPropString(parameters, "managerid");
-        virtPortTypeID = virXMLPropString(parameters, "typeid");
-        virtPortTypeIDVersion = virXMLPropString(parameters, "typeidversion");
-        virtPortInstanceID = virXMLPropString(parameters, "instanceid");
-        virtPortProfileID = virXMLPropString(parameters, "profileid");
-        virtPortInterfaceID = virXMLPropString(parameters, "interfaceid");
-    }
-
-    if (virtPortManagerID) {
+        int rc;
         unsigned int val;
+        g_autofree char *virtPortProfileID = virXMLPropString(parameters, "profileid");
 
-        if (virStrToLong_ui(virtPortManagerID, NULL, 0, &val)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("cannot parse value of managerid parameter"));
+        if ((rc = virXMLPropUInt(parameters, "managerid", 10, VIR_XML_PROP_NONE, &val)) < 0)
             return NULL;
+
+        if (rc > 0) {
+            if (val > 0xff) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("value of managerid out of range"));
+                return NULL;
+            }
+
+            virtPort->managerID = (uint8_t)val;
+            virtPort->managerID_specified = true;
         }
-        if (val > 0xff) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("value of managerid out of range"));
+
+        if ((rc = virXMLPropUInt(parameters, "typeid", 10, VIR_XML_PROP_NONE, &val)) < 0)
             return NULL;
-        }
-        virtPort->managerID = (uint8_t)val;
-        virtPort->managerID_specified = true;
-    }
 
-    if (virtPortTypeID) {
-        unsigned int val;
+        if (rc > 0) {
+            if (val > 0xffffff) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("value for typeid out of range"));
+                return NULL;
+            }
 
-        if (virStrToLong_ui(virtPortTypeID, NULL, 0, &val)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("cannot parse value of typeid parameter"));
-            return NULL;
+            virtPort->typeID = (uint32_t)val;
+            virtPort->typeID_specified = true;
         }
-        if (val > 0xffffff) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("value for typeid out of range"));
+
+        if ((rc = virXMLPropUInt(parameters, "typeidversion", 10, VIR_XML_PROP_NONE, &val)) < 0)
             return NULL;
-        }
-        virtPort->typeID = (uint32_t)val;
-        virtPort->typeID_specified = true;
-    }
 
-    if (virtPortTypeIDVersion) {
-        unsigned int val;
+        if (rc > 0) {
+            if (val > 0xff) {
+                virReportError(VIR_ERR_XML_ERROR, "%s",
+                               _("value of typeidversion out of range"));
+                return NULL;
+            }
 
-        if (virStrToLong_ui(virtPortTypeIDVersion, NULL, 0, &val)) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("cannot parse value of typeidversion parameter"));
-            return NULL;
+            virtPort->typeIDVersion = (uint8_t)val;
+            virtPort->typeIDVersion_specified = true;
         }
-        if (val > 0xff) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("value of typeidversion out of range"));
+
+        if ((rc = virXMLPropUUID(parameters, "instanceid", VIR_XML_PROP_NONE, virtPort->instanceID)) < 0)
             return NULL;
-        }
-        virtPort->typeIDVersion = (uint8_t)val;
-        virtPort->typeIDVersion_specified = true;
-    }
 
-    if (virtPortInstanceID) {
-        if (virUUIDParse(virtPortInstanceID, virtPort->instanceID) < 0) {
-            virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("cannot parse instanceid parameter as a uuid"));
+        if (rc > 0)
+            virtPort->instanceID_specified = true;
+
+        if ((rc = virXMLPropUUID(parameters, "interfaceid", VIR_XML_PROP_NONE, virtPort->interfaceID)) < 0)
             return NULL;
-        }
-        virtPort->instanceID_specified = true;
-    }
 
-    if (virtPortProfileID &&
-        virStrcpyStatic(virtPort->profileID, virtPortProfileID) < 0) {
-        virReportError(VIR_ERR_XML_ERROR, "%s",
-                       _("profileid parameter too long"));
-        return NULL;
-    }
+        if (rc > 0)
+            virtPort->interfaceID_specified = true;
 
-    if (virtPortInterfaceID) {
-        if (virUUIDParse(virtPortInterfaceID, virtPort->interfaceID) < 0) {
+        if (virtPortProfileID &&
+            virStrcpyStatic(virtPort->profileID, virtPortProfileID) < 0) {
             virReportError(VIR_ERR_XML_ERROR, "%s",
-                           _("cannot parse interfaceid parameter as a uuid"));
+                           _("profileid parameter too long"));
             return NULL;
         }
-        virtPort->interfaceID_specified = true;
     }
 
     /* generate default instanceID/interfaceID if appropriate */
-- 
2.49.0
Re: [PATCH v2 1/4] conf: virNetDevVPortProfileParse refactor
Posted by Michal Prívozník via Devel 1 month, 2 weeks ago
On 7/22/25 17:12, Kirill Shchetiniuk via Devel wrote:
> From: Kirill Shchetiniuk <kshcheti@redhat.com>
> 
> Refactored the virNetDevVPortProfileParse function to use the appropriate
> virXMLProp* functions to parse input configuration XML.
> 
> Signed-off-by: Kirill Shchetiniuk <kshcheti@redhat.com>
> ---
>  src/conf/netdev_vport_profile_conf.c | 112 +++++++++++----------------
>  1 file changed, 44 insertions(+), 68 deletions(-)
> 
> diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c
> index 032a3147d7..92819c2f34 100644
> --- a/src/conf/netdev_vport_profile_conf.c
> +++ b/src/conf/netdev_vport_profile_conf.c
> @@ -29,12 +29,6 @@ virNetDevVPortProfile *
>  virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags)
>  {
>      g_autofree char *virtPortType = NULL;
> -    g_autofree char *virtPortManagerID = NULL;
> -    g_autofree char *virtPortTypeID = NULL;
> -    g_autofree char *virtPortTypeIDVersion = NULL;
> -    g_autofree char *virtPortInstanceID = NULL;
> -    g_autofree char *virtPortProfileID = NULL;
> -    g_autofree char *virtPortInterfaceID = NULL;
>      g_autofree virNetDevVPortProfile *virtPort = NULL;
>      xmlNodePtr parameters;
>  
> @@ -55,88 +49,70 @@ virNetDevVPortProfileParse(xmlNodePtr node, unsigned int flags)
>      }
>  
>      if ((parameters = virXMLNodeGetSubelement(node, "parameters"))) {
> -        virtPortManagerID = virXMLPropString(parameters, "managerid");
> -        virtPortTypeID = virXMLPropString(parameters, "typeid");
> -        virtPortTypeIDVersion = virXMLPropString(parameters, "typeidversion");
> -        virtPortInstanceID = virXMLPropString(parameters, "instanceid");
> -        virtPortProfileID = virXMLPropString(parameters, "profileid");
> -        virtPortInterfaceID = virXMLPropString(parameters, "interfaceid");
> -    }
> -
> -    if (virtPortManagerID) {
> +        int rc;
>          unsigned int val;
> +        g_autofree char *virtPortProfileID = virXMLPropString(parameters, "profileid");
>  
> -        if (virStrToLong_ui(virtPortManagerID, NULL, 0, &val)) {
> -            virReportError(VIR_ERR_XML_ERROR, "%s",
> -                           _("cannot parse value of managerid parameter"));
> +        if ((rc = virXMLPropUInt(parameters, "managerid", 10, VIR_XML_PROP_NONE, &val)) < 0)

We don't really like long lines. Our coding style says there's a limit
at 80 characters.

>              return NULL;
> +
> +        if (rc > 0) {
> +            if (val > 0xff) {
> +                virReportError(VIR_ERR_XML_ERROR, "%s",
> +                               _("value of managerid out of range"));
> +                return NULL;
> +            }
> +
> +            virtPort->managerID = (uint8_t)val;

This seems rather redundant. The type of managerID is already uint8_t.
Both compilers that we care about (gcc and clang) handle this
typecasting automatically.

> +            virtPort->managerID_specified = true;
>          }

Michal