From patchwork Mon Sep 7 12:14:09 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?UTF-8?B?Wm9sdMOhbiBLxZF2w6Fnw7M=?= X-Patchwork-Id: 515109 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 90D4D140281 for ; Mon, 7 Sep 2015 22:14:39 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b=oysFXYEL; dkim-atps=neutral Received: from localhost ([::1]:56307 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYvJl-0001B6-QO for incoming@patchwork.ozlabs.org; Mon, 07 Sep 2015 08:14:37 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38435) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYvJC-0000Oe-Se for qemu-devel@nongnu.org; Mon, 07 Sep 2015 08:14:04 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZYvJA-0005wr-Ht for qemu-devel@nongnu.org; Mon, 07 Sep 2015 08:14:02 -0400 Received: from mail-wi0-x22a.google.com ([2a00:1450:400c:c05::22a]:36308) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZYvJA-0005wV-7J for qemu-devel@nongnu.org; Mon, 07 Sep 2015 08:14:00 -0400 Received: by wicgb1 with SMTP id gb1so43665212wic.1 for ; Mon, 07 Sep 2015 05:13:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-type:content-transfer-encoding; bh=qeW/W0eOoFFaOeQI/2sV/geBOGXQJJ/5GvP8niZo+eo=; b=oysFXYELVjGiE9L6k+mEF6F/d9B7uWU7j2c9HRGVeYNzj6EbMJ5iALC63bgMtqMuvB cYGXMLV1JVg9k9WQLkVLzhlRkyL8QD7MeS+Lq8blgQngV5+ldaB7lDzcHZHtf8OR2Dst R68HU0rR4t9OkzZEtOHZcDTIYXSbXPilGliw1jJuIBPyjovjLkZbhNuXsZHcMqBivm46 k3DR8Bpw0nOEpUPPaWQUWEN3F37selrZ0C5gj6j/lmVJqRI1oEsPqv9eT8To+JnGmjNu Vmc/6sSuO1aV7WqBjWLyebQx17R3tn+F9rmixGkgeWb1mm+0wgTcLTN2i1wFvrFTpMyC 5GNw== X-Received: by 10.180.86.138 with SMTP id p10mr34594046wiz.39.1441628039579; Mon, 07 Sep 2015 05:13:59 -0700 (PDT) Received: from nullptr.home.dirty-ice.org (188-143-119-255.pool.digikabel.hu. [188.143.119.255]) by smtp.gmail.com with ESMTPSA id p5sm20117214wiy.17.2015.09.07.05.13.58 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 07 Sep 2015 05:13:58 -0700 (PDT) From: "=?UTF-8?q?K=C5=91v=C3=A1g=C3=B3=2C=20Zolt=C3=A1n?=" X-Google-Original-From: =?UTF-8?q?K=C5=91v=C3=A1g=C3=B3=2C=20Zolt=C3=A1n?= To: qemu-devel@nongnu.org Date: Mon, 7 Sep 2015 14:14:09 +0200 Message-Id: <828b3ad8c9f32d5938f39068328d0794cfd9abf4.1441627176.git.DirtY.iCE.hu@gmail.com> X-Mailer: git-send-email 2.5.1 In-Reply-To: References: MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: Error: Malformed IPv6 address (bad octet value). X-Received-From: 2a00:1450:400c:c05::22a Cc: Markus Armbruster , Michael Roth Subject: [Qemu-devel] [PATCH 7/7] qapi: support nested structs in OptsVisitor X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org The current OptsVisitor flattens the whole structure, if there are same named fields under different paths (like `in' and `out' in `Audiodev'), the current visitor can't cope with them (for example setting `frequency=44100' will set the in's frequency to 44100 and leave out's frequency unspecified). This patch fixes it, by always requiring a complete path in case of nested structs. Fields in the path are separated by dots, similar to C structs (without pointers), like `in.frequency' or `out.frequency'. You must provide a full path even in non-ambiguous cases. The qapi flattening commits hopefully ensures that this change doesn't create backward compatibility problems. Signed-off-by: Kővágó, Zoltán --- qapi/opts-visitor.c | 116 ++++++++++++++++++++++++++------ tests/qapi-schema/qapi-schema-test.json | 9 ++- tests/qapi-schema/qapi-schema-test.out | 6 +- tests/test-opts-visitor.c | 34 ++++++++++ 4 files changed, 141 insertions(+), 24 deletions(-) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index aa68814..2c2710b 100644 --- a/qapi/opts-visitor.c +++ b/qapi/opts-visitor.c @@ -71,6 +71,7 @@ struct OptsVisitor * schema, with a single mandatory scalar member. */ ListMode list_mode; GQueue *repeated_opts; + char *repeated_name; /* When parsing a list of repeating options as integers, values of the form * "a-b", representing a closed interval, are allowed. Elements in the @@ -86,6 +87,9 @@ struct OptsVisitor * not survive or escape the OptsVisitor object. */ QemuOpt *fake_id_opt; + + /* List of field names leading to the current structure. */ + GQueue *nested_names; }; @@ -100,6 +104,7 @@ static void opts_visitor_insert(GHashTable *unprocessed_opts, const QemuOpt *opt) { GQueue *list; + assert(opt); list = g_hash_table_lookup(unprocessed_opts, opt->name); if (list == NULL) { @@ -127,6 +132,9 @@ opts_start_struct(Visitor *v, void **obj, const char *kind, if (obj) { *obj = g_malloc0(size > 0 ? size : 1); } + + g_queue_push_tail(ov->nested_names, (gpointer) name); + if (ov->depth++ > 0) { return; } @@ -169,6 +177,8 @@ opts_end_struct(Visitor *v, Error **errp) OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); GQueue *any; + g_queue_pop_tail(ov->nested_names); + if (--ov->depth > 0) { return; } @@ -198,15 +208,55 @@ opts_end_implicit_struct(Visitor *v, Error **errp) } +static void +sum_strlen(gpointer data, gpointer user_data) +{ + const char *str = data; + size_t *sum_len = user_data; + + if (str) { /* skip NULLs */ + *sum_len += strlen(str) + 1; + } +} + +static void +append_str(gpointer data, gpointer user_data) +{ + const char *str = data; + char *concat_str = user_data; + + if (str) { + strcat(concat_str, str); + strcat(concat_str, "."); + } +} + +/* lookup a name, using a fully qualified version */ static GQueue * -lookup_distinct(const OptsVisitor *ov, const char *name, Error **errp) +lookup_distinct(const OptsVisitor *ov, const char *name, char **out_key, + Error **errp) { - GQueue *list; + GQueue *list = NULL; + char *key; + size_t sum_len = strlen(name); + + g_queue_foreach(ov->nested_names, sum_strlen, &sum_len); + key = g_malloc(sum_len+1); + key[0] = 0; + g_queue_foreach(ov->nested_names, append_str, key); + strcat(key, name); + + list = g_hash_table_lookup(ov->unprocessed_opts, key); + if (list && out_key) { + *out_key = key; + key = NULL; + } - list = g_hash_table_lookup(ov->unprocessed_opts, name); if (!list) { error_setg(errp, QERR_MISSING_PARAMETER, name); } + + g_free(key); return list; } @@ -218,7 +268,8 @@ opts_start_list(Visitor *v, const char *name, Error **errp) /* we can't traverse a list in a list */ assert(ov->list_mode == LM_NONE); - ov->repeated_opts = lookup_distinct(ov, name, errp); + assert(ov->repeated_opts == NULL && ov->repeated_name == NULL); + ov->repeated_opts = lookup_distinct(ov, name, &ov->repeated_name, errp); if (ov->repeated_opts != NULL) { ov->list_mode = LM_STARTED; } @@ -254,11 +305,9 @@ opts_next_list(Visitor *v, GenericList **list, Error **errp) /* range has been completed, fall through in order to pop option */ case LM_IN_PROGRESS: { - const QemuOpt *opt; - - opt = g_queue_pop_head(ov->repeated_opts); + g_queue_pop_head(ov->repeated_opts); if (g_queue_is_empty(ov->repeated_opts)) { - g_hash_table_remove(ov->unprocessed_opts, opt->name); + g_hash_table_remove(ov->unprocessed_opts, ov->repeated_name); return NULL; } link = &(*list)->next; @@ -284,22 +333,28 @@ opts_end_list(Visitor *v, Error **errp) ov->list_mode == LM_SIGNED_INTERVAL || ov->list_mode == LM_UNSIGNED_INTERVAL); ov->repeated_opts = NULL; + + g_free(ov->repeated_name); + ov->repeated_name = NULL; + ov->list_mode = LM_NONE; } static const QemuOpt * -lookup_scalar(const OptsVisitor *ov, const char *name, Error **errp) +lookup_scalar(const OptsVisitor *ov, const char *name, char** out_key, + Error **errp) { if (ov->list_mode == LM_NONE) { GQueue *list; /* the last occurrence of any QemuOpt takes effect when queried by name */ - list = lookup_distinct(ov, name, errp); + list = lookup_distinct(ov, name, out_key, errp); return list ? g_queue_peek_tail(list) : NULL; } assert(ov->list_mode == LM_IN_PROGRESS); + assert(out_key == NULL || *out_key == NULL); return g_queue_peek_head(ov->repeated_opts); } @@ -321,13 +376,15 @@ opts_type_str(Visitor *v, char **obj, const char *name, Error **errp) { OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); const QemuOpt *opt; + char *key = NULL; - opt = lookup_scalar(ov, name, errp); + opt = lookup_scalar(ov, name, &key, errp); if (!opt) { return; } *obj = g_strdup(opt->str ? opt->str : ""); - processed(ov, name); + processed(ov, key); + g_free(key); } @@ -337,8 +394,9 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) { OptsVisitor *ov = DO_UPCAST(OptsVisitor, visitor, v); const QemuOpt *opt; + char *key = NULL; - opt = lookup_scalar(ov, name, errp); + opt = lookup_scalar(ov, name, &key, errp); if (!opt) { return; } @@ -355,13 +413,15 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) } else { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, "on|yes|y|off|no|n"); + g_free(key); return; } } else { *obj = true; } - processed(ov, name); + processed(ov, key); + g_free(key); } @@ -373,13 +433,14 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) const char *str; long long val; char *endptr; + char *key = NULL; if (ov->list_mode == LM_SIGNED_INTERVAL) { *obj = ov->range_next.s; return; } - opt = lookup_scalar(ov, name, errp); + opt = lookup_scalar(ov, name, &key, errp); if (!opt) { return; } @@ -393,11 +454,13 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) if (errno == 0 && endptr > str && INT64_MIN <= val && val <= INT64_MAX) { if (*endptr == '\0') { *obj = val; - processed(ov, name); + processed(ov, key); + g_free(key); return; } if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) { long long val2; + assert(key == NULL); str = endptr + 1; val2 = strtoll(str, &endptr, 0); @@ -418,6 +481,7 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, (ov->list_mode == LM_NONE) ? "an int64 value" : "an int64 value or range"); + g_free(key); } @@ -429,13 +493,14 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp) const char *str; unsigned long long val; char *endptr; + char *key = NULL; if (ov->list_mode == LM_UNSIGNED_INTERVAL) { *obj = ov->range_next.u; return; } - opt = lookup_scalar(ov, name, errp); + opt = lookup_scalar(ov, name, &key, errp); if (!opt) { return; } @@ -447,11 +512,13 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp) if (parse_uint(str, &val, &endptr, 0) == 0 && val <= UINT64_MAX) { if (*endptr == '\0') { *obj = val; - processed(ov, name); + processed(ov, key); + g_free(key); return; } if (*endptr == '-' && ov->list_mode == LM_IN_PROGRESS) { unsigned long long val2; + assert(key == NULL); str = endptr + 1; if (parse_uint_full(str, &val2, 0) == 0 && @@ -470,6 +537,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp) error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, (ov->list_mode == LM_NONE) ? "a uint64 value" : "a uint64 value or range"); + g_free(key); } @@ -480,8 +548,9 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) const QemuOpt *opt; int64_t val; char *endptr; + char *key = NULL; - opt = lookup_scalar(ov, name, errp); + opt = lookup_scalar(ov, name, &key, errp); if (!opt) { return; } @@ -491,11 +560,13 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) if (val < 0 || *endptr) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name, "a size value representible as a non-negative int64"); + g_free(key); return; } *obj = val; - processed(ov, name); + processed(ov, key); + g_free(key); } @@ -506,7 +577,7 @@ opts_optional(Visitor *v, bool *present, const char *name, Error **errp) /* we only support a single mandatory scalar field in a list node */ assert(ov->list_mode == LM_NONE); - *present = (lookup_distinct(ov, name, NULL) != NULL); + *present = (lookup_distinct(ov, name, NULL, NULL) != NULL); } @@ -517,6 +588,8 @@ opts_visitor_new(const QemuOpts *opts) ov = g_malloc0(sizeof *ov); + ov->nested_names = g_queue_new(); + ov->visitor.start_struct = &opts_start_struct; ov->visitor.end_struct = &opts_end_struct; @@ -560,6 +633,7 @@ opts_visitor_cleanup(OptsVisitor *ov) if (ov->unprocessed_opts != NULL) { g_hash_table_destroy(ov->unprocessed_opts); } + g_queue_free(ov->nested_names); g_free(ov->fake_id_opt); g_free(ov); } diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json index a9e5aab..c4ab8b9 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -87,6 +87,11 @@ { 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' }, 'returns': 'int' } +# For testing hierarchy support in opts-visitor +{ 'struct': 'UserDefOptionsSub', + 'data': { + '*nint': 'int' } } + # For testing integer range flattening in opts-visitor. The following schema # corresponds to the option format: # @@ -100,7 +105,9 @@ '*u64' : [ 'uint64' ], '*u16' : [ 'uint16' ], '*i64x': 'int' , - '*u64x': 'uint64' } } + '*u64x': 'uint64' , + 'sub0': 'UserDefOptionsSub', + 'sub1': 'UserDefOptionsSub' } } # testing event { 'struct': 'EventStructOne', diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out index b0b7187..0e8baca 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -17,7 +17,8 @@ OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]), OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('*ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')]), OrderedDict([('command', 'user_def_cmd3'), ('data', OrderedDict([('a', 'int'), ('*b', 'int')])), ('returns', 'int')]), - OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]), + OrderedDict([('struct', 'UserDefOptionsSub'), ('data', OrderedDict([('*nint', 'int')]))]), + OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64'), ('sub0', 'UserDefOptionsSub'), ('sub1', 'UserDefOptionsSub')]))]), OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]), OrderedDict([('event', 'EVENT_A')]), OrderedDict([('event', 'EVENT_B'), ('data', OrderedDict())]), @@ -48,7 +49,8 @@ OrderedDict([('struct', 'UserDefB'), ('data', OrderedDict([('intb', 'int')]))]), OrderedDict([('struct', 'UserDefUnionBase'), ('base', 'UserDefZero'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]), - OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]), + OrderedDict([('struct', 'UserDefOptionsSub'), ('data', OrderedDict([('*nint', 'int')]))]), + OrderedDict([('struct', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64'), ('sub0', 'UserDefOptionsSub'), ('sub1', 'UserDefOptionsSub')]))]), OrderedDict([('struct', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))]), OrderedDict([('struct', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member1', '__org.qemu_x-Enum')]))]), OrderedDict([('struct', '__org.qemu_x-Struct'), ('base', '__org.qemu_x-Base'), ('data', OrderedDict([('__org.qemu_x-member2', 'str')]))]), diff --git a/tests/test-opts-visitor.c b/tests/test-opts-visitor.c index 1c753d9..4393266 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -178,6 +178,34 @@ expect_u64_max(OptsVisitorFixture *f, gconstpointer test_data) g_assert(f->userdef->u64->value == UINT64_MAX); } +static void +expect_both(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->sub0->has_nint); + g_assert(f->userdef->sub0->nint == 13); + g_assert(f->userdef->sub1->has_nint); + g_assert(f->userdef->sub1->nint == 17); +} + +static void +expect_sub0(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(f->userdef->sub0->has_nint); + g_assert(f->userdef->sub0->nint == 13); + g_assert(!f->userdef->sub1->has_nint); +} + +static void +expect_sub1(OptsVisitorFixture *f, gconstpointer test_data) +{ + expect_ok(f, test_data); + g_assert(!f->userdef->sub0->has_nint); + g_assert(f->userdef->sub1->has_nint); + g_assert(f->userdef->sub1->nint == 13); +} + /* test cases */ int @@ -271,6 +299,12 @@ main(int argc, char **argv) add_test("/visitor/opts/i64/range/2big/full", &expect_fail, "i64=-0x8000000000000000-0x7fffffffffffffff"); + /* Test nested structs support */ + add_test("/visitor/opts/nested/unqualified", &expect_fail, "nint=13"); + add_test("/visitor/opts/nested/both", &expect_both, + "sub0.nint=13,sub1.nint=17"); + add_test("/visitor/opts/nested/sub0", &expect_sub0, "sub0.nint=13"); + add_test("/visitor/opts/nested/sub1", &expect_sub1, "sub1.nint=13"); g_test_run(); return 0; }