[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCHv2 04/13] virbuf: add auto-indentation support



On 09/29/2011 06:22 PM, Eric Blake wrote:
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 |    2 +
  src/util/buf.c           |  138 ++++++++++++++++++++++++++++-----------------
  src/util/buf.h           |   12 +++-
  tests/virbuftest.c       |   85 +++++++++++++++++++++++++---
  4 files changed, 172 insertions(+), 65 deletions(-)

@@ -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(const virBufferPtr buf, const char *str, int len)
  {
      unsigned int needSize;
+    int indent;
/* context for a later comment */
-    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(const 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);
  }
This has slightly more overhead than the previous version, but I don't think adding a char is such a common operation that it would impact performance. This also simplifies mantenance. I like it.
  /**
@@ -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,9 @@ virBufferVasprintf(virBufferPtr buf, const char *format, va_list argptr)
      if (buf->error)
          return;

+    if (buf->indent)
+        virBufferAdd(buf, "", -1);
+
virBufferAdd is a no-op, if this expression is true (see context above): " if (!str || !buf || (len == 0 && buf->indent == 0))". You could save a call to strlen and the if clause if you call virBufferAdd(buf, "", 0) The len=0 will be automaticaly used (as the only way to accept len==0 as a correct value is if indentation is requested). You can also save a line with the if clause and call it directly. ( Looks like you've prepared it for this to happen :D)
      if (buf->size == 0&&
          virBufferGrow(buf, 100)<  0)
          return;
@@ -431,7 +472,7 @@ virBufferEscapeSexpr(virBufferPtr buf,
   *
   * 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)
@@ -447,6 +488,9 @@ virBufferURIEncodeString(virBufferPtr buf, const char *str)
      if (buf->error)
          return;

+    if (buf->indent)
+        virBufferAdd(buf, "", -1);
Same as above ...
@@ -476,7 +520,8 @@ virBufferURIEncodeString(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, ...)
@@ -488,18 +533,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);
  }
Elegant.
diff --git a/tests/virbuftest.c b/tests/virbuftest.c

The tests look fine to me and it's nice to have them.

ACK,

Peter


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]