>On Wed, Jun 10, 2020 at 09:20:35AM +0800, Shi Lei wrote:
>> Signed-off-by: Shi Lei <shi_lei@xxxxxxxxxxxxxx>
>> ---
>> 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 :|
>
