From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52977) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bosD0-0004uC-DX for qemu-devel@nongnu.org; Tue, 27 Sep 2016 09:14:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bosCt-0003mz-Vm for qemu-devel@nongnu.org; Tue, 27 Sep 2016 09:14:05 -0400 From: "Daniel P. Berrange" Date: Tue, 27 Sep 2016 14:13:10 +0100 Message-Id: <1474982001-20878-9-git-send-email-berrange@redhat.com> In-Reply-To: <1474982001-20878-1-git-send-email-berrange@redhat.com> References: <1474982001-20878-1-git-send-email-berrange@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: [Qemu-devel] [PATCH v14 08/19] qapi: permit scalar type conversions in QObjectInputVisitor List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: qemu-block@nongnu.org, Markus Armbruster , Max Reitz , Paolo Bonzini , =?UTF-8?q?Andreas=20F=C3=A4rber?= , "Daniel P. Berrange" Currently the QObjectInputVisitor assumes that all scalar values are directly represented as the final types declared by the thing being visited. ie it assumes an 'int' is using QInt, and a 'bool' is using QBool, etc. This is good when QObjectInputVisitor is fed a QObject that came from a JSON document on the QMP monitor, as it will strictly validate correctness. To allow QObjectInputVisitor to be reused for visiting a QObject originating from QemuOpts, an alternative mode is needed where all the scalars types are represented as QString and converted on the fly to the final desired type. Reviewed-by: Kevin Wolf Reviewed-by: Marc-Andr=C3=A9 Lureau Signed-off-by: Daniel P. Berrange --- include/qapi/qobject-input-visitor.h | 32 +++++- qapi/qobject-input-visitor.c | 132 ++++++++++++++++++++++++ tests/test-qobject-input-visitor.c | 194 +++++++++++++++++++++++++++++= +++++- 3 files changed, 350 insertions(+), 8 deletions(-) diff --git a/include/qapi/qobject-input-visitor.h b/include/qapi/qobject-= input-visitor.h index cde328d..5022297 100644 --- a/include/qapi/qobject-input-visitor.h +++ b/include/qapi/qobject-input-visitor.h @@ -19,12 +19,36 @@ =20 typedef struct QObjectInputVisitor QObjectInputVisitor; =20 -/* - * Return a new input visitor that converts a QObject to a QAPI object. +/** + * Create a new input visitor that converts @obj to a QAPI object. + * + * Any scalar values in the @obj input data structure should be in the + * required type already. i.e. if visiting a bool, the value should + * already be a QBool instance. * - * Set @strict to reject a parse that doesn't consume all keys of a - * dictionary; otherwise excess input is ignored. + * If @strict is set to true, then an error will be reported if any + * dict keys are not consumed during visitation. If @strict is false + * then extra dict keys are silently ignored. + * + * The returned input visitor should be released by calling + * visit_free() when no longer required. */ Visitor *qobject_input_visitor_new(QObject *obj, bool strict); =20 +/** + * Create a new input visitor that converts @obj to a QAPI object. + * + * Any scalar values in the @obj input data structure should always be + * represented as strings. i.e. if visiting a boolean, the value should + * be a QString whose contents represent a valid boolean. + * + * The visitor always operates in strict mode, requiring all dict keys + * to be consumed during visitation. An error will be reported if this + * does not happen. + * + * The returned input visitor should be released by calling + * visit_free() when no longer required. + */ +Visitor *qobject_input_visitor_new_autocast(QObject *obj); + #endif diff --git a/qapi/qobject-input-visitor.c b/qapi/qobject-input-visitor.c index 5ff3db3..cf41df6 100644 --- a/qapi/qobject-input-visitor.c +++ b/qapi/qobject-input-visitor.c @@ -20,6 +20,7 @@ #include "qemu-common.h" #include "qapi/qmp/types.h" #include "qapi/qmp/qerror.h" +#include "qemu/cutils.h" =20 #define QIV_STACK_SIZE 1024 =20 @@ -263,6 +264,28 @@ static void qobject_input_type_int64(Visitor *v, con= st char *name, int64_t *obj, *obj =3D qint_get_int(qint); } =20 + +static void qobject_input_type_int64_autocast(Visitor *v, const char *na= me, + int64_t *obj, Error **errp= ) +{ + QObjectInputVisitor *qiv =3D to_qiv(v); + QString *qstr =3D qobject_to_qstring(qobject_input_get_object(qiv, n= ame, + true)); + int64_t ret; + + if (!qstr || !qstr->string) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nul= l", + "string"); + return; + } + + if (qemu_strtoll(qstr->string, NULL, 0, &ret) < 0) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number")= ; + return; + } + *obj =3D ret; +} + static void qobject_input_type_uint64(Visitor *v, const char *name, uint64_t *obj, Error **errp) { @@ -279,6 +302,27 @@ static void qobject_input_type_uint64(Visitor *v, co= nst char *name, *obj =3D qint_get_int(qint); } =20 +static void qobject_input_type_uint64_autocast(Visitor *v, const char *n= ame, + uint64_t *obj, Error **er= rp) +{ + QObjectInputVisitor *qiv =3D to_qiv(v); + QString *qstr =3D qobject_to_qstring(qobject_input_get_object(qiv, n= ame, + true)); + unsigned long long ret; + + if (!qstr || !qstr->string) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nul= l", + "string"); + return; + } + + if (parse_uint_full(qstr->string, &ret, 0) < 0) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name, "a number")= ; + return; + } + *obj =3D ret; +} + static void qobject_input_type_bool(Visitor *v, const char *name, bool *= obj, Error **errp) { @@ -294,6 +338,22 @@ static void qobject_input_type_bool(Visitor *v, cons= t char *name, bool *obj, *obj =3D qbool_get_bool(qbool); } =20 +static void qobject_input_type_bool_autocast(Visitor *v, const char *nam= e, + bool *obj, Error **errp) +{ + QObjectInputVisitor *qiv =3D to_qiv(v); + QString *qstr =3D qobject_to_qstring(qobject_input_get_object(qiv, n= ame, + true)); + + if (!qstr || !qstr->string) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nul= l", + "string"); + return; + } + + parse_option_bool(name, qstr->string, obj, errp); +} + static void qobject_input_type_str(Visitor *v, const char *name, char **= obj, Error **errp) { @@ -335,6 +395,30 @@ static void qobject_input_type_number(Visitor *v, co= nst char *name, double *obj, "number"); } =20 +static void qobject_input_type_number_autocast(Visitor *v, const char *n= ame, + double *obj, Error **errp= ) +{ + QObjectInputVisitor *qiv =3D to_qiv(v); + QString *qstr =3D qobject_to_qstring(qobject_input_get_object(qiv, n= ame, + true)); + char *endp; + + if (!qstr || !qstr->string) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nul= l", + "string"); + return; + } + + errno =3D 0; + *obj =3D strtod(qstr->string, &endp); + if (errno =3D=3D 0 && endp !=3D qstr->string && *endp =3D=3D '\0') { + return; + } + + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "null", + "number"); +} + static void qobject_input_type_any(Visitor *v, const char *name, QObject= **obj, Error **errp) { @@ -356,6 +440,22 @@ static void qobject_input_type_null(Visitor *v, cons= t char *name, Error **errp) } } =20 +static void qobject_input_type_size_autocast(Visitor *v, const char *nam= e, + uint64_t *obj, Error **errp= ) +{ + QObjectInputVisitor *qiv =3D to_qiv(v); + QString *qstr =3D qobject_to_qstring(qobject_input_get_object(qiv, n= ame, + true)); + + if (!qstr || !qstr->string) { + error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name ? name : "nul= l", + "string"); + return; + } + + parse_option_size(name, qstr->string, obj, errp); +} + static void qobject_input_optional(Visitor *v, const char *name, bool *p= resent) { QObjectInputVisitor *qiv =3D to_qiv(v); @@ -413,3 +513,35 @@ Visitor *qobject_input_visitor_new(QObject *obj, boo= l strict) =20 return &v->visitor; } + +Visitor *qobject_input_visitor_new_autocast(QObject *obj) +{ + QObjectInputVisitor *v; + + v =3D g_malloc0(sizeof(*v)); + + v->visitor.type =3D VISITOR_INPUT; + v->visitor.start_struct =3D qobject_input_start_struct; + v->visitor.check_struct =3D qobject_input_check_struct; + v->visitor.end_struct =3D qobject_input_pop; + v->visitor.start_list =3D qobject_input_start_list; + v->visitor.next_list =3D qobject_input_next_list; + v->visitor.end_list =3D qobject_input_pop; + v->visitor.start_alternate =3D qobject_input_start_alternate; + v->visitor.type_int64 =3D qobject_input_type_int64_autocast; + v->visitor.type_uint64 =3D qobject_input_type_uint64_autocast; + v->visitor.type_bool =3D qobject_input_type_bool_autocast; + v->visitor.type_str =3D qobject_input_type_str; + v->visitor.type_number =3D qobject_input_type_number_autocast; + v->visitor.type_any =3D qobject_input_type_any; + v->visitor.type_null =3D qobject_input_type_null; + v->visitor.type_size =3D qobject_input_type_size_autocast; + v->visitor.optional =3D qobject_input_optional; + v->visitor.free =3D qobject_input_free; + v->strict =3D true; + + v->root =3D obj; + qobject_incref(obj); + + return &v->visitor; +} diff --git a/tests/test-qobject-input-visitor.c b/tests/test-qobject-inpu= t-visitor.c index 26c5012..d5b8044 100644 --- a/tests/test-qobject-input-visitor.c +++ b/tests/test-qobject-input-visitor.c @@ -41,6 +41,7 @@ static void visitor_input_teardown(TestInputVisitorData= *data, function so that the JSON string used by the tests are kept in the te= st functions (and not in main()). */ static Visitor *visitor_input_test_init_internal(TestInputVisitorData *d= ata, + bool strict, bool autoc= ast, const char *json_string= , va_list *ap) { @@ -49,11 +50,31 @@ static Visitor *visitor_input_test_init_internal(Test= InputVisitorData *data, data->obj =3D qobject_from_jsonv(json_string, ap); g_assert(data->obj); =20 - data->qiv =3D qobject_input_visitor_new(data->obj, false); + if (autocast) { + assert(strict); + data->qiv =3D qobject_input_visitor_new_autocast(data->obj); + } else { + data->qiv =3D qobject_input_visitor_new(data->obj, strict); + } g_assert(data->qiv); return data->qiv; } =20 +static GCC_FMT_ATTR(4, 5) +Visitor *visitor_input_test_init_full(TestInputVisitorData *data, + bool strict, bool autocast, + const char *json_string, ...) +{ + Visitor *v; + va_list ap; + + va_start(ap, json_string); + v =3D visitor_input_test_init_internal(data, strict, autocast, + json_string, &ap); + va_end(ap); + return v; +} + static GCC_FMT_ATTR(2, 3) Visitor *visitor_input_test_init(TestInputVisitorData *data, const char *json_string, ...) @@ -62,7 +83,8 @@ Visitor *visitor_input_test_init(TestInputVisitorData *= data, va_list ap; =20 va_start(ap, json_string); - v =3D visitor_input_test_init_internal(data, json_string, &ap); + v =3D visitor_input_test_init_internal(data, true, false, + json_string, &ap); va_end(ap); return v; } @@ -77,7 +99,8 @@ 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, json_string, NULL); + return visitor_input_test_init_internal(data, true, false, + json_string, NULL); } =20 static void test_visitor_in_int(TestInputVisitorData *data, @@ -109,6 +132,45 @@ static void test_visitor_in_int_overflow(TestInputVi= sitorData *data, error_free_or_abort(&err); } =20 +static void test_visitor_in_int_autocast(TestInputVisitorData *data, + const void *unused) +{ + int64_t res =3D 0, value =3D -42; + Error *err =3D NULL; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, + "%" PRId64, value); + visit_type_int(v, NULL, &res, &err); + error_free_or_abort(&err); +} + +static void test_visitor_in_int_str_autocast(TestInputVisitorData *data, + const void *unused) +{ + int64_t res =3D 0, value =3D -42; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, + "\"-42\""); + + visit_type_int(v, NULL, &res, &error_abort); + g_assert_cmpint(res, =3D=3D, value); +} + +static void test_visitor_in_int_str_noautocast(TestInputVisitorData *dat= a, + const void *unused) +{ + int64_t res =3D 0; + Visitor *v; + Error *err =3D NULL; + + v =3D visitor_input_test_init(data, "\"-42\""); + + visit_type_int(v, NULL, &res, &err); + error_free_or_abort(&err); +} + static void test_visitor_in_bool(TestInputVisitorData *data, const void *unused) { @@ -121,6 +183,44 @@ static void test_visitor_in_bool(TestInputVisitorDat= a *data, g_assert_cmpint(res, =3D=3D, true); } =20 +static void test_visitor_in_bool_autocast(TestInputVisitorData *data, + const void *unused) +{ + bool res =3D false; + Error *err =3D NULL; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, "true"); + + visit_type_bool(v, NULL, &res, &err); + error_free_or_abort(&err); +} + +static void test_visitor_in_bool_str_autocast(TestInputVisitorData *data= , + const void *unused) +{ + bool res =3D false; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, "\"yes\""); + + visit_type_bool(v, NULL, &res, &error_abort); + g_assert_cmpint(res, =3D=3D, true); +} + +static void test_visitor_in_bool_str_noautocast(TestInputVisitorData *da= ta, + const void *unused) +{ + bool res =3D false; + Visitor *v; + Error *err =3D NULL; + + v =3D visitor_input_test_init(data, "\"true\""); + + visit_type_bool(v, NULL, &res, &err); + error_free_or_abort(&err); +} + static void test_visitor_in_number(TestInputVisitorData *data, const void *unused) { @@ -133,6 +233,69 @@ static void test_visitor_in_number(TestInputVisitorD= ata *data, g_assert_cmpfloat(res, =3D=3D, value); } =20 +static void test_visitor_in_number_autocast(TestInputVisitorData *data, + const void *unused) +{ + double res =3D 0, value =3D 3.14; + Error *err =3D NULL; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, "%f", value); + + visit_type_number(v, NULL, &res, &err); + error_free_or_abort(&err); +} + +static void test_visitor_in_number_str_autocast(TestInputVisitorData *da= ta, + const void *unused) +{ + double res =3D 0, value =3D 3.14; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, "\"3.14\""); + + visit_type_number(v, NULL, &res, &error_abort); + g_assert_cmpfloat(res, =3D=3D, value); +} + +static void test_visitor_in_number_str_noautocast(TestInputVisitorData *= data, + const void *unused) +{ + double res =3D 0; + Visitor *v; + Error *err =3D NULL; + + v =3D visitor_input_test_init(data, "\"3.14\""); + + visit_type_number(v, NULL, &res, &err); + error_free_or_abort(&err); +} + +static void test_visitor_in_size_str_autocast(TestInputVisitorData *data= , + const void *unused) +{ + uint64_t res, value =3D 500 * 1024 * 1024; + Visitor *v; + + v =3D visitor_input_test_init_full(data, true, true, "\"500M\""); + + visit_type_size(v, NULL, &res, &error_abort); + g_assert_cmpfloat(res, =3D=3D, value); +} + +static void test_visitor_in_size_str_noautocast(TestInputVisitorData *da= ta, + const void *unused) +{ + uint64_t res =3D 0; + Visitor *v; + Error *err =3D NULL; + + v =3D visitor_input_test_init(data, "\"500M\""); + + visit_type_size(v, NULL, &res, &err); + error_free_or_abort(&err); +} + static void test_visitor_in_string(TestInputVisitorData *data, const void *unused) { @@ -289,7 +452,8 @@ static void test_visitor_in_null(TestInputVisitorData= *data, * when input is not null. */ =20 - v =3D visitor_input_test_init(data, "{ 'a': null, 'b': '' }"); + v =3D visitor_input_test_init_full(data, false, false, + "{ 'a': null, 'b': '' }"); visit_start_struct(v, NULL, NULL, 0, &error_abort); visit_type_null(v, "a", &error_abort); visit_type_str(v, "a", &tmp, &err); @@ -840,10 +1004,32 @@ int main(int argc, char **argv) NULL, test_visitor_in_int); input_visitor_test_add("/visitor/input/int_overflow", NULL, test_visitor_in_int_overflow); + input_visitor_test_add("/visitor/input/int_autocast", + NULL, test_visitor_in_int_autocast); + input_visitor_test_add("/visitor/input/int_str_autocast", + NULL, test_visitor_in_int_str_autocast); + input_visitor_test_add("/visitor/input/int_str_noautocast", + NULL, test_visitor_in_int_str_noautocast); input_visitor_test_add("/visitor/input/bool", NULL, test_visitor_in_bool); + input_visitor_test_add("/visitor/input/bool_autocast", + NULL, test_visitor_in_bool_autocast); + input_visitor_test_add("/visitor/input/bool_str_autocast", + NULL, test_visitor_in_bool_str_autocast); + input_visitor_test_add("/visitor/input/bool_str_noautocast", + NULL, test_visitor_in_bool_str_noautocast); input_visitor_test_add("/visitor/input/number", NULL, test_visitor_in_number); + input_visitor_test_add("/visitor/input/number_autocast", + NULL, test_visitor_in_number_autocast); + input_visitor_test_add("/visitor/input/number_str_autocast", + NULL, test_visitor_in_number_str_autocast); + input_visitor_test_add("/visitor/input/number_str_noautocast", + NULL, test_visitor_in_number_str_noautocast); + input_visitor_test_add("/visitor/input/size_str_autocast", + NULL, test_visitor_in_size_str_autocast); + input_visitor_test_add("/visitor/input/size_str_noautocast", + NULL, test_visitor_in_size_str_noautocast); input_visitor_test_add("/visitor/input/string", NULL, test_visitor_in_string); input_visitor_test_add("/visitor/input/enum", --=20 2.7.4