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