Re: [RFC 07/21] conf: Replace virNetworkDNSTxtDefParseXML(hardcoded) with namesake(generated)
On Wed, Jun 10, 2020 at 09:20:35AM +0800, Shi Lei wrote:
Signed-off-by: Shi Lei <shi_lei(a)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()
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 :|