In some places when generating XML output in C code we use some clever
macros:

  start_element ("memory") {
    attribute ("unit", "MiB");
    string_format ("%d", g->memsize);
  } end_element ();

This commit which is mostly refactoring moves the repeated definitions
of these macros into a common header file.

I also took this opportunity to change / clean up the macros:

 - The macros are now documented properly.

 - comment() and empty_element() macros are now available everywhere.

 - Error handling has been made generic.

 - Added do..while(0) around some of the macros to make them safe to
   use in all contexts.

The last point causes the most churn since we must change the callers
to avoid format string security bugs.
---
 common/utils/Makefile.am             |   1 +
 common/utils/libxml2-writer-macros.h | 159 +++++++++++++++++++++++++++
 docs/C_SOURCE_FILES                  |   1 +
 lib/launch-libvirt.c                 |  83 ++------------
 p2v/physical-xml.c                   |  65 ++---------
 5 files changed, 181 insertions(+), 128 deletions(-)

diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
index 143e2c141..a0154d85f 100644
--- a/common/utils/Makefile.am
+++ b/common/utils/Makefile.am
@@ -26,6 +26,7 @@ libutils_la_SOURCES = \
        gnulib-cleanups.c \
        guestfs-utils.h \
        libxml2-cleanups.c \
+       libxml2-writer-macros.h \
        utils.c
 libutils_la_CPPFLAGS = \
        -DGUESTFS_WARN_DEPRECATED=1 \
diff --git a/common/utils/libxml2-writer-macros.h 
b/common/utils/libxml2-writer-macros.h
new file mode 100644
index 000000000..e8f3726f1
--- /dev/null
+++ b/common/utils/libxml2-writer-macros.h
@@ -0,0 +1,159 @@
+/* libguestfs
+ * Copyright (C) 2009-2018 Red Hat Inc.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+/**
+ * These macros make it easier to write XML.  To use them correctly
+ * you must be aware of these assumptions:
+ *
+ * =over 4
+ *
+ * =item *
+ *
+ * The C<xmlTextWriterPtr> is called C<xo>.  It is used implicitly
+ * by all the macros.
+ *
+ * =item *
+ *
+ * On failure, a function called C<xml_error> is called which you must
+ * define (usually as a macro).  You must use C<CLEANUP_*> macros in
+ * your functions if you want correct cleanup of local variables along
+ * the error path.
+ *
+ * =item *
+ *
+ * All the "bad" casting is hidden inside the macros.
+ *
+ * =back
+ */
+
+#ifndef GUESTFS_LIBXML2_WRITER_MACROS_H_
+#define GUESTFS_LIBXML2_WRITER_MACROS_H_
+
+#include <stdarg.h>
+
+/**
+ * To define an XML element use:
+ *
+ *  start_element ("name") {
+ *    ...
+ *  } end_element ();
+ *
+ * which produces C<<< <name>...</name> >>>
+ */
+#define start_element(element)                                         \
+  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) {      \
+    xml_error ("xmlTextWriterStartElement");                           \
+  }                                                                    \
+  do
+
+#define end_element()                          \
+  while (0);                                   \
+  do {                                         \
+    if (xmlTextWriterEndElement (xo) == -1) {  \
+      xml_error ("xmlTextWriterEndElement");   \
+    }                                          \
+  } while (0)
+
+/**
+ * To define an empty element:
+ *
+ *  empty_element ("name");
+ *
+ * which produces C<<< <name/> >>>
+ */
+#define empty_element(element)                                  \
+  do { start_element ((element)) {} end_element (); } while (0)
+
+/**
+ * To define an XML element with attributes, use:
+ *
+ *  start_element ("name") {
+ *    attribute ("foo", "bar");
+ *    attribute_format ("count", "%d", count);
+ *    ...
+ *  } end_element ();
+ *
+ * which produces C<<< <name foo="bar" count="123">...</name> >>>
+ */
+#define attribute(key,value)                                            \
+  do {                                                                  \
+    if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key),                \
+                                     BAD_CAST (value)) == -1) {         \
+      xml_error ("xmlTextWriterWriteAttribute");                        \
+    }                                                                   \
+  } while (0)
+
+#define attribute_format(key,fs,...)                                    \
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),          \
+                                           fs, ##__VA_ARGS__) == -1) {  \
+      xml_error ("xmlTextWriterWriteFormatAttribute");                  \
+    }                                                                   \
+  } while (0)
+
+/**
+ * C<attribute_ns (prefix, key, namespace_uri, value)> defines a
+ * namespaced attribute.
+ */
+#define attribute_ns(prefix,key,namespace_uri,value)                    \
+  do {                                                                  \
+    if (xmlTextWriterWriteAttributeNS (xo, BAD_CAST (prefix),           \
+                                       BAD_CAST (key),                  \
+                                       BAD_CAST (namespace_uri),        \
+                                       BAD_CAST (value)) == -1) {       \
+      xml_error ("xmlTextWriterWriteAttribute");                        \
+    }                                                                   \
+  } while (0)
+
+/**
+ * To define a verbatim string, use:
+ *
+ *  string ("hello");
+ */
+#define string(str)                                                     \
+  do {                                                                  \
+    if (xmlTextWriterWriteString (xo, BAD_CAST(str)) == -1) {           \
+      xml_error ("xmlTextWriterWriteString");                           \
+    }                                                                   \
+  } while (0)
+
+/**
+ * To define a verbatim string using a format string, use:
+ *
+ *  string ("%s, world", greeting);
+ */
+#define string_format(fs,...)                                           \
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) { \
+      xml_error ("xmlTextWriterWriteFormatString");                     \
+    }                                                                   \
+  } while (0)
+
+/**
+ * To define a comment in the XML, use:
+ *
+ *   comment ("number of items = %d", nr_items);
+ */
+#define comment(fs,...)                                                 \
+  do {                                                                  \
+    if (xmlTextWriterWriteFormatComment (xo, fs, ##__VA_ARGS__) == -1) { \
+      xml_error ("xmlTextWriterWriteFormatComment");                    \
+    }                                                                   \
+  } while (0)
+
+#endif /* GUESTFS_LIBXML2_WRITER_MACROS_H_ */
diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 1c262973c..7f1c60b30 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -62,6 +62,7 @@ common/utils/cleanups.h
 common/utils/gnulib-cleanups.c
 common/utils/guestfs-utils.h
 common/utils/libxml2-cleanups.c
+common/utils/libxml2-writer-macros.h
 common/utils/utils.c
 common/visit/visit.c
 common/visit/visit.h
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 7121aee1b..f80932001 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -52,6 +52,16 @@
 #include "guestfs-internal.h"
 #include "guestfs_protocol.h"
 
+#include "libxml2-writer-macros.h"
+
+/* This macro is used by the macros in "libxml2-writer-macros.h"
+ * when an error occurs.
+ */
+#define xml_error(fn)                                                   \
+  perrorf (g, _("%s:%d: error constructing libvirt XML near call to \"%s\""), \
+          __FILE__, __LINE__, (fn));                                   \
+  return -1;
+
 /* Fixes for Mac OS X */
 #ifndef SOCK_CLOEXEC
 # define SOCK_CLOEXEC O_CLOEXEC
@@ -956,79 +966,6 @@ static int construct_libvirt_xml_disk_source_hosts 
(guestfs_h *g, xmlTextWriterP
 static int construct_libvirt_xml_disk_source_seclabel (guestfs_h *g, const 
struct backend_libvirt_data *data, xmlTextWriterPtr xo);
 static int construct_libvirt_xml_appliance (guestfs_h *g, const struct 
libvirt_xml_params *params, xmlTextWriterPtr xo);
 
-/* These macros make it easier to write XML, but they also make a lot
- * of assumptions:
- *
- * - The xmlTextWriterPtr is called 'xo'.  It is used implicitly.
- *
- * - The guestfs handle is called 'g'.  It is used implicitly for errors.
- *
- * - It is safe to 'return -1' on failure.  This is OK provided you
- *   always use CLEANUP_* macros.
- *
- * - All the "bad" casting is hidden inside the macros.
- */
-
-/* <element */
-#define start_element(element)                                         \
-  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1) {      \
-    xml_error ("xmlTextWriterStartElement");                           \
-    return -1;                                                         \
-  }                                                                    \
-  do
-
-/* finish current </element> */
-#define end_element()                          \
-  while (0);                                   \
-  do {                                         \
-    if (xmlTextWriterEndElement (xo) == -1) {  \
-      xml_error ("xmlTextWriterEndElement");   \
-      return -1;                               \
-    }                                          \
-  } while (0)
-
-/* key=value attribute of the current element. */
-#define attribute(key,value)                                            \
-  if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) == 
-1){ \
-    xml_error ("xmlTextWriterWriteAttribute");                          \
-    return -1;                                                          \
-  }
-
-/* key=value, but value is a printf-style format string. */
-#define attribute_format(key,fs,...)                                    \
-  if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),            \
-                                         fs, ##__VA_ARGS__) == -1) {    \
-    xml_error ("xmlTextWriterWriteFormatAttribute");                    \
-    return -1;                                                          \
-  }
-
-/* attribute with namespace. */
-#define attribute_ns(prefix,key,namespace_uri,value)                    \
-  if (xmlTextWriterWriteAttributeNS (xo, BAD_CAST (prefix),             \
-                                     BAD_CAST (key), BAD_CAST (namespace_uri), 
\
-                                     BAD_CAST (value)) == -1) {         \
-    xml_error ("xmlTextWriterWriteAttribute");                          \
-    return -1;                                                          \
-  }
-
-/* A string, eg. within an element. */
-#define string(str)                                            \
-  if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1) {   \
-    xml_error ("xmlTextWriterWriteString");                    \
-    return -1;                                                 \
-  }
-
-/* A string, using printf-style formatting. */
-#define string_format(fs,...)                                           \
-  if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1) {   \
-    xml_error ("xmlTextWriterWriteFormatString");                       \
-    return -1;                                                          \
-  }
-
-#define xml_error(fn)                                                   \
-  perrorf (g, _("%s:%d: error constructing libvirt XML near call to \"%s\""), \
-          __FILE__, __LINE__, (fn));
-
 static xmlChar *
 construct_libvirt_xml (guestfs_h *g, const struct libvirt_xml_params *params)
 {
diff --git a/p2v/physical-xml.c b/p2v/physical-xml.c
index f65a514d5..b2169fc83 100644
--- a/p2v/physical-xml.c
+++ b/p2v/physical-xml.c
@@ -39,65 +39,20 @@
 
 #include "getprogname.h"
 
+#include "libxml2-writer-macros.h"
+
 #include "p2v.h"
 
+/* This macro is used by the macros in "libxml2-writer-macros.h"
+ * when an error occurs.
+ */
+#define xml_error(fn)                                           \
+  error (EXIT_FAILURE, errno,                                   \
+         "%s:%d: error constructing XML near call to \"%s\"",   \
+         __FILE__, __LINE__, (fn));
+
 static const char *map_interface_to_network (struct config *, const char 
*interface);
 
-/* Macros "inspired" by lib/launch-libvirt.c */
-/* <element */
-#define start_element(element)                                         \
-  if (xmlTextWriterStartElement (xo, BAD_CAST (element)) == -1)         \
-    error (EXIT_FAILURE, errno, "xmlTextWriterStartElement");          \
-  do
-
-/* finish current </element> */
-#define end_element()                                          \
-  while (0);                                                   \
-  do {                                                         \
-    if (xmlTextWriterEndElement (xo) == -1)                    \
-      error (EXIT_FAILURE, errno, "xmlTextWriterEndElement");  \
-  } while (0)
-
-/* <element/> */
-#define empty_element(element)                                 \
-  do { start_element(element) {} end_element (); } while (0)
-
-/* key=value attribute of the current element. */
-#define attribute(key,value)                                            \
-  do {                                                                  \
-    if (xmlTextWriterWriteAttribute (xo, BAD_CAST (key), BAD_CAST (value)) == 
-1) \
-    error (EXIT_FAILURE, errno, "xmlTextWriterWriteAttribute");         \
-  } while (0)
-
-/* key=value, but value is a printf-style format string. */
-#define attribute_format(key,fs,...)                                    \
-  do {                                                                  \
-    if (xmlTextWriterWriteFormatAttribute (xo, BAD_CAST (key),          \
-                                           fs, ##__VA_ARGS__) == -1)   \
-      error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatAttribute"); \
-  } while (0)
-
-/* A string, eg. within an element. */
-#define string(str)                                             \
-  do {                                                          \
-    if (xmlTextWriterWriteString (xo, BAD_CAST (str)) == -1)   \
-      error (EXIT_FAILURE, errno, "xmlTextWriterWriteString"); \
-  } while (0)
-
-/* A string, using printf-style formatting. */
-#define string_format(fs,...)                                           \
-  do {                                                                  \
-    if (xmlTextWriterWriteFormatString (xo, fs, ##__VA_ARGS__) == -1)   \
-      error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatString");    \
-  } while (0)
-
-/* An XML comment. */
-#define comment(fs,...)                                                 \
-  do {                                                                  \
-    if (xmlTextWriterWriteFormatComment (xo, fs, ##__VA_ARGS__) == -1) \
-      error (EXIT_FAILURE, errno, "xmlTextWriterWriteFormatComment");   \
-  } while (0)
-
 /**
  * Write the libvirt XML for this physical machine.
  *
-- 
2.19.0.rc0

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

Reply via email to