Re: [PATCH 1/5] conf: virNetDevVPortProfileParse refactor

14. 7. 2025 v 19:22, Ján Tomko <jtomko(a)redhat.com&gt;: On a Monday in 2025, Kirill Shchetiniuk via Devel wrote: > From: Kirill Shchetiniuk <kshcheti(a)redhat.com&gt; > > Refactored the virNetDevVPortProfileParse function to use the appropriate > virXMLProp* functions to parse input configuration XML. > > Signed-off-by: Kirill Shchetiniuk <kshcheti(a)redhat.com&gt; > --- > src/conf/netdev_vport_profile_conf.c | 120 +++++++++++---------------- > 1 file changed, 48 insertions(+), 72 deletions(-) > > diff --git a/src/conf/netdev_vport_profile_conf.c b/src/conf/netdev_vport_profile_conf.c > index 032a3147d7..9be19de808 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"); > - } > + int ret; Generally, we use 'ret' as the value that will be returned by the current function, and 'rc' for storing temporary values of the functions we call. > + unsigned int tempUInt; There's no need to encode the type in the variable name, the old 'val' was just fine. > + g_autofree char *virtPortProfileID = virXMLPropString(parameters, "profileid"); > > - if (virtPortManagerID) { > - unsigned int val; > + if ((ret = virXMLPropUInt(parameters, "managerid", 10, VIR_XML_PROP_NONE, &tempUInt))) { Omitting the != 0 should be avoided according to our coding style. https://libvirt.org/coding-style.html#conditional-expressions Moreover, writing this as: if ((rc = virXMLPropUint(...)) < 0) return NULL; if (rc > 0) { .... } is easier to read, IMO > + if (ret < 0) > + return NULL; > Jano <signature.asc>