qobject_from_jsonv() was unusual in that it took a va_list*, instead
of the more typical va_list; this was so that callers could pass
NULL to avoid % interpolation. While this works under the hood, it
is awkward for callers, so move the magic into qjson.c rather than
in the public interface, and finally improve the documentation of
qobject_from_jsonf().
test-qobject-input-visitor.c's visitor_input_test_init_internal()
was the only caller passing NULL, fix it to instead use a QObject
created by the various callers, who now use the appropriate form
of qobject_from_json*() according to whether % interpolation is
desired.
Once that is done, all remaining callers to qobject_from_jsonv() are
passing &error_abort; drop this parameter to match the counterpart
qobject_from_jsonf() which assert()s success instead. Besides, all
current callers that need interpolation live in the testsuite, where
enforcing well-defined input by asserts can help catch typos, and
where we should not be operating on dynamic untrusted arbitrary
input in the first place.
Asserting success has another nice benefit: if we pass more than one
%p, but could return failure, we would have to worry about whether
all arguments in the va_list had consistent refcount treatment (it
should be an all-or-none decision on whether each QObject in the
va_list had its reference count altered - but whichever way we
prefer, it's a lot of overhead to ensure we do it for everything
in the va_list even if we failed halfway through). But now that we
promise success, we can now easily promise that all %p arguments will
now be cleaned up when freeing the returned object.
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
include/qapi/qmp/qjson.h | 2 +-
tests/libqtest.c | 10 ++------
qobject/qjson.c | 49 +++++++++++++++++++++++++++++++++++---
tests/test-qobject-input-visitor.c | 18 ++++++++------
4 files changed, 60 insertions(+), 19 deletions(-)
diff --git a/include/qapi/qmp/qjson.h b/include/qapi/qmp/qjson.h
index 6e84082d5f..9aacb1ccf6 100644
--- a/include/qapi/qmp/qjson.h
+++ b/include/qapi/qmp/qjson.h
@@ -19,7 +19,7 @@
QObject *qobject_from_json(const char *string, Error **errp);
QObject *qobject_from_jsonf(const char *string, ...) GCC_FMT_ATTR(1, 2);
-QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp)
+QObject *qobject_from_jsonv(const char *string, va_list ap)
GCC_FMT_ATTR(1, 0);
QString *qobject_to_json(const QObject *obj);
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 99a07c246f..cde737ec5a 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -448,7 +448,6 @@ QDict *qtest_qmp_receive(QTestState *s)
*/
void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
{
- va_list ap_copy;
QObject *qobj;
int log = getenv("QTEST_LOG") != NULL;
QString *qstr;
@@ -463,13 +462,8 @@ void qmp_fd_sendv(int fd, const char *fmt, va_list ap)
}
assert(*fmt);
- /* Going through qobject ensures we escape strings properly.
- * This seemingly unnecessary copy is required in case va_list
- * is an array type.
- */
- va_copy(ap_copy, ap);
- qobj = qobject_from_jsonv(fmt, &ap_copy, &error_abort);
- va_end(ap_copy);
+ /* Going through qobject ensures we escape strings properly. */
+ qobj = qobject_from_jsonv(fmt, ap);
qstr = qobject_to_json(qobj);
/*
diff --git a/qobject/qjson.c b/qobject/qjson.c
index 2e0930884e..210c290aa9 100644
--- a/qobject/qjson.c
+++ b/qobject/qjson.c
@@ -35,7 +35,8 @@ static void parse_json(JSONMessageParser *parser, GQueue
*tokens)
s->result = json_parser_parse_err(tokens, s->ap, &s->err);
}
-QObject *qobject_from_jsonv(const char *string, va_list *ap, Error **errp)
+static QObject *qobject_from_json_internal(const char *string, va_list *ap,
+ Error **errp)
{
JSONParsingState state = {};
@@ -50,12 +51,31 @@ QObject *qobject_from_jsonv(const char *string, va_list
*ap, Error **errp)
return state.result;
}
+/*
+ * Parses JSON input without interpolation.
+ *
+ * Returns a QObject matching the input on success, or NULL with
+ * an error set if the input is not valid JSON.
+ */
QObject *qobject_from_json(const char *string, Error **errp)
{
- return qobject_from_jsonv(string, NULL, errp);
+ return qobject_from_json_internal(string, NULL, errp);
}
/*
+ * Parses JSON input with interpolation of % sequences.
+ *
+ * The set of sequences recognized is compatible with gcc's -Wformat
+ * warnings, although not all printf sequences are valid. All use of
+ * % must occur outside JSON strings.
+ *
+ * %i - treat corresponding integer value as JSON bool
+ * %[l[l]]d, %PRId64 - treat sized integer value as signed JSON number
+ * %[l[l]]u, %PRIu64 - treat sized integer value as unsigned JSON number
+ * %f - treat double as JSON number (undefined for inf, NaN)
+ * %s - convert char * into JSON string (adds escapes, outer quotes)
+ * %p - embed QObject, transferring the reference to the returned object
+ *
* IMPORTANT: This function aborts on error, thus it must not
* be used with untrusted arguments.
*/
@@ -65,13 +85,36 @@ QObject *qobject_from_jsonf(const char *string, ...)
va_list ap;
va_start(ap, string);
- obj = qobject_from_jsonv(string, &ap, &error_abort);
+ obj = qobject_from_json_internal(string, &ap, &error_abort);
va_end(ap);
assert(obj != NULL);
return obj;
}
+/*
+ * va_list form of qobject_from_jsonf().
+ *
+ * IMPORTANT: This function aborts on error, thus it must not
+ * be used with untrusted arguments.
+ */
+QObject *qobject_from_jsonv(const char *string, va_list ap)
+{
+ QObject *obj;
+ va_list ap_copy;
+
+ /*
+ * This seemingly unnecessary copy is required in case va_list
+ * is an array type.
+ */
+ va_copy(ap_copy, ap);
+ obj = qobject_from_json_internal(string, &ap_copy, &error_abort);
+ va_end(ap_copy);
+
+ assert(obj != NULL);
+ return obj;
+}
+
typedef struct ToJsonIterState
{
int indent;
diff --git a/tests/test-qobject-input-visitor.c
b/tests/test-qobject-input-visitor.c
index bcf02617dc..a9addd9f98 100644
--- a/tests/test-qobject-input-visitor.c
+++ b/tests/test-qobject-input-visitor.c
@@ -45,13 +45,11 @@ static void visitor_input_teardown(TestInputVisitorData
*data,
function so that the JSON string used by the tests are kept in the test
functions (and not in main()). */
static Visitor *visitor_input_test_init_internal(TestInputVisitorData *data,
- bool keyval,
- const char *json_string,
- va_list *ap)
+ bool keyval, QObject *obj)
{
visitor_input_teardown(data, NULL);
- data->obj = qobject_from_jsonv(json_string, ap, &error_abort);
+ data->obj = obj;
g_assert(data->obj);
if (keyval) {
@@ -69,10 +67,12 @@ Visitor *visitor_input_test_init_full(TestInputVisitorData
*data,
const char *json_string, ...)
{
Visitor *v;
+ QObject *obj;
va_list ap;
va_start(ap, json_string);
- v = visitor_input_test_init_internal(data, keyval, json_string, &ap);
+ obj = qobject_from_jsonv(json_string, ap);
+ v = visitor_input_test_init_internal(data, keyval, obj);
va_end(ap);
return v;
}
@@ -82,10 +82,12 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
const char *json_string, ...)
{
Visitor *v;
+ QObject *obj;
va_list ap;
va_start(ap, json_string);
- v = visitor_input_test_init_internal(data, false, json_string, &ap);
+ obj = qobject_from_jsonv(json_string, ap);
+ v = visitor_input_test_init_internal(data, false, obj);
va_end(ap);
return v;
}
@@ -100,7 +102,9 @@ Visitor *visitor_input_test_init(TestInputVisitorData *data,
static Visitor *visitor_input_test_init_raw(TestInputVisitorData *data,
const char *json_string)
{
- return visitor_input_test_init_internal(data, false, json_string, NULL);
+ QObject *obj = qobject_from_json(json_string, &error_abort);
+
+ return visitor_input_test_init_internal(data, false, obj);
}
static void test_visitor_in_int(TestInputVisitorData *data,
--
2.13.3
