virbuf: add auto-indentation support

Rather than having to adjust all callers in a chain to deal with
indentation, it is nicer to have virBuffer do auto-indentation.

* src/util/buf.h (_virBuffer): Increase size.
(virBufferAdjustIndent, virBufferGetIndent): New prototypes.
* src/libvirt_private.syms (buf.h): Export new functions.
* src/util/buf.c (virBufferAdjustIndent, virBufferGetIndent): New
functions.
(virBufferSetError, virBufferAdd, virBufferAddChar)
(virBufferVasprintf, virBufferStrcat, virBufferURIEncodeString):
Implement auto-indentation.
* tests/virbuftest.c (testBufAutoIndent): Test it.
(testBufInfiniteLoop): Don't rely on internals.
Idea by Daniel P. Berrange.

src/libvirt_private.syms

@@ -22,6 +22,7 @@ virBitmapString;
 # buf.h
 virBufferAdd;
 virBufferAddChar;
+virBufferAdjustIndent;
 virBufferAsprintf;
 virBufferContentAndReset;
 virBufferError;
@@ -30,6 +31,7 @@ virBufferEscapeSexpr;
 virBufferEscapeShell;
 virBufferEscapeString;
 virBufferFreeAndReset;
+virBufferGetIndent;
 virBufferStrcat;
 virBufferURIEncodeString;
 virBufferUse;

src/util/buf.c

@@ -28,6 +28,7 @@ struct _virBuffer {
     unsigned int size;
     unsigned int use;
     unsigned int error; /* errno value, or -1 for usage error */
+    int indent;
     char *content;
 };
 
@@ -44,9 +45,53 @@ virBufferSetError(virBufferPtr buf, int error)
     VIR_FREE(buf->content);
     buf->size = 0;
     buf->use = 0;
+    buf->indent = 0;
     buf->error = error;
 }
 
+/**
+ * virBufferAdjustIndent:
+ * @buf: the buffer
+ * @indent: adjustment to make
+ *
+ * Alter the auto-indent value by adding indent (positive to increase,
+ * negative to decrease).  Automatic indentation is performed by all
+ * additive functions when the existing buffer is empty or ends with a
+ * newline (however, note that no indentation is added after newlines
+ * embedded in an appended string).  If @indent would cause overflow,
+ * the buffer error indicator is set.
+ */
+void
+virBufferAdjustIndent(virBufferPtr buf, int indent)
+{
+    if (!buf || buf->error)
+        return;
+    if (indent > 0 ? INT_MAX - indent < buf->indent
+        : buf->indent < -indent) {
+        virBufferSetError(buf, -1);
+        return;
+    }
+    buf->indent += indent;
+}
+
+/**
+ * virBufferGetIndent:
+ * @buf: the buffer
+ * @dynamic: if false, return set value; if true, return 0 unless next
+ * append would be affected by auto-indent
+ *
+ * Return the current auto-indent value, or -1 if there has been an error.
+ */
+int
+virBufferGetIndent(const virBufferPtr buf, bool dynamic)
+{
+    if (!buf || buf->error)
+        return -1;
+    if (dynamic && buf->use && buf->content[buf->use - 1] != '\n')
+        return 0;
+    return buf->indent;
+}
+
 /**
  * virBufferGrow:
  * @buf:  the buffer
@@ -79,35 +124,39 @@ virBufferGrow(virBufferPtr buf, unsigned int len)
 
 /**
  * virBufferAdd:
- * @buf:  the buffer to add to
- * @str:  the string
- * @len:  the number of bytes to add
+ * @buf: the buffer to append to
+ * @str: the string
+ * @len: the number of bytes to add, or -1
  *
- * Add a string range to an XML buffer. if len == -1, the length of
- * str is recomputed to the full string.
+ * Add a string range to an XML buffer. If @len == -1, the length of
+ * str is recomputed to the full string.  Auto indentation may be applied.
  *
  */
 void
 virBufferAdd(virBufferPtr buf, const char *str, int len)
 {
     unsigned int needSize;
+    int indent;
 
-    if ((str == NULL) || (buf == NULL) || (len == 0))
+    if (!str || !buf || (len == 0 && buf->indent == 0))
         return;
 
     if (buf->error)
         return;
 
+    indent = virBufferGetIndent(buf, true);
+
     if (len < 0)
         len = strlen(str);
 
-    needSize = buf->use + len + 2;
+    needSize = buf->use + indent + len + 2;
     if (needSize > buf->size &&
         virBufferGrow(buf, needSize - buf->use) < 0)
         return;
 
-    memcpy (&buf->content[buf->use], str, len);
-    buf->use += len;
+    memset(&buf->content[buf->use], ' ', indent);
+    memcpy(&buf->content[buf->use + indent], str, len);
+    buf->use += indent + len;
     buf->content[buf->use] = '\0';
 }
 
@@ -116,27 +165,13 @@ virBufferAdd(virBufferPtr buf, const char *str, int len)
  * @buf: the buffer to append to
  * @c: the character to add
  *
- * Add a single character 'c' to a buffer.
+ * Add a single character 'c' to a buffer.  Auto indentation may be applied.
  *
  */
 void
-virBufferAddChar (virBufferPtr buf, char c)
+virBufferAddChar(virBufferPtr buf, char c)
 {
-    unsigned int needSize;
-
-    if (buf == NULL)
-        return;
-
-    if (buf->error)
-        return;
-
-    needSize = buf->use + 2;
-    if (needSize > buf->size &&
-        virBufferGrow (buf, needSize - buf->use) < 0)
-        return;
-
-    buf->content[buf->use++] = c;
-    buf->content[buf->use] = '\0';
+    virBufferAdd(buf, &c, 1);
 }
 
 /**
@@ -218,7 +253,7 @@ virBufferUse(const virBufferPtr buf)
  * @format:  the format
  * @...:  the variable list of arguments
  *
- * Do a formatted print to an XML buffer.
+ * Do a formatted print to an XML buffer.  Auto indentation may be applied.
  */
 void
 virBufferAsprintf(virBufferPtr buf, const char *format, ...)
@@ -231,11 +266,11 @@ virBufferAsprintf(virBufferPtr buf, const char *format, ...)
 
 /**
  * virBufferVasprintf:
- * @buf:  the buffer to dump
+ * @buf: the buffer to append to
  * @format:  the format
  * @argptr:  the variable list of arguments
  *
- * Do a formatted print to an XML buffer.
+ * Do a formatted print to an XML buffer.  Auto indentation may be applied.
  */
 void
 virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
@@ -249,6 +284,8 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
     if (buf->error)
         return;
 
+    virBufferAddLit(buf, ""); /* auto-indent */
+
     if (buf->size == 0 &&
         virBufferGrow(buf, 100) < 0)
         return;
@@ -287,10 +324,12 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
  * virBufferEscapeString:
  * @buf: the buffer to append to
  * @format: a printf like format string but with only one %s parameter
- * @str:  the string argument which need to be escaped
+ * @str: the string argument which needs to be escaped
  *
- * Do a formatted print with a single string to an XML buffer. The string
- * is escaped to avoid generating a not well-formed XML instance.
+ * Do a formatted print with a single string to an XML buffer. The
+ * string is escaped for use in XML.  If @str is NULL, nothing is
+ * added (not even the rest of @format).  Auto indentation may be
+ * applied.
  */
 void
 virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)
@@ -372,11 +411,12 @@ virBufferEscapeString(virBufferPtr buf, const char *format, const char *str)
  * virBufferEscapeSexpr:
  * @buf: the buffer to append to
  * @format: a printf like format string but with only one %s parameter
- * @str:  the string argument which need to be escaped
+ * @str: the string argument which needs to be escaped
  *
- * Do a formatted print with a single string to an sexpr buffer. The string
- * is escaped to avoid generating a sexpr that xen will choke on. This
- * doesn't fully escape the sexpr, just enough for our code to work.
+ * Do a formatted print with a single string to an sexpr buffer. The
+ * string is escaped to avoid generating a sexpr that xen will choke
+ * on. This doesn't fully escape the sexpr, just enough for our code
+ * to work.  Auto indentation may be applied.
  */
 void
 virBufferEscapeSexpr(virBufferPtr buf,
@@ -394,7 +434,8 @@ virBufferEscapeSexpr(virBufferPtr buf,
  * @str: the string argument which needs to be escaped
  *
  * Do a formatted print with a single string to a buffer.  Any characters
- * in the provided list are escaped with a preceeding \.
+ * in the provided list are escaped with a preceeding \.  Auto indentation
+ * may be applied.
  */
 void
 virBufferEscape(virBufferPtr buf, const char *toescape,
@@ -443,7 +484,7 @@ virBufferEscape(virBufferPtr buf, const char *toescape,
  *
  * Append the string to the buffer.  The string will be URI-encoded
  * during the append (ie any non alpha-numeric characters are replaced
- * with '%xx' hex sequences).
+ * with '%xx' hex sequences).  Auto indentation may be applied.
  */
 void
 virBufferURIEncodeString(virBufferPtr buf, const char *str)
@@ -459,6 +500,8 @@ virBufferURIEncodeString(virBufferPtr buf, const char *str)
     if (buf->error)
         return;
 
+    virBufferAddLit(buf, ""); /* auto-indent */
+
     for (p = str; *p; ++p) {
         if (c_isalnum(*p))
             grow_size++;
@@ -466,7 +509,7 @@ virBufferURIEncodeString(virBufferPtr buf, const char *str)
             grow_size += 3; /* %ab */
     }
 
-    if (virBufferGrow (buf, grow_size) < 0)
+    if (virBufferGrow(buf, grow_size) < 0)
         return;
 
     for (p = str; *p; ++p) {
@@ -485,11 +528,11 @@ virBufferURIEncodeString(virBufferPtr buf, const char *str)
 
 /**
  * virBufferEscapeShell:
- * @buf:  the buffer to append to
- * @str:  an unquoted string
+ * @buf: the buffer to append to
+ * @str: an unquoted string
  *
  * Quotes a string so that the shell (/bin/sh) will interpret the
- * quoted string to mean str.
+ * quoted string to mean str.  Auto indentation may be applied.
  */
 void
 virBufferEscapeShell(virBufferPtr buf, const char *str)
@@ -547,7 +590,8 @@ virBufferEscapeShell(virBufferPtr buf, const char *str)
  * @buf: the buffer to append to
  * @...:  the variable list of strings, the last argument must be NULL
  *
- * Concatenate strings to an XML buffer.
+ * Concatenate strings to an XML buffer.  Auto indentation may be applied
+ * after each string argument.
  */
 void
 virBufferStrcat(virBufferPtr buf, ...)
@@ -559,18 +603,7 @@ virBufferStrcat(virBufferPtr buf, ...)
         return;
 
     va_start(ap, buf);
-
-    while ((str = va_arg(ap, char *)) != NULL) {
-        unsigned int len = strlen(str);
-        unsigned int needSize = buf->use + len + 2;
-
-        if (needSize > buf->size) {
-            if (virBufferGrow(buf, needSize - buf->use) < 0)
-                break;
-        }
-        memcpy(&buf->content[buf->use], str, len);
-        buf->use += len;
-        buf->content[buf->use] = 0;
-    }
+    while ((str = va_arg(ap, char *)) != NULL)
+        virBufferAdd(buf, str, -1);
     va_end(ap);
 }

src/util/buf.h

@@ -24,15 +24,16 @@ typedef struct _virBuffer virBuffer;
 typedef virBuffer *virBufferPtr;
 
 # ifndef __VIR_BUFFER_C__
-#  define VIR_BUFFER_INITIALIZER { 0, 0, 0, NULL }
+#  define VIR_BUFFER_INITIALIZER { 0, 0, 0, 0, NULL }
 
-/* This struct must be kept in syn with the real struct
+/* This struct must be kept in sync with the real struct
    in the buf.c impl file */
 struct _virBuffer {
     unsigned int a;
     unsigned int b;
     unsigned int c;
-    char *d;
+    int d;
+    char *e;
 };
 # endif
 
@@ -60,4 +61,7 @@ void virBufferURIEncodeString(virBufferPtr buf, const char *str);
 # define virBufferAddLit(buf_, literal_string_) \
     virBufferAdd(buf_, "" literal_string_ "", sizeof literal_string_ - 1)
 
+void virBufferAdjustIndent(virBufferPtr buf, int indent);
+int virBufferGetIndent(const virBufferPtr buf, bool dynamic);
+
 #endif /* __VIR_BUFFER_H__ */

tests/virbuftest.c

@@ -20,7 +20,7 @@ struct testInfo {
     int doEscape;
 };
 
-static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED)
+static int testBufInfiniteLoop(const void *data)
 {
     virBuffer bufinit = VIR_BUFFER_INITIALIZER;
     virBufferPtr buf = &bufinit;
@@ -28,18 +28,13 @@ static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED)
     int ret = -1;
     const struct testInfo *info = data;
 
-    /* This relies of virBuffer internals, so may break if things change
-     * in the future */
     virBufferAddChar(buf, 'a');
-    if (buf->a != 1002 || buf->b != 1) {
-        TEST_ERROR("Buf was not expected size, size=%d use=%d\n",
-                   buf->a, buf->b);
-        goto out;
-    }
 
     /*
-     * Infinite loop triggers if:
+     * Infinite loop used to trigger if:
      * (strlen + 1 > 1000) && (strlen == buf-size - buf-use - 1)
+     * which was the case after the above addchar at the time of the bug.
+     * This test is a bit fragile, since it relies on virBuffer internals.
      */
     if (virAsprintf(&addstr, "%*s", buf->a - buf->b - 1, "a") < 0) {
         goto out;
@@ -63,6 +58,81 @@ out:
     return ret;
 }
 
+static int testBufAutoIndent(const void *data ATTRIBUTE_UNUSED)
+{
+    virBuffer bufinit = VIR_BUFFER_INITIALIZER;
+    virBufferPtr buf = &bufinit;
+    const char expected[] =
+        "  1\n  2\n  3\n  4\n  5\n  6\n  7\n  &amp;\n  8\n  9\n  10\n  ' 11'\n";
+    char *result = NULL;
+    int ret = 0;
+
+    if (virBufferGetIndent(buf, false) != 0 ||
+        virBufferGetIndent(buf, true) != 0) {
+        TEST_ERROR("Wrong indentation");
+        ret = -1;
+    }
+    virBufferAdjustIndent(buf, 3);
+    if (virBufferGetIndent(buf, false) != 3 ||
+        virBufferGetIndent(buf, true) != 3 ||
+        virBufferError(buf)) {
+        TEST_ERROR("Wrong indentation");
+        ret = -1;
+    }
+    virBufferAdjustIndent(buf, -2);
+    if (virBufferGetIndent(buf, false) != 1 ||
+        virBufferGetIndent(buf, true) != 1 ||
+        virBufferError(buf)) {
+        TEST_ERROR("Wrong indentation");
+        ret = -1;
+    }
+    virBufferAdjustIndent(buf, -3);
+    if (virBufferGetIndent(buf, false) != -1 ||
+        virBufferGetIndent(buf, true) != -1 ||
+        virBufferError(buf) != -1) {
+        TEST_ERROR("Usage error not flagged");
+        ret = -1;
+    }
+    virBufferFreeAndReset(buf);
+    if (virBufferGetIndent(buf, false) != 0 ||
+        virBufferGetIndent(buf, true) != 0 ||
+        virBufferError(buf)) {
+        TEST_ERROR("Reset didn't clear indentation");
+        ret = -1;
+    }
+    virBufferAdjustIndent(buf, 2);
+    virBufferAddLit(buf, "1");
+    if (virBufferGetIndent(buf, false) != 2 ||
+        virBufferGetIndent(buf, true) != 0) {
+        TEST_ERROR("Wrong indentation");
+        ret = -1;
+    }
+    virBufferAddLit(buf, "\n");
+    virBufferAdd(buf, "" "2\n", -1); /* Extra "" appeases syntax-check */
+    virBufferAddChar(buf, '3');
+    virBufferAddChar(buf, '\n');
+    virBufferAsprintf(buf, "%d", 4);
+    virBufferAsprintf(buf, "%c", '\n');
+    virBufferStrcat(buf, "5", "\n", "6\n", NULL);
+    virBufferEscapeString(buf, "%s\n", "7");
+    virBufferEscapeString(buf, "%s\n", "&");
+    virBufferEscapeSexpr(buf, "%s", "8\n");
+    virBufferURIEncodeString(buf, "9");
+    virBufferAddChar(buf, '\n');
+    virBufferEscapeShell(buf, "10");
+    virBufferAddChar(buf, '\n');
+    virBufferEscapeShell(buf, " 11");
+    virBufferAddChar(buf, '\n');
+
+    result = virBufferContentAndReset(buf);
+    if (!result || STRNEQ(result, expected)) {
+        virtTestDifference(stderr, expected, result);
+        ret = -1;
+    }
+    VIR_FREE(result);
+    return ret;
+}
+
 static int
 mymain(void)
 {
@@ -78,6 +148,7 @@ mymain(void)
 
     DO_TEST("EscapeString infinite loop", testBufInfiniteLoop, 1);
     DO_TEST("VSprintf infinite loop", testBufInfiniteLoop, 0);
+    DO_TEST("Auto-indentation", testBufAutoIndent, 0);
 
     return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
 }