> 14. 7. 2025 v 19:22, Ján Tomko <jto...@redhat.com>:
>
> On a Monday in 2025, Kirill Shchetiniuk via Devel wrote:
>> From: Kirill Shchetiniuk <kshch...@redhat.com>
>>
>> Refactored the virNetDevVPortProfileParse function to use the appropriate
>> virXMLProp* functions to parse input configuration XML.
>>
>> Signed-off-by: Kirill Shchetiniuk <kshch...@redhat.com>
>> ---
>> 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>
Sure, will be fixed within next series, thanks!
Kirill
