[libvirt] [PATCHv2 04/13] virbuf: add auto-indentation support
Hai Dong Li
haidongl at linux.vnet.ibm.com
Tue Oct 18 10:11:32 UTC 2011
On 09/30/2011 12:22 AM, 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(-)
>
> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index c2a3fab..695ff33 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -22,12 +22,14 @@ virBitmapString;
> # buf.h
> virBufferAdd;
> virBufferAddChar;
> +virBufferAdjustIndent;
> virBufferAsprintf;
> virBufferContentAndReset;
> virBufferError;
> virBufferEscapeSexpr;
> virBufferEscapeString;
> virBufferFreeAndReset;
> +virBufferGetIndent;
> virBufferStrcat;
> virBufferURIEncodeString;
> virBufferUse;
> diff --git a/src/util/buf.c b/src/util/buf.c
> index 98bb681..b409de4 100644
> --- a/src/util/buf.c
> +++ b/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,10 +45,54 @@ 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 if you append a
> + * string with an embedded newline). 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
The priority of operators is all right here, yet maybe parentheses
around the expression (INT_MAX - indent) will make this more obviously?.
Just saying, never mind.
> + : 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
> * @len: the minimum free size to allocate on top of existing used space
> @@ -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;
>
> - 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);
> }
>
> /**
> @@ -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);
> +
> if (buf->size == 0&&
> virBufferGrow(buf, 100)< 0)
> return;
> @@ -287,10 +325,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 +412,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,
> @@ -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);
> +
> for (p = str; *p; ++p) {
> if (c_isalnum(*p))
> grow_size++;
> @@ -454,7 +498,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) {
> @@ -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);
> }
> diff --git a/src/util/buf.h b/src/util/buf.h
> index 42a5044..08cb727 100644
> --- a/src/util/buf.h
> +++ b/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
>
> @@ -55,6 +56,9 @@ void virBufferEscapeSexpr(virBufferPtr buf, const char *format,
> void virBufferURIEncodeString(virBufferPtr buf, const char *str);
>
> # define virBufferAddLit(buf_, literal_string_) \
> - virBufferAdd (buf_, "" literal_string_ "", sizeof literal_string_ - 1)
> + 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__ */
> diff --git a/tests/virbuftest.c b/tests/virbuftest.c
> index 01db313..2f94e1c 100644
> --- a/tests/virbuftest.c
> +++ b/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,77 @@ 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&\n 8\n 9\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;
> + }
So now buf->indent is 1. Go to the next step, the indent is given -2
again, see what will happen.
if virBufferAdjustIndent failed to check the indent overflow, the
buf->indent will be -1,too, so it may avoid the check
(virBufferGetIndent(buf, false) != -1) and (virBufferGetIndent(buf,
true) != -1).
> + virBufferAdjustIndent(buf, -2);
So I think -3 may be better.
> + 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');
> +
> + 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 +144,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);
> }
More information about the libvir-list
mailing list