Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com> > Reviewed-by: Fam Zheng <f...@redhat.com> > > --- > 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 <lcapitul...@redhat.com> > @@ -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); > } > };