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
/** @@ -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
On 09/29/2011 06:22 PM, Eric Blake wrote: think adding a char is such a common operation that it would impact performance. This also simplifies mantenance. I like it. 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