From patchwork Fri Aug 21 15:37:05 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: 509534 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 E345F140187 for ; Sat, 22 Aug 2015 01:46:34 +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=sx4WvCSD; dkim-atps=neutral Received: from localhost ([::1]:42212 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSoWW-0006n4-NG for incoming@patchwork.ozlabs.org; Fri, 21 Aug 2015 11:46:32 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50647) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSoO7-0002FL-KZ for qemu-devel@nongnu.org; Fri, 21 Aug 2015 11:37:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZSoO5-00021j-Hs for qemu-devel@nongnu.org; Fri, 21 Aug 2015 11:37:51 -0400 Received: from mail-wi0-x22b.google.com ([2a00:1450:400c:c05::22b]:38639) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZSoO5-00021W-7d for qemu-devel@nongnu.org; Fri, 21 Aug 2015 11:37:49 -0400 Received: by wicja10 with SMTP id ja10so19169564wic.1 for ; Fri, 21 Aug 2015 08:37:48 -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=BqfsbH3XZcWIo0xD5Wn2kET4yMm3Kc2uLmFxrZUTryg=; b=sx4WvCSDvTuGqV8R6G+yK/qu4OhDiA/hsCnZqAMBe9h3b9a8ximjapXuYvTuYjalpY V+o7RYgx3U/LMKpAzTokJjkRKRkanT0o7su236hffhVBUR+bV32ZvbBhhghvnwa8kRSI KJuIVV8pCQUvQ/iLA8o5G8L3jkpAcmeflPRnpZ1pZx03tdZceGQvAM0EOaFk1MQwCLeK FMSVlJ4AWpSXHIZEx6MemxjEWpcSO7bVj2FniiYX+VBjsUFvlwUpG1iviWr8rdxm0XPl pPWcBBizSDSDJs3b11HXL1tLgAr0+mEC5cuR2WU2gyABkz9yKWrd7BMXXsCLhBLg3u3a chDg== X-Received: by 10.180.107.70 with SMTP id ha6mr6634745wib.20.1440171468681; Fri, 21 Aug 2015 08:37:48 -0700 (PDT) Received: from nullptr.home.dirty-ice.org (178-164-172-216.pool.digikabel.hu. [178.164.172.216]) by smtp.gmail.com with ESMTPSA id v8sm10479895wjr.15.2015.08.21.08.37.47 (version=TLSv1/SSLv3 cipher=OTHER); Fri, 21 Aug 2015 08:37:48 -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: Fri, 21 Aug 2015 17:37:05 +0200 Message-Id: X-Mailer: git-send-email 2.5.0 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::22b Cc: Michael Roth , Gerd Hoffmann , Markus Armbruster Subject: [Qemu-devel] [PATCH v2 09/49] 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-ambigous 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 c7eaa86..a818eff 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -81,6 +81,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: # @@ -94,7 +99,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 cf0ccc4..a758cc9 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([('integer', 'int')]))]), OrderedDict([('struct', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]), OrderedDict([('struct', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]), - 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; }