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/po/POTFILES.in b/po/POTFILES.in
index 2d7eb1e..a0bd358 100644
--- a/po/POTFILES.in
+++ b/po/POTFILES.in
@@ -3,6 +3,7 @@
 @BUILDDIR@/src/access/viraccessapicheckqemu.c
 @BUILDDIR@/src/admin/admin_client.h
 @BUILDDIR@/src/admin/admin_server_dispatch_stubs.h
+@BUILDDIR@/src/conf/network_conf.generated.c
 @BUILDDIR@/src/remote/remote_client_bodies.h
 @BUILDDIR@/src/remote/remote_daemon_dispatch_stubs.h
 @SRCDIR@/build-aux/generator/directive.py
diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am
index 3bd2199..0b36a8b 100644
--- a/src/conf/Makefile.inc.am
+++ b/src/conf/Makefile.inc.am
@@ -161,6 +161,8 @@ DEVICE_CONF_SOURCES = \
 	$(NULL)
 
 CONF_GENERATED_SOURCES = \
+	conf/network_conf.generated.c \
+	conf/network_conf.generated.h \
 	$(NULL)
 
 CONF_SOURCES = \
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;
-}
-
-
 static int
 virNetworkDNSDefParseXML(const char *networkName,
                          xmlNodePtr node,
@@ -1100,10 +1065,10 @@ virNetworkDNSDefParseXML(const char *networkName,
             goto cleanup;
 
         for (i = 0; i < ntxts; i++) {
-            if (virNetworkDNSTxtDefParseXML(networkName, txtNodes[i],
-                                            &def->txts[def->ntxts], false) < 0) {
+            if (virNetworkDNSTxtDefParseXML(txtNodes[i], &def->txts[def->ntxts],
+                                            networkName, NULL) < 0)
                 goto cleanup;
-            }
+
             def->ntxts++;
         }
     }
@@ -3714,6 +3679,7 @@ virNetworkDefUpdateDNSTxt(virNetworkDefPtr def,
     virNetworkDNSTxtDef txt;
     bool isAdd = (command == VIR_NETWORK_UPDATE_COMMAND_ADD_FIRST ||
                   command == VIR_NETWORK_UPDATE_COMMAND_ADD_LAST);
+    bool notAdd;
 
     memset(&txt, 0, sizeof(txt));
 
@@ -3727,7 +3693,8 @@ virNetworkDefUpdateDNSTxt(virNetworkDefPtr def,
     if (virNetworkDefUpdateCheckElementName(def, ctxt->node, "txt") < 0)
         goto cleanup;
 
-    if (virNetworkDNSTxtDefParseXML(def->name, ctxt->node, &txt, !isAdd) < 0)
+    notAdd = !isAdd;
+    if (virNetworkDNSTxtDefParseXML(ctxt->node, &txt, def->name, &notAdd) < 0)
         goto cleanup;
 
     for (foundIdx = 0; foundIdx < dns->ntxts; foundIdx++) {
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index f2dc388..eac8a76 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -130,9 +130,9 @@ struct _virNetworkDHCPHostDef {
 
 typedef struct _virNetworkDNSTxtDef virNetworkDNSTxtDef;
 typedef virNetworkDNSTxtDef *virNetworkDNSTxtDefPtr;
-struct _virNetworkDNSTxtDef {
-    char *name;
-    char *value;
+struct _virNetworkDNSTxtDef {   /* genparse:concisehook */
+    char *name;                 /* xmlattr, required */
+    char *value;                /* xmlattr */
 };
 
 typedef struct _virNetworkDNSSrvDef virNetworkDNSSrvDef;
@@ -440,3 +440,5 @@ virNetworkDefUpdateSection(virNetworkDefPtr def,
                            unsigned int flags);  /* virNetworkUpdateFlags */
 
 VIR_ENUM_DECL(virNetworkTaint);
+
+#include "network_conf.generated.h"
-- 
2.17.1

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()



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 :|