[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 06/18] qapi: Add qstring_append_format()
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v3 06/18] qapi: Add qstring_append_format() |
Date: |
Fri, 29 Apr 2016 15:40:45 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Back in commit 764c1ca (Nov 2009), we added qstring_append_int().
> However, it did not see any use until commit 190c882 (Jan 2015).
> Furthermore, it has a rather limited use case - to print anything
> else, callers still have to format into a temporary buffer, unless
> we want to introduce an explosion of new qstring_append_* methods
> for each useful type to print.
>
> A much better approach is to add a wrapper that merges printf
> behavior onto qstring_append, via the new qstring_append_format()
> (and its vararg counterpart). In fact, with that in place, we
> no longer need qstring_append_int().
>
> Other immediate uses for the new function include simplifying
> two existing clients of qstring_append() on a just-formatted
> buffer, and the fact that we can take advantage of printf width
> manipulations for more efficient indentation.
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Fam Zheng <address@hidden>
>
> ---
> v3: rebase to master
> v2: also simplify qstring_append_json_string(), add assertion that
> format is well-formed
> ---
> include/qapi/qmp/qstring.h | 7 +++++--
> qjson.c | 2 +-
> qobject/qobject-json.c | 25 +++++--------------------
> qobject/qstring.c | 37 +++++++++++++++++++++++++------------
> 4 files changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h
> index f00e3df..10afa5d 100644
> --- a/include/qapi/qmp/qstring.h
> +++ b/include/qapi/qmp/qstring.h
> @@ -1,7 +1,7 @@
> /*
> * QString Module
> *
> - * Copyright (C) 2009, 2015 Red Hat Inc.
> + * Copyright (C) 2009-2016 Red Hat Inc.
> *
> * Authors:
> * Luiz Capitulino <address@hidden>
> @@ -28,9 +28,12 @@ QString *qstring_from_str(const char *str);
> QString *qstring_from_substr(const char *str, int start, int end);
> size_t qstring_get_length(const QString *qstring);
> const char *qstring_get_str(const QString *qstring);
> -void qstring_append_int(QString *qstring, int64_t value);
> void qstring_append(QString *qstring, const char *str);
> void qstring_append_chr(QString *qstring, int c);
> +void qstring_append_format(QString *qstring, const char *fmt, ...)
> + GCC_FMT_ATTR(2, 3);
> +void qstring_append_vformat(QString *qstring, const char *fmt, va_list ap)
> + GCC_FMT_ATTR(2, 0);
Let's call these qstring_append_printf() and qstring_append_vprintf(),
to match GLib's g_string_append_printf() and g_string_append_vprintf().
> void qstring_append_json_string(QString *qstring, const char *raw);
> void qstring_append_json_number(QString *qstring, double number, Error
> **errp);
> QString *qobject_to_qstring(const QObject *obj);
> diff --git a/qjson.c b/qjson.c
> index b9a9a36..d172b1f 100644
> --- a/qjson.c
> +++ b/qjson.c
> @@ -70,7 +70,7 @@ void json_end_array(QJSON *json)
> void json_prop_int(QJSON *json, const char *name, int64_t val)
> {
> json_emit_element(json, name);
> - qstring_append_int(json->str, val);
> + qstring_append_format(json->str, "%" PRId64, val);
> }
>
> void json_prop_str(QJSON *json, const char *name, const char *str)
> diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c
> index 97bccb7..963de07 100644
> --- a/qobject/qobject-json.c
> +++ b/qobject/qobject-json.c
> @@ -80,16 +80,13 @@ static void to_json(const QObject *obj, QString *str, int
> pretty, int indent);
> static void to_json_dict_iter(const char *key, QObject *obj, void *opaque)
> {
> ToJsonIterState *s = opaque;
> - int j;
>
> if (s->count) {
> qstring_append(s->str, s->pretty ? "," : ", ");
> }
>
> if (s->pretty) {
> - qstring_append(s->str, "\n");
> - for (j = 0 ; j < s->indent ; j++)
> - qstring_append(s->str, " ");
> + qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
> }
>
> qstring_append_json_string(s->str, key);
> @@ -102,16 +99,13 @@ static void to_json_dict_iter(const char *key, QObject
> *obj, void *opaque)
> static void to_json_list_iter(QObject *obj, void *opaque)
> {
> ToJsonIterState *s = opaque;
> - int j;
>
> if (s->count) {
> qstring_append(s->str, s->pretty ? "," : ", ");
> }
>
> if (s->pretty) {
> - qstring_append(s->str, "\n");
> - for (j = 0 ; j < s->indent ; j++)
> - qstring_append(s->str, " ");
> + qstring_append_format(s->str, "\n%*s", 4 * s->indent, "");
> }
>
> to_json(obj, s->str, s->pretty, s->indent);
> @@ -126,10 +120,7 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> break;
> case QTYPE_QINT: {
> QInt *val = qobject_to_qint(obj);
> - char buffer[1024];
> -
> - snprintf(buffer, sizeof(buffer), "%" PRId64, qint_get_int(val));
> - qstring_append(str, buffer);
> + qstring_append_format(str, "%" PRId64, qint_get_int(val));
> break;
> }
> case QTYPE_QSTRING: {
> @@ -148,10 +139,7 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> qstring_append(str, "{");
> qdict_iter(val, to_json_dict_iter, &s);
> if (pretty) {
> - int j;
> - qstring_append(str, "\n");
> - for (j = 0 ; j < indent ; j++)
> - qstring_append(str, " ");
> + qstring_append_format(str, "\n%*s", 4 * indent, "");
> }
> qstring_append(str, "}");
> break;
> @@ -167,10 +155,7 @@ static void to_json(const QObject *obj, QString *str,
> int pretty, int indent)
> qstring_append(str, "[");
> qlist_iter(val, (void *)to_json_list_iter, &s);
> if (pretty) {
> - int j;
> - qstring_append(str, "\n");
> - for (j = 0 ; j < indent ; j++)
> - qstring_append(str, " ");
> + qstring_append_format(str, "\n%*s", 4 * indent, "");
> }
> qstring_append(str, "]");
> break;
> diff --git a/qobject/qstring.c b/qobject/qstring.c
> index 618dd8f..0285d53 100644
> --- a/qobject/qstring.c
> +++ b/qobject/qstring.c
> @@ -90,12 +90,28 @@ void qstring_append(QString *qstring, const char *str)
> qstring->string[qstring->length] = 0;
> }
>
> -void qstring_append_int(QString *qstring, int64_t value)
> +void qstring_append_format(QString *qstring, const char *fmt, ...)
> {
> - char num[32];
> + va_list ap;
>
> - snprintf(num, sizeof(num), "%" PRId64, value);
> - qstring_append(qstring, num);
> + va_start(ap, fmt);
> + qstring_append_vformat(qstring, fmt, ap);
> + va_end(ap);
> +}
> +
> +void qstring_append_vformat(QString *qstring, const char *fmt, va_list ap)
> +{
> + va_list ap2;
> + int len;
> +
> + va_copy(ap2, ap);
> + len = vsnprintf(qstring->string + qstring->length, 0, fmt, ap2);
Why bother with the first argument here? NULL would do :)
> + assert(len >= 0);
> + va_end(ap2);
> +
> + capacity_increase(qstring, len);
> + vsnprintf(qstring->string + qstring->length, len + 1, fmt, ap);
> + qstring->length += len;
> }
>
> /**
> @@ -116,7 +132,6 @@ void qstring_append_json_string(QString *qstring, const
> char *raw)
> {
> const char *ptr = raw;
> int cp;
> - char buf[16];
> char *end;
>
> qstring_append(qstring, "\"");
> @@ -151,16 +166,14 @@ void qstring_append_json_string(QString *qstring, const
> char *raw)
> }
> if (cp > 0xFFFF) {
> /* beyond BMP; need a surrogate pair */
> - snprintf(buf, sizeof(buf), "\\u%04X\\u%04X",
> - 0xD800 + ((cp - 0x10000) >> 10),
> - 0xDC00 + ((cp - 0x10000) & 0x3FF));
> + qstring_append_format(qstring, "\\u%04X\\u%04X",
> + 0xD800 + ((cp - 0x10000) >> 10),
> + 0xDC00 + ((cp - 0x10000) & 0x3FF));
> } else if (cp < 0x20 || cp >= 0x7F) {
> - snprintf(buf, sizeof(buf), "\\u%04X", cp);
> + qstring_append_format(qstring, "\\u%04X", cp);
> } else {
> - buf[0] = cp;
> - buf[1] = 0;
> + qstring_append_chr(qstring, cp);
> }
> - qstring_append(qstring, buf);
> }
> };
- [Qemu-devel] [PATCH v3 00/18] Add qapi-to-JSON and clone visitors, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 05/18] qapi: Use qstring_append_chr() where appropriate, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 03/18] qapi: Factor out JSON string escaping, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 09/18] Revert "qjson: Simplify by using json-output-visitor", Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 08/18] qjson: Simplify by using json-output-visitor, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 02/18] qapi: Improve use of qmp/types.h, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 06/18] qapi: Add qstring_append_format(), Eric Blake, 2016/04/29
- Re: [Qemu-devel] [PATCH v3 06/18] qapi: Add qstring_append_format(),
Markus Armbruster <=
- [Qemu-devel] [PATCH v3 01/18] qapi: Rename (one) qjson.h to qobject-json.h, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 11/18] qjson: Remove unused file, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 16/18] sockets: Use new QAPI cloning, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 04/18] qapi: Factor out JSON number formatting, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 07/18] qapi: Add json output visitor, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 10/18] vmstate: Use new JSON output visitor, Eric Blake, 2016/04/29
- [Qemu-devel] [PATCH v3 15/18] qapi: Add new clone visitor, Eric Blake, 2016/04/29