| Message ID | 1461903820-3092-4-git-send-email-eblake@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Eric Blake <eblake@redhat.com> writes: > Pull out a new qstring_append_json_string() helper, so that all > JSON output producers can use the same output escaping rules. > > While it appears that vmstate's use of the simpler qjson.c > formatter is not currently encountering any string that needs > escapes to be valid JSON, it is better to be safe than sorry. > > Signed-off-by: Eric Blake <eblake@redhat.com> > Reviewed-by: Fam Zheng <famz@redhat.com> > > --- > v3: rebase to include cleanups in master > v2: no change > --- > include/qapi/qmp/qstring.h | 1 + > qjson.c | 9 +++---- > qobject/qobject-json.c | 59 ++------------------------------------------- > qobject/qstring.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++ > 4 files changed, 66 insertions(+), 63 deletions(-) > > diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h > index 10076b7..a254ee3 100644 > --- a/include/qapi/qmp/qstring.h > +++ b/include/qapi/qmp/qstring.h > @@ -30,6 +30,7 @@ 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_json_string(QString *qstring, const char *raw); > QString *qobject_to_qstring(const QObject *obj); > void qstring_destroy_obj(QObject *obj); > > diff --git a/qjson.c b/qjson.c > index b65ca6e..b9a9a36 100644 > --- a/qjson.c > +++ b/qjson.c > @@ -36,9 +36,8 @@ static void json_emit_element(QJSON *json, const char *name) > } > > if (name) { > - qstring_append(json->str, "\""); > - qstring_append(json->str, name); > - qstring_append(json->str, "\" : "); > + qstring_append_json_string(json->str, name); > + qstring_append(json->str, " : "); > } > } > > @@ -77,9 +76,7 @@ void json_prop_int(QJSON *json, const char *name, int64_t val) > void json_prop_str(QJSON *json, const char *name, const char *str) > { > json_emit_element(json, name); > - qstring_append_chr(json->str, '"'); > - qstring_append(json->str, str); > - qstring_append_chr(json->str, '"'); > + qstring_append_json_string(json->str, str); > } > > const char *qjson_get_str(QJSON *json) > diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c > index 24e7d80..6fed1ee 100644 > --- a/qobject/qobject-json.c > +++ b/qobject/qobject-json.c > @@ -16,7 +16,6 @@ > #include "qapi/qmp/json-parser.h" > #include "qapi/qmp/json-streamer.h" > #include "qapi/qmp/qobject-json.h" > -#include "qemu/unicode.h" > #include "qapi/qmp/types.h" > > typedef struct JSONParsingState > @@ -81,7 +80,6 @@ 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; > - QString *qkey; > int j; > > if (s->count) { > @@ -94,9 +92,7 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) > qstring_append(s->str, " "); > } > > - qkey = qstring_from_str(key); > - to_json(QOBJECT(qkey), s->str, s->pretty, s->indent); > - QDECREF(qkey); > + qstring_append_json_string(s->str, key); > > qstring_append(s->str, ": "); > to_json(obj, s->str, s->pretty, s->indent); > @@ -138,58 +134,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) > } > case QTYPE_QSTRING: { > QString *val = qobject_to_qstring(obj); > - const char *ptr; > - int cp; > - char buf[16]; > - char *end; > - > - ptr = qstring_get_str(val); > - qstring_append(str, "\""); > - > - for (; *ptr; ptr = end) { > - cp = mod_utf8_codepoint(ptr, 6, &end); > - switch (cp) { > - case '\"': > - qstring_append(str, "\\\""); > - break; > - case '\\': > - qstring_append(str, "\\\\"); > - break; > - case '\b': > - qstring_append(str, "\\b"); > - break; > - case '\f': > - qstring_append(str, "\\f"); > - break; > - case '\n': > - qstring_append(str, "\\n"); > - break; > - case '\r': > - qstring_append(str, "\\r"); > - break; > - case '\t': > - qstring_append(str, "\\t"); > - break; > - default: > - if (cp < 0) { > - cp = 0xFFFD; /* replacement character */ > - } > - 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)); > - } else if (cp < 0x20 || cp >= 0x7F) { > - snprintf(buf, sizeof(buf), "\\u%04X", cp); > - } else { > - buf[0] = cp; > - buf[1] = 0; > - } > - qstring_append(str, buf); > - } > - }; > - > - qstring_append(str, "\""); > + qstring_append_json_string(str, qstring_get_str(val)); > break; > } > case QTYPE_QDICT: { > diff --git a/qobject/qstring.c b/qobject/qstring.c > index 5da7b5f..9f0df5b 100644 > --- a/qobject/qstring.c > +++ b/qobject/qstring.c > @@ -14,6 +14,7 @@ > #include "qapi/qmp/qobject.h" > #include "qapi/qmp/qstring.h" > #include "qemu-common.h" > +#include "qemu/unicode.h" > > /** > * qstring_new(): Create a new empty QString > @@ -107,6 +108,65 @@ void qstring_append_chr(QString *qstring, int c) > } > > /** > + * qstring_append_json_string(): Append a raw string to a QString, while > + * adding outer "" and JSON escape sequences. > + */ > +void qstring_append_json_string(QString *qstring, const char *raw) > +{ > + const char *ptr = raw; > + int cp; > + char buf[16]; > + char *end; > + > + qstring_append(qstring, "\""); > + > + for (; *ptr; ptr = end) { Make that for (ptr = raw; *ptr; ptr = end) { and drop ptr's initializer. > + cp = mod_utf8_codepoint(ptr, 6, &end); > + switch (cp) { > + case '\"': > + qstring_append(qstring, "\\\""); > + break; > + case '\\': > + qstring_append(qstring, "\\\\"); > + break; > + case '\b': > + qstring_append(qstring, "\\b"); > + break; > + case '\f': > + qstring_append(qstring, "\\f"); > + break; > + case '\n': > + qstring_append(qstring, "\\n"); > + break; > + case '\r': > + qstring_append(qstring, "\\r"); > + break; > + case '\t': > + qstring_append(qstring, "\\t"); > + break; > + default: > + if (cp < 0) { > + cp = 0xFFFD; /* replacement character */ > + } > + 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)); > + } else if (cp < 0x20 || cp >= 0x7F) { > + snprintf(buf, sizeof(buf), "\\u%04X", cp); > + } else { > + buf[0] = cp; > + buf[1] = 0; > + } > + qstring_append(qstring, buf); > + } > + }; > + > + qstring_append(qstring, "\""); > +} I think this belongs to qobject-json.c, because it's very much about JSON (it encapsulates knowledge on JSON string escaping), and a mere user of QString (it knows nothing about QString's implementation). Precedence: qobject_from_json() & friends are there, not in qobject.c. Since qjson.c doesn't already use the space-wasting "start the function comment with the function's name" style, let's drop that waste of screen space and simply write /* * Append a JSON string to @qstring that encodes the C string @str. * The JSON string is enclosed in double quotes and has funny * characters escaped. */ and rename raw to str. > + > +/** > * qobject_to_qstring(): Convert a QObject to a QString > */ > QString *qobject_to_qstring(const QObject *obj)
On 04/29/2016 06:09 AM, Markus Armbruster wrote: > Eric Blake <eblake@redhat.com> writes: > >> Pull out a new qstring_append_json_string() helper, so that all >> JSON output producers can use the same output escaping rules. >> >> While it appears that vmstate's use of the simpler qjson.c >> formatter is not currently encountering any string that needs >> escapes to be valid JSON, it is better to be safe than sorry. >> >> Signed-off-by: Eric Blake <eblake@redhat.com> >> Reviewed-by: Fam Zheng <famz@redhat.com> >> - qstring_append(str, "\""); >> + qstring_append_json_string(str, qstring_get_str(val)); >> break; > I think this belongs to qobject-json.c, because it's very much about > JSON (it encapsulates knowledge on JSON string escaping), and a mere > user of QString (it knows nothing about QString's implementation). > > Precedence: qobject_from_json() & friends are there, not in qobject.c. Fair enough. Does the name qstring_append_json_string() still work, or do I need to adjust the name to something with 'qobject' in there, to make it easier to know which header and .c file to use for the function?
Eric Blake <eblake@redhat.com> writes: > On 04/29/2016 06:09 AM, Markus Armbruster wrote: >> Eric Blake <eblake@redhat.com> writes: >> >>> Pull out a new qstring_append_json_string() helper, so that all >>> JSON output producers can use the same output escaping rules. >>> >>> While it appears that vmstate's use of the simpler qjson.c >>> formatter is not currently encountering any string that needs >>> escapes to be valid JSON, it is better to be safe than sorry. >>> >>> Signed-off-by: Eric Blake <eblake@redhat.com> >>> Reviewed-by: Fam Zheng <famz@redhat.com> > >>> - qstring_append(str, "\""); >>> + qstring_append_json_string(str, qstring_get_str(val)); >>> break; > >> I think this belongs to qobject-json.c, because it's very much about >> JSON (it encapsulates knowledge on JSON string escaping), and a mere >> user of QString (it knows nothing about QString's implementation). >> >> Precedence: qobject_from_json() & friends are there, not in qobject.c. > > Fair enough. Does the name qstring_append_json_string() still work, or > do I need to adjust the name to something with 'qobject' in there, to > make it easier to know which header and .c file to use for the function? I think the name is fine. If you strongly prefer to encode the source file in the identifier, you could use qobject_json_string_append_to_qstring(), but that's even longer, and less clear.
diff --git a/include/qapi/qmp/qstring.h b/include/qapi/qmp/qstring.h index 10076b7..a254ee3 100644 --- a/include/qapi/qmp/qstring.h +++ b/include/qapi/qmp/qstring.h @@ -30,6 +30,7 @@ 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_json_string(QString *qstring, const char *raw); QString *qobject_to_qstring(const QObject *obj); void qstring_destroy_obj(QObject *obj); diff --git a/qjson.c b/qjson.c index b65ca6e..b9a9a36 100644 --- a/qjson.c +++ b/qjson.c @@ -36,9 +36,8 @@ static void json_emit_element(QJSON *json, const char *name) } if (name) { - qstring_append(json->str, "\""); - qstring_append(json->str, name); - qstring_append(json->str, "\" : "); + qstring_append_json_string(json->str, name); + qstring_append(json->str, " : "); } } @@ -77,9 +76,7 @@ void json_prop_int(QJSON *json, const char *name, int64_t val) void json_prop_str(QJSON *json, const char *name, const char *str) { json_emit_element(json, name); - qstring_append_chr(json->str, '"'); - qstring_append(json->str, str); - qstring_append_chr(json->str, '"'); + qstring_append_json_string(json->str, str); } const char *qjson_get_str(QJSON *json) diff --git a/qobject/qobject-json.c b/qobject/qobject-json.c index 24e7d80..6fed1ee 100644 --- a/qobject/qobject-json.c +++ b/qobject/qobject-json.c @@ -16,7 +16,6 @@ #include "qapi/qmp/json-parser.h" #include "qapi/qmp/json-streamer.h" #include "qapi/qmp/qobject-json.h" -#include "qemu/unicode.h" #include "qapi/qmp/types.h" typedef struct JSONParsingState @@ -81,7 +80,6 @@ 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; - QString *qkey; int j; if (s->count) { @@ -94,9 +92,7 @@ static void to_json_dict_iter(const char *key, QObject *obj, void *opaque) qstring_append(s->str, " "); } - qkey = qstring_from_str(key); - to_json(QOBJECT(qkey), s->str, s->pretty, s->indent); - QDECREF(qkey); + qstring_append_json_string(s->str, key); qstring_append(s->str, ": "); to_json(obj, s->str, s->pretty, s->indent); @@ -138,58 +134,7 @@ static void to_json(const QObject *obj, QString *str, int pretty, int indent) } case QTYPE_QSTRING: { QString *val = qobject_to_qstring(obj); - const char *ptr; - int cp; - char buf[16]; - char *end; - - ptr = qstring_get_str(val); - qstring_append(str, "\""); - - for (; *ptr; ptr = end) { - cp = mod_utf8_codepoint(ptr, 6, &end); - switch (cp) { - case '\"': - qstring_append(str, "\\\""); - break; - case '\\': - qstring_append(str, "\\\\"); - break; - case '\b': - qstring_append(str, "\\b"); - break; - case '\f': - qstring_append(str, "\\f"); - break; - case '\n': - qstring_append(str, "\\n"); - break; - case '\r': - qstring_append(str, "\\r"); - break; - case '\t': - qstring_append(str, "\\t"); - break; - default: - if (cp < 0) { - cp = 0xFFFD; /* replacement character */ - } - 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)); - } else if (cp < 0x20 || cp >= 0x7F) { - snprintf(buf, sizeof(buf), "\\u%04X", cp); - } else { - buf[0] = cp; - buf[1] = 0; - } - qstring_append(str, buf); - } - }; - - qstring_append(str, "\""); + qstring_append_json_string(str, qstring_get_str(val)); break; } case QTYPE_QDICT: { diff --git a/qobject/qstring.c b/qobject/qstring.c index 5da7b5f..9f0df5b 100644 --- a/qobject/qstring.c +++ b/qobject/qstring.c @@ -14,6 +14,7 @@ #include "qapi/qmp/qobject.h" #include "qapi/qmp/qstring.h" #include "qemu-common.h" +#include "qemu/unicode.h" /** * qstring_new(): Create a new empty QString @@ -107,6 +108,65 @@ void qstring_append_chr(QString *qstring, int c) } /** + * qstring_append_json_string(): Append a raw string to a QString, while + * adding outer "" and JSON escape sequences. + */ +void qstring_append_json_string(QString *qstring, const char *raw) +{ + const char *ptr = raw; + int cp; + char buf[16]; + char *end; + + qstring_append(qstring, "\""); + + for (; *ptr; ptr = end) { + cp = mod_utf8_codepoint(ptr, 6, &end); + switch (cp) { + case '\"': + qstring_append(qstring, "\\\""); + break; + case '\\': + qstring_append(qstring, "\\\\"); + break; + case '\b': + qstring_append(qstring, "\\b"); + break; + case '\f': + qstring_append(qstring, "\\f"); + break; + case '\n': + qstring_append(qstring, "\\n"); + break; + case '\r': + qstring_append(qstring, "\\r"); + break; + case '\t': + qstring_append(qstring, "\\t"); + break; + default: + if (cp < 0) { + cp = 0xFFFD; /* replacement character */ + } + 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)); + } else if (cp < 0x20 || cp >= 0x7F) { + snprintf(buf, sizeof(buf), "\\u%04X", cp); + } else { + buf[0] = cp; + buf[1] = 0; + } + qstring_append(qstring, buf); + } + }; + + qstring_append(qstring, "\""); +} + +/** * qobject_to_qstring(): Convert a QObject to a QString */ QString *qobject_to_qstring(const QObject *obj)