From patchwork Wed Sep 23 14:27:36 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: 521715 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 F2C051401AF for ; Thu, 24 Sep 2015 00:28:21 +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=yy8ekmcX; dkim-atps=neutral Received: from localhost ([::1]:48148 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zel1w-0004Vm-2B for incoming@patchwork.ozlabs.org; Wed, 23 Sep 2015 10:28:20 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37834) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zel0a-00027g-K9 for qemu-devel@nongnu.org; Wed, 23 Sep 2015 10:26:58 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Zel0Y-0005vX-KS for qemu-devel@nongnu.org; Wed, 23 Sep 2015 10:26:56 -0400 Received: from mail-wi0-x22e.google.com ([2a00:1450:400c:c05::22e]:33477) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Zel0Y-0005vL-A1 for qemu-devel@nongnu.org; Wed, 23 Sep 2015 10:26:54 -0400 Received: by wiclk2 with SMTP id lk2so242127819wic.0 for ; Wed, 23 Sep 2015 07:26:53 -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=EOvhxGsNbBG25a9SKJcqCLoWSy1SJiL8LuBESQrHroU=; b=yy8ekmcXkI8k4o1PkUi8QxL2qHYqOdjka0/V3EGVCw5bp7AB+5zb6KvXnl+sCnuMT2 ykERRII9ac/9PPF38+A/ix75Bbo7xV1fBivPgHJ/kH+gsQ5h2RarCAkzZplhfQtz78yc qppTSQertuJj3TLgwsOlrCiCyjE9C4Y0V6MCZI4K+owUZxCI0sHMYE0JVVQRNu9vZzhO Dq1286U0ksI+80b30A4D83K6dfMaao5iaWA3u0ktu/zxRexMWU6jreb3jo0HRuZ9PvUO yeYTMxAN4ipeyd2Iq+wZzD961bbyLCDmBM+WFVcJgl8V/+EVSFBBC95p/Uw2/jRSmDMK ksFg== X-Received: by 10.194.117.70 with SMTP id kc6mr23610293wjb.13.1443018413709; Wed, 23 Sep 2015 07:26:53 -0700 (PDT) Received: from nullptr.home.dirty-ice.org (94-21-30-197.pool.digikabel.hu. [94.21.30.197]) by smtp.gmail.com with ESMTPSA id fv13sm662088wic.7.2015.09.23.07.26.52 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 23 Sep 2015 07:26:52 -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: Wed, 23 Sep 2015 16:27:36 +0200 Message-Id: <3b3b08ea31687178b4c918f3e80792a651499816.1443017811.git.DirtY.iCE.hu@gmail.com> X-Mailer: git-send-email 2.5.2 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::22e Cc: Markus Armbruster , Michael Roth Subject: [Qemu-devel] [PATCH v2 3/3] 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 for Numa and Netdev and NetLegacy ensures that this change doesn't create backward compatibility problems. (Previously OptsVisitor didn't bother with structs and only consulted field names, so something like `data.helper' was invalid previously, and stays so, since unions are flattened now.) 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 | 4 ++ tests/test-opts-visitor.c | 34 ++++++++++ 4 files changed, 141 insertions(+), 22 deletions(-) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index 900b2fa..70dbaf0 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) /* 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 ad37013..3a650c7 100644 --- a/tests/qapi-schema/qapi-schema-test.json +++ b/tests/qapi-schema/qapi-schema-test.json @@ -106,6 +106,11 @@ { 'command': 'boxed', 'box': true, 'data': 'UserDefZero' } { 'command': 'boxed2', 'data': 'UserDefNativeListUnion', 'box': true } +# 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: # @@ -119,7 +124,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 b3b4cb8..83f0d69 100644 --- a/tests/qapi-schema/qapi-schema-test.out +++ b/tests/qapi-schema/qapi-schema-test.out @@ -148,6 +148,10 @@ object UserDefOptions member u16: uint16List optional=True member i64x: int optional=True member u64x: uint64 optional=True + member sub0: UserDefOptionsSub optional=False + member sub1: UserDefOptionsSub optional=False +object UserDefOptionsSub + member nint: int optional=True object UserDefThree base UserDefOne member base: str optional=False 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; }