[RFC 07/21] conf: Replace virNetworkDNSTxtDefParseXML(hardcoded) with namesake(generated)

[Shi Lei]

>On Wed, Jun 10, 2020 at 09:20:35AM +0800, Shi Lei wrote:
>> Signed-off-by: Shi Lei <shi_lei at massclouds.com>
>> ---
>>  po/POTFILES.in           |  1 +
>>  src/conf/Makefile.inc.am |  2 ++
>>  src/conf/network_conf.c  | 47 ++++++----------------------------------
>>  src/conf/network_conf.h  |  8 ++++---
>>  4 files changed, 15 insertions(+), 43 deletions(-)
>
>> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
>> index 47aaef3..964a8a7 100644
>> --- a/src/conf/network_conf.c
>> +++ b/src/conf/network_conf.c
>> @@ -174,14 +174,6 @@ virNetworkIPDefClear(virNetworkIPDefPtr def)
>>  }
>> 
>> 
>> -static void
>> -virNetworkDNSTxtDefClear(virNetworkDNSTxtDefPtr def)
>> -{
>> -    VIR_FREE(def->name);
>> -    VIR_FREE(def->value);
>> -}
>> -
>> -
>>  static void
>>  virNetworkDNSHostDefClear(virNetworkDNSHostDefPtr def)
>>  {
>> @@ -903,7 +895,7 @@ virNetworkDNSSrvDefParseXML(const char *networkName,
>>  }
>> 
>> 
>> -static int
>> +int
>>  virNetworkDNSTxtDefParseXMLHook(xmlNodePtr node G_GNUC_UNUSED,
>>                                  virNetworkDNSTxtDefPtr def,
>>                                  const char *networkName,
>> @@ -943,33 +935,6 @@ virNetworkDNSTxtDefParseXMLHook(xmlNodePtr node G_GNUC_UNUSED,
>>  }
>> 
>> 
>> -static int
>> -virNetworkDNSTxtDefParseXML(const char *networkName,
>> -                            xmlNodePtr node,
>> -                            virNetworkDNSTxtDefPtr def,
>> -                            bool partialOkay)
>> -{
>> -    if (!(def->name = virXMLPropString(node, "name"))) {
>> -        virReportError(VIR_ERR_XML_DETAIL,
>> -                       _("missing required name attribute in DNS TXT record "
>> -                         "of network %s"), networkName);
>> -        goto error;
>> -    }
>> -
>> -    def->value = virXMLPropString(node, "value");
>> -
>> -    if (virNetworkDNSTxtDefParseXMLHook(node, def, networkName,
>> -                                        &partialOkay) < 0)
>> -        goto error;
>> -
>> -    return 0;
>> -
>> - error:
>> -    virNetworkDNSTxtDefClear(def);
>> -    return -1;
>> -}
>
>Comparing this old code to the new generated code:
>
>
>int
>virNetworkDNSTxtDefParseXML(xmlNodePtr node,
>                            virNetworkDNSTxtDefPtr def,
>                            const char *instname,
>                            void *opaque)
>{
>    g_autofree char *nameStr = NULL;
>    g_autofree char *valueStr = NULL;
>    VIR_USED(instname);
>    VIR_USED(opaque);
>
>    if (!def)
>        goto error;
>
>    nameStr = virXMLPropString(node, "name");
>    if (nameStr == NULL) {
>        virReportError(VIR_ERR_XML_ERROR,
>                       _("Missing 'name' setting in '%s'"),
>                       instname);
>        goto error;
>    }
>
>    def->name = g_strdup(nameStr);
>
>    valueStr = virXMLPropString(node, "value");
>    if (valueStr)
>        def->value = g_strdup(valueStr);
>
>    if (virNetworkDNSTxtDefParseXMLHook(node, def, instname, opaque) < 0)
>        goto error;
>
>    return 0;
>
> error:
>   
>    virNetworkDNSTxtDefClear(def);
>    return -1;
>}
>
>
>The main changes I'd suggest are
>
>*  Change
> 
>        virReportError(VIR_ERR_XML_ERROR,
>                       _("Missing 'name' setting in '%s'"),
>                       instname);
>
>   To
>
> 
>        virReportError(VIR_ERR_XML_ERROR,
>                       _("Missing '%s' attribute in '%s'"),
>                       "name", instname);
>
>  so that it reduces the number of translatable strings
>
>* Remove the extra g_strdup's
>
>
>In an earlier patch I complained about your use of Clear()
>functions. Now I see this patch though, I understand why
>you were using Clear() - this pointer is just a member of
>an array, so we can't directly Free() it.  So I withdraw
>my objection on the earlier patch about use of Clear()
> 

Got it!

Regards,
Shi Lei

>
>
>Regards,
>Daniel
>--
>|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>|: https://libvirt.org -o- https://fstop138.berrange.com :|
>|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>