Richard W.M. Jones
2018-Nov-02 15:05 UTC
[Libguestfs] [PATCH v3 0/4] common/utils: Move libxml2 writer macros to a common header file.
v1 was here: https://www.redhat.com/archives/libguestfs/2018-October/msg00047.html v2 was here: https://www.redhat.com/archives/libguestfs/2018-October/msg00051.html v3: - Back to using string/string_format and attribute/attribute_format. - Add both single_element and single_element_format. - Rebased and retested. Rich.
Richard W.M. Jones
2018-Nov-02 15:05 UTC
[Libguestfs] [PATCH v3 1/4] docs: Allow enhanced comments for macros (ie. #define).
---
docs/make-internal-documentation.pl | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/docs/make-internal-documentation.pl
b/docs/make-internal-documentation.pl
index a6673c48f..e08adad70 100755
--- a/docs/make-internal-documentation.pl
+++ b/docs/make-internal-documentation.pl
@@ -245,13 +245,19 @@ foreach $dir (@dirs) {
}
}
else {
- # First line tells us if this is a struct
- # or function.
+ # First line tells us if this is a struct,
+ # define or function.
if (/^struct ([\w_]+) \{$/) {
$thing = "Structure";
$name = $1;
$end = "};";
}
+ elsif (/^#define ([\w_]+)/) {
+ $thing = "Definition";
+ $name = $1;
+ $found_end = 1;
+ last;
+ }
else {
$thing = "Function";
$end = "{";
@@ -276,6 +282,10 @@ foreach $dir (@dirs) {
push @defn, "};"
}
+ if ($thing eq "Definition") {
+ @defn = ( "#define $name" )
+ }
+
# Print the definition, followed by the comment.
print OUTPUT "=head4 $thing
C<$input:$name>\n\n";
print OUTPUT " ", join ("\n ", @defn),
"\n";
--
2.19.0.rc0
Richard W.M. Jones
2018-Nov-02 15:05 UTC
[Libguestfs] [PATCH v3 2/4] common/utils: Move libxml2 writer macros to a common header file.
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
Richard W.M. Jones
2018-Nov-02 15:05 UTC
[Libguestfs] [PATCH v3 3/4] inspector: Use libxml writer macros.
Change virt-inspector so it uses the common set of macros. I also
added:
- single_element() and single_element_format():
creates <foo>bar</foo> which is used extensively by
virt-inspector
- base64():
used by virt-inspector for the icon
---
common/utils/libxml2-writer-macros.h | 41 +++
inspector/inspector.c | 521 ++++++++++++---------------
2 files changed, 270 insertions(+), 292 deletions(-)
diff --git a/common/utils/libxml2-writer-macros.h
b/common/utils/libxml2-writer-macros.h
index e8f3726f1..d74dcd465 100644
--- a/common/utils/libxml2-writer-macros.h
+++ b/common/utils/libxml2-writer-macros.h
@@ -79,6 +79,35 @@
#define empty_element(element) \
do { start_element ((element)) {} end_element (); } while (0)
+/**
+ * To define a single element with no attributes containing some text:
+ *
+ * single_element ("name", text);
+ *
+ * which produces C<<< <name>text</name> >>>
+ */
+#define single_element(element, str) \
+ do { \
+ start_element ((element)) { \
+ string ((str)); \
+ } end_element (); \
+ } while (0)
+
+/**
+ * To define a single element with no attributes containing some text
+ * using a format string:
+ *
+ * single_element_format ("cores", "%d", nr_cores);
+ *
+ * which produces C<<< <cores>4</cores> >>>
+ */
+#define single_element_format(element,fs,...) \
+ do { \
+ start_element ((element)) { \
+ string_format ((fs), ##__VA_ARGS__); \
+ } end_element (); \
+ } while (0)
+
/**
* To define an XML element with attributes, use:
*
@@ -144,6 +173,18 @@
} \
} while (0)
+/**
+ * To write a string encoded as base64:
+ *
+ * base64 (data, size);
+ */
+#define base64(data, size) \
+ do { \
+ if (xmlTextWriterWriteBase64 (xo, (data), 0, (size)) == -1) { \
+ xml_error ("xmlTextWriterWriteBase64");
\
+ } \
+ } while (0)
+
/**
* To define a comment in the XML, use:
*
diff --git a/inspector/inspector.c b/inspector/inspector.c
index 9be8f5110..fa8e721ff 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -43,6 +43,7 @@
#include "structs-cleanups.h"
#include "options.h"
#include "display-options.h"
+#include "libxml2-writer-macros.h"
/* Currently open libguestfs handle. */
guestfs_h *g;
@@ -69,6 +70,14 @@ static void output_drive_mappings (xmlTextWriterPtr xo, char
*root);
static void output_applications (xmlTextWriterPtr xo, char *root);
static void do_xpath (const char *query);
+/* 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 void __attribute__((noreturn))
usage (int status)
{
@@ -307,11 +316,6 @@ main (int argc, char *argv[])
exit (EXIT_SUCCESS);
}
-#define XMLERROR(code,e) do { \
- if ((e) == (code)) \
- error (EXIT_FAILURE, errno, _("XML write error at
\"%s\""), #e); \
- } while (0)
-
static void
output (char **roots)
{
@@ -327,12 +331,17 @@ output (char **roots)
_("xmlNewTextWriter: failed to create libxml2 writer"));
/* Pretty-print the output. */
- XMLERROR (-1, xmlTextWriterSetIndent (xo, 1));
- XMLERROR (-1, xmlTextWriterSetIndentString (xo, BAD_CAST " "));
+ if (xmlTextWriterSetIndent (xo, 1) == -1 ||
+ xmlTextWriterSetIndentString (xo, BAD_CAST " ") == -1)
+ error (EXIT_FAILURE, errno, "could not set XML indent");
+
+ if (xmlTextWriterStartDocument (xo, NULL, NULL, NULL) == -1)
+ error (EXIT_FAILURE, errno, "xmlTextWriterStartDocument");
- XMLERROR (-1, xmlTextWriterStartDocument (xo, NULL, NULL, NULL));
output_roots (xo, roots);
- XMLERROR (-1, xmlTextWriterEndDocument (xo));
+
+ if (xmlTextWriterEndDocument (xo) == -1)
+ error (EXIT_FAILURE, errno, "xmlTextWriterEndDocument");
}
static void
@@ -340,10 +349,10 @@ output_roots (xmlTextWriterPtr xo, char **roots)
{
size_t i;
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystems"));
- for (i = 0; roots[i] != NULL; ++i)
- output_root (xo, roots[i]);
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("operatingsystems") {
+ for (i = 0; roots[i] != NULL; ++i)
+ output_root (xo, roots[i]);
+ } end_element ();
}
static void
@@ -351,152 +360,129 @@ output_root (xmlTextWriterPtr xo, char *root)
{
char *str;
int i;
- char buf[32];
char *canonical_root;
size_t size;
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"operatingsystem"));
-
- canonical_root = guestfs_canonical_device_name (g, root);
- if (canonical_root == NULL)
- exit (EXIT_FAILURE);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "root", BAD_CAST
canonical_root));
- free (canonical_root);
-
- str = guestfs_inspect_get_type (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "name", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_arch (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "arch", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_distro (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "distro", BAD_CAST
str));
- free (str);
-
- str = guestfs_inspect_get_product_name (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "product_name",
BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_product_variant (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "product_variant",
BAD_CAST str));
- free (str);
-
- i = guestfs_inspect_get_major_version (g, root);
- snprintf (buf, sizeof buf, "%d", i);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "major_version",
BAD_CAST buf));
- i = guestfs_inspect_get_minor_version (g, root);
- snprintf (buf, sizeof buf, "%d", i);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "minor_version",
BAD_CAST buf));
-
- str = guestfs_inspect_get_package_format (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "package_format",
BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_package_management (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "package_management",
- BAD_CAST str));
- free (str);
-
- /* inspect-get-windows-systemroot will fail with non-windows guests,
- * or if the systemroot could not be determined for a windows guest.
- * Disable error output around this call.
- */
- guestfs_push_error_handler (g, NULL, NULL);
- str = guestfs_inspect_get_windows_systemroot (g, root);
- if (str)
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST
"windows_systemroot",
- BAD_CAST str));
- free (str);
- str = guestfs_inspect_get_windows_current_control_set (g, root);
- if (str)
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST
"windows_current_control_set",
- BAD_CAST str));
- free (str);
- guestfs_pop_error_handler (g);
-
- str = guestfs_inspect_get_hostname (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "hostname",
- BAD_CAST str));
- free (str);
-
- str = guestfs_inspect_get_osinfo (g, root);
- if (!str) exit (EXIT_FAILURE);
- if (STRNEQ (str, "unknown"))
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "osinfo",
BAD_CAST str));
- free (str);
-
- output_mountpoints (xo, root);
-
- output_filesystems (xo, root);
-
- output_drive_mappings (xo, root);
-
- /* We need to mount everything up in order to read out the list of
- * applications and the icon, ie. everything below this point.
- */
- if (inspect_apps || inspect_icon) {
- inspect_mount_root (g, root);
-
- if (inspect_apps)
- output_applications (xo, root);
-
- if (inspect_icon) {
- /* Don't return favicon. RHEL 7 and Fedora have crappy 16x16
- * favicons in the base distro.
- */
- str = guestfs_inspect_get_icon (g, root, &size,
- GUESTFS_INSPECT_GET_ICON_FAVICON, 0,
- -1);
- if (!str) exit (EXIT_FAILURE);
- if (size > 0) {
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"icon"));
- XMLERROR (-1, xmlTextWriterWriteBase64 (xo, str, 0, size));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("operatingsystem") {
+ canonical_root = guestfs_canonical_device_name (g, root);
+ if (canonical_root == NULL)
+ exit (EXIT_FAILURE);
+ single_element ("root", canonical_root);
+ free (canonical_root);
+
+ str = guestfs_inspect_get_type (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("name", str);
+ free (str);
+
+ str = guestfs_inspect_get_arch (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("arch", str);
+ free (str);
+
+ str = guestfs_inspect_get_distro (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("distro", str);
+ free (str);
+
+ str = guestfs_inspect_get_product_name (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("product_name", str);
+ free (str);
+
+ str = guestfs_inspect_get_product_variant (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("product_variant", str);
+ free (str);
+
+ i = guestfs_inspect_get_major_version (g, root);
+ single_element_format ("major_version", "%d", i);
+ i = guestfs_inspect_get_minor_version (g, root);
+ single_element_format ("minor_version", "%d", i);
+
+ str = guestfs_inspect_get_package_format (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("package_format", str);
+ free (str);
+
+ str = guestfs_inspect_get_package_management (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("package_management", str);
+ free (str);
+
+ /* inspect-get-windows-systemroot will fail with non-windows guests,
+ * or if the systemroot could not be determined for a windows guest.
+ * Disable error output around this call.
+ */
+ guestfs_push_error_handler (g, NULL, NULL);
+ str = guestfs_inspect_get_windows_systemroot (g, root);
+ if (str)
+ single_element ("windows_systemroot", str);
+ free (str);
+ str = guestfs_inspect_get_windows_current_control_set (g, root);
+ if (str)
+ single_element ("windows_current_control_set", str);
+ free (str);
+ guestfs_pop_error_handler (g);
+
+ str = guestfs_inspect_get_hostname (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("hostname", str);
+ free (str);
+
+ str = guestfs_inspect_get_osinfo (g, root);
+ if (!str) exit (EXIT_FAILURE);
+ if (STRNEQ (str, "unknown"))
+ single_element ("osinfo", str);
+ free (str);
+
+ output_mountpoints (xo, root);
+
+ output_filesystems (xo, root);
+
+ output_drive_mappings (xo, root);
+
+ /* We need to mount everything up in order to read out the list of
+ * applications and the icon, ie. everything below this point.
+ */
+ if (inspect_apps || inspect_icon) {
+ inspect_mount_root (g, root);
+
+ if (inspect_apps)
+ output_applications (xo, root);
+
+ if (inspect_icon) {
+ /* Don't return favicon. RHEL 7 and Fedora have crappy 16x16
+ * favicons in the base distro.
+ */
+ str = guestfs_inspect_get_icon (g, root, &size,
+ GUESTFS_INSPECT_GET_ICON_FAVICON, 0,
+ -1);
+ if (!str) exit (EXIT_FAILURE);
+ if (size > 0) {
+ start_element ("icon") {
+ base64 (str, size);
+ } end_element ();
+ }
+ /* Note we must free (str) even if size == 0, because that indicates
+ * there was no icon.
+ */
+ free (str);
}
- /* Note we must free (str) even if size == 0, because that indicates
- * there was no icon.
- */
- free (str);
+
+ /* Unmount (see inspect_mount_root above). */
+ if (guestfs_umount_all (g) == -1)
+ exit (EXIT_FAILURE);
}
-
- /* Unmount (see inspect_mount_root above). */
- if (guestfs_umount_all (g) == -1)
- exit (EXIT_FAILURE);
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ } end_element (); /* operatingsystem */
}
static int
@@ -548,23 +534,19 @@ output_mountpoints (xmlTextWriterPtr xo, char *root)
2 * sizeof (char *),
compare_keys_len);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"mountpoints"));
+ start_element ("mountpoints") {
+ for (i = 0; mountpoints[i] != NULL; i += 2) {
+ CLEANUP_FREE char *p + guestfs_canonical_device_name (g,
mountpoints[i+1]);
+ if (!p)
+ exit (EXIT_FAILURE);
- for (i = 0; mountpoints[i] != NULL; i += 2) {
- CLEANUP_FREE char *p = guestfs_canonical_device_name (g, mountpoints[i+1]);
- if (!p)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1,
- xmlTextWriterStartElement (xo, BAD_CAST "mountpoint"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
BAD_CAST p));
- XMLERROR (-1,
- xmlTextWriterWriteString (xo, BAD_CAST mountpoints[i]));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("mountpoint") {
+ attribute ("dev", p);
+ string (mountpoints[i]);
+ } end_element ();
+ }
+ } end_element ();
}
static void
@@ -582,47 +564,37 @@ output_filesystems (xmlTextWriterPtr xo, char *root)
qsort (filesystems, guestfs_int_count_strings (filesystems), sizeof (char *),
compare_keys);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"filesystems"));
-
- for (i = 0; filesystems[i] != NULL; ++i) {
- str = guestfs_canonical_device_name (g, filesystems[i]);
- if (!str)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"filesystem"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "dev",
BAD_CAST str));
- free (str);
-
- guestfs_push_error_handler (g, NULL, NULL);
-
- str = guestfs_vfs_type (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "type",
- BAD_CAST str));
- free (str);
-
- str = guestfs_vfs_label (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "label",
- BAD_CAST str));
- free (str);
-
- str = guestfs_vfs_uuid (g, filesystems[i]);
- if (str && str[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "uuid",
- BAD_CAST str));
- free (str);
-
- guestfs_pop_error_handler (g);
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("filesystems") {
+ for (i = 0; filesystems[i] != NULL; ++i) {
+ str = guestfs_canonical_device_name (g, filesystems[i]);
+ if (!str)
+ exit (EXIT_FAILURE);
+
+ start_element ("filesystem") {
+ attribute ("dev", str);
+ free (str);
+
+ guestfs_push_error_handler (g, NULL, NULL);
+
+ str = guestfs_vfs_type (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("type", str);
+ free (str);
+
+ str = guestfs_vfs_label (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("label", str);
+ free (str);
+
+ str = guestfs_vfs_uuid (g, filesystems[i]);
+ if (str && str[0])
+ single_element ("uuid", str);
+ free (str);
+
+ guestfs_pop_error_handler (g);
+ } end_element ();
+ }
+ } end_element ();
}
static void
@@ -646,26 +618,20 @@ output_drive_mappings (xmlTextWriterPtr xo, char *root)
guestfs_int_count_strings (drive_mappings) / 2, 2 * sizeof (char *),
compare_keys_nocase);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"drive_mappings"));
-
- for (i = 0; drive_mappings[i] != NULL; i += 2) {
- str = guestfs_canonical_device_name (g, drive_mappings[i+1]);
- if (!str)
- exit (EXIT_FAILURE);
-
- XMLERROR (-1,
- xmlTextWriterStartElement (xo, BAD_CAST
"drive_mapping"));
- XMLERROR (-1,
- xmlTextWriterWriteAttribute (xo, BAD_CAST "name",
- BAD_CAST drive_mappings[i]));
- XMLERROR (-1,
- xmlTextWriterWriteString (xo, BAD_CAST str));
- XMLERROR (-1, xmlTextWriterEndElement (xo));
-
- free (str);
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ start_element ("drive_mappings") {
+ for (i = 0; drive_mappings[i] != NULL; i += 2) {
+ str = guestfs_canonical_device_name (g, drive_mappings[i+1]);
+ if (!str)
+ exit (EXIT_FAILURE);
+
+ start_element ("drive_mapping") {
+ attribute ("name", drive_mappings[i]);
+ string (str);
+ } end_element ();
+
+ free (str);
+ }
+ } end_element ();
}
static void
@@ -681,71 +647,42 @@ output_applications (xmlTextWriterPtr xo, char *root)
if (apps == NULL)
exit (EXIT_FAILURE);
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"applications"));
+ start_element ("applications") {
+ for (i = 0; i < apps->len; ++i) {
+ start_element ("application") {
+ assert (apps->val[i].app2_name &&
apps->val[i].app2_name[0]);
+ single_element ("name", apps->val[i].app2_name);
- for (i = 0; i < apps->len; ++i) {
- XMLERROR (-1, xmlTextWriterStartElement (xo, BAD_CAST
"application"));
+ if (apps->val[i].app2_display_name &&
+ apps->val[i].app2_display_name[0])
+ single_element ("display_name",
apps->val[i].app2_display_name);
- assert (apps->val[i].app2_name && apps->val[i].app2_name[0]);
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "name",
- BAD_CAST apps->val[i].app2_name));
+ if (apps->val[i].app2_epoch != 0)
+ single_element_format ("epoch", "%d",
apps->val[i].app2_epoch);
- if (apps->val[i].app2_display_name &&
apps->val[i].app2_display_name[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "display_name",
- BAD_CAST apps->val[i].app2_display_name));
-
- if (apps->val[i].app2_epoch != 0) {
- char buf[32];
-
- snprintf (buf, sizeof buf, "%d", apps->val[i].app2_epoch);
-
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "epoch", BAD_CAST buf));
+ if (apps->val[i].app2_version &&
apps->val[i].app2_version[0])
+ single_element ("version", apps->val[i].app2_version);
+ if (apps->val[i].app2_release &&
apps->val[i].app2_release[0])
+ single_element ("release", apps->val[i].app2_release);
+ if (apps->val[i].app2_arch && apps->val[i].app2_arch[0])
+ single_element ("arch", apps->val[i].app2_arch);
+ if (apps->val[i].app2_install_path &&
+ apps->val[i].app2_install_path[0])
+ single_element ("install_path",
apps->val[i].app2_install_path);
+ if (apps->val[i].app2_publisher &&
apps->val[i].app2_publisher[0])
+ single_element ("publisher",
apps->val[i].app2_publisher);
+ if (apps->val[i].app2_url && apps->val[i].app2_url[0])
+ single_element ("url", apps->val[i].app2_url);
+ if (apps->val[i].app2_source_package &&
+ apps->val[i].app2_source_package[0])
+ single_element ("source_package",
apps->val[i].app2_source_package);
+ if (apps->val[i].app2_summary &&
apps->val[i].app2_summary[0])
+ single_element ("summary", apps->val[i].app2_summary);
+ if (apps->val[i].app2_description &&
apps->val[i].app2_description[0])
+ single_element ("description",
apps->val[i].app2_description);
+ } end_element ();
}
-
- if (apps->val[i].app2_version &&
apps->val[i].app2_version[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "version",
- BAD_CAST apps->val[i].app2_version));
- if (apps->val[i].app2_release &&
apps->val[i].app2_release[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "release",
- BAD_CAST apps->val[i].app2_release));
- if (apps->val[i].app2_arch && apps->val[i].app2_arch[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "arch",
- BAD_CAST apps->val[i].app2_arch));
- if (apps->val[i].app2_install_path &&
apps->val[i].app2_install_path[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "install_path",
- BAD_CAST apps->val[i].app2_install_path));
- if (apps->val[i].app2_publisher &&
apps->val[i].app2_publisher[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "publisher",
- BAD_CAST apps->val[i].app2_publisher));
- if (apps->val[i].app2_url && apps->val[i].app2_url[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "url",
- BAD_CAST apps->val[i].app2_url));
- if (apps->val[i].app2_source_package &&
apps->val[i].app2_source_package[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "source_package",
- BAD_CAST apps->val[i].app2_source_package));
- if (apps->val[i].app2_summary &&
apps->val[i].app2_summary[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "summary",
- BAD_CAST apps->val[i].app2_summary));
- if (apps->val[i].app2_description &&
apps->val[i].app2_description[0])
- XMLERROR (-1,
- xmlTextWriterWriteElement (xo, BAD_CAST "description",
- BAD_CAST apps->val[i].app2_description));
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
- }
-
- XMLERROR (-1, xmlTextWriterEndElement (xo));
+ } end_element ();
}
/* Run an XPath query on XML on stdin, print results to stdout. */
--
2.19.0.rc0
Richard W.M. Jones
2018-Nov-02 15:05 UTC
[Libguestfs] [PATCH v3 4/4] lib, p2v: Use single_element() macro where possible.
After the previous commit, wherever we had:
start_element ("foo") {
string ("bar");
} end_element ();
this can now be replaced by:
single_element ("foo", "bar");
---
common/utils/libxml2-writer-macros.h | 2 +-
lib/launch-libvirt.c | 78 ++++++++--------------------
p2v/physical-xml.c | 15 ++----
3 files changed, 26 insertions(+), 69 deletions(-)
diff --git a/common/utils/libxml2-writer-macros.h
b/common/utils/libxml2-writer-macros.h
index d74dcd465..a99b245bb 100644
--- a/common/utils/libxml2-writer-macros.h
+++ b/common/utils/libxml2-writer-macros.h
@@ -86,7 +86,7 @@
*
* which produces C<<< <name>text</name> >>>
*/
-#define single_element(element, str) \
+#define single_element(element,str) \
do { \
start_element ((element)) { \
string ((str)); \
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index f80932001..bf6b01c49 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -1053,10 +1053,7 @@ construct_libvirt_xml_name (guestfs_h *g,
const struct libvirt_xml_params *params,
xmlTextWriterPtr xo)
{
- start_element ("name") {
- string (params->data->name);
- } end_element ();
-
+ single_element ("name", params->data->name);
return 0;
}
@@ -1087,17 +1084,12 @@ construct_libvirt_xml_cpu (guestfs_h *g,
attribute ("fallback", "allow");
} end_element ();
}
- else {
- start_element ("model") {
- string (cpu_model);
- } end_element ();
- }
+ else
+ single_element ("model", cpu_model);
} end_element ();
}
- start_element ("vcpu") {
- string_format ("%d", g->smp);
- } end_element ();
+ single_element_format ("vcpu", "%d", g->smp);
start_element ("clock") {
attribute ("offset", "utc");
@@ -1158,24 +1150,13 @@ construct_libvirt_xml_boot (guestfs_h *g,
string (params->data->uefi_code);
} end_element ();
- if (params->data->uefi_vars) {
- start_element ("nvram") {
- string (params->data->uefi_vars);
- } end_element ();
- }
+ if (params->data->uefi_vars)
+ single_element ("nvram", params->data->uefi_vars);
}
- start_element ("kernel") {
- string (params->kernel);
- } end_element ();
-
- start_element ("initrd") {
- string (params->initrd);
- } end_element ();
-
- start_element ("cmdline") {
- string (cmdline);
- } end_element ();
+ single_element ("kernel", params->kernel);
+ single_element ("initrd", params->initrd);
+ single_element ("cmdline", cmdline);
#if defined(__i386__) || defined(__x86_64__)
if (g->verbose) {
@@ -1210,12 +1191,8 @@ construct_libvirt_xml_seclabel (guestfs_h *g,
attribute ("type", "static");
attribute ("model", "selinux");
attribute ("relabel", "yes");
- start_element ("label") {
- string (params->data->selinux_label);
- } end_element ();
- start_element ("imagelabel") {
- string (params->data->selinux_imagelabel);
- } end_element ();
+ single_element ("label", params->data->selinux_label);
+ single_element ("imagelabel",
params->data->selinux_imagelabel);
} end_element ();
}
@@ -1228,10 +1205,7 @@ construct_libvirt_xml_lifecycle (guestfs_h *g,
const struct libvirt_xml_params *params,
xmlTextWriterPtr xo)
{
- start_element ("on_reboot") {
- string ("destroy");
- } end_element ();
-
+ single_element ("on_reboot", "destroy");
return 0;
}
@@ -1249,20 +1223,14 @@ construct_libvirt_xml_devices (guestfs_h *g,
/* Path to hypervisor. Only write this if the user has changed the
* default, otherwise allow libvirt to choose the best one.
*/
- if (is_custom_hv (g)) {
- start_element ("emulator") {
- string (g->hv);
- } end_element ();
- }
+ if (is_custom_hv (g))
+ single_element ("emulator", g->hv);
#if defined(__arm__)
/* Hopefully temporary hack to make ARM work (otherwise libvirt
* chooses to run /usr/bin/qemu-kvm).
*/
- else {
- start_element ("emulator") {
- string (QEMU);
- } end_element ();
- }
+ else
+ single_element ("emulator", QEMU);
#endif
/* Add a random number generator (backend for virtio-rng). This
@@ -1533,11 +1501,8 @@ construct_libvirt_xml_disk (guestfs_h *g,
return -1;
}
- if (drv->disk_label) {
- start_element ("serial") {
- string (drv->disk_label);
- } end_element ();
- }
+ if (drv->disk_label)
+ single_element ("serial", drv->disk_label);
if (construct_libvirt_xml_disk_address (g, xo, drv_index) == -1)
return -1;
@@ -1786,10 +1751,9 @@ construct_libvirt_xml_secret (guestfs_h *g,
start_element ("secret") {
attribute ("ephemeral", "yes");
attribute ("private", "yes");
- start_element ("description") {
- string_format ("guestfs secret associated with %s %s",
- data->name, drv->src.u.path);
- } end_element ();
+ single_element_format ("description",
+ "guestfs secret associated with %s %s",
+ data->name, drv->src.u.path);
} end_element ();
return 0;
diff --git a/p2v/physical-xml.c b/p2v/physical-xml.c
index b2169fc83..7d80ab9c5 100644
--- a/p2v/physical-xml.c
+++ b/p2v/physical-xml.c
@@ -97,9 +97,7 @@ generate_physical_xml (struct config *config, struct data_conn
*data_conns,
start_element ("domain") {
attribute ("type", "physical");
- start_element ("name") {
- string (config->guestname);
- } end_element ();
+ single_element ("name", config->guestname);
start_element ("memory") {
attribute ("unit", "KiB");
@@ -111,20 +109,15 @@ generate_physical_xml (struct config *config, struct
data_conn *data_conns,
string_format ("%" PRIu64, memkb);
} end_element ();
- start_element ("vcpu") {
- string_format ("%d", config->vcpus);
- } end_element ();
+ single_element_format ("vcpu", "%d", config->vcpus);
if (config->cpu.vendor || config->cpu.model ||
config->cpu.sockets || config->cpu.cores ||
config->cpu.threads) {
/* https://libvirt.org/formatdomain.html#elementsCPU */
start_element ("cpu") {
attribute ("match", "minimum");
- if (config->cpu.vendor) {
- start_element ("vendor") {
- string (config->cpu.vendor);
- } end_element ();
- }
+ if (config->cpu.vendor)
+ single_element ("vendor", config->cpu.vendor);
if (config->cpu.model) {
start_element ("model") {
attribute ("fallback", "allow");
--
2.19.0.rc0
Richard W.M. Jones
2018-Nov-02 15:42 UTC
Re: [Libguestfs] [PATCH v3 2/4] common/utils: Move libxml2 writer macros to a common header file.
On Fri, Nov 02, 2018 at 03:05:02PM +0000, Richard W.M. Jones wrote:> The last point causes the most churn since we must change the callers > to avoid format string security bugs.This comment was left in error and I've removed it in my copy. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top
Pino Toscano
2018-Nov-02 16:15 UTC
Re: [Libguestfs] [PATCH v3 4/4] lib, p2v: Use single_element() macro where possible.
On Friday, 2 November 2018 16:05:04 CET Richard W.M. Jones wrote:> diff --git a/common/utils/libxml2-writer-macros.h b/common/utils/libxml2-writer-macros.h > index d74dcd465..a99b245bb 100644 > --- a/common/utils/libxml2-writer-macros.h > +++ b/common/utils/libxml2-writer-macros.h > @@ -86,7 +86,7 @@ > * > * which produces C<<< <name>text</name> >>> > */ > -#define single_element(element, str) \ > +#define single_element(element,str) \ > do { \ > start_element ((element)) { \ > string ((str)); \This fits better as squashed in patch #3.> - if (params->data->uefi_vars) { > - start_element ("nvram") { > - string (params->data->uefi_vars); > - } end_element (); > - } > + if (params->data->uefi_vars) > + single_element ("nvram", params->data->uefi_vars);Hm indented with tab? -- Pino Toscano
Pino Toscano
2018-Nov-02 16:36 UTC
Re: [Libguestfs] [PATCH v3 0/4] common/utils: Move libxml2 writer macros to a common header file.
On Friday, 2 November 2018 16:05:00 CET Richard W.M. Jones wrote:> v1 was here: > https://www.redhat.com/archives/libguestfs/2018-October/msg00047.html > v2 was here: > https://www.redhat.com/archives/libguestfs/2018-October/msg00051.html > > v3: > > - Back to using string/string_format and attribute/attribute_format. > > - Add both single_element and single_element_format. > > - Rebased and retested.Except from your own note to patch #2, and my notes for patch #4, the series LGTM. -- Pino Toscano
Reasonably Related Threads
- [PATCH v2 2/4] common/utils: Move libxml2 writer macros to a common header file.
- Re: [PATCH v3 4/4] lib, p2v: Use single_element() macro where possible.
- [PATCH v3 0/4] common/utils: Move libxml2 writer macros to a common header file.
- [PATCH v2 0/4] common/utils: Move libxml2 writer macros to a common header file.
- [PATCH 0/2] Use common macros to help with libxml2 writer.