From patchwork Thu Jun 18 16:43:47 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: 486423 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 AA0F8140134 for ; Fri, 19 Jun 2015 02:48:40 +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=EDg/jHgN; dkim-atps=neutral Received: from localhost ([::1]:53942 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5czW-0006vN-UC for incoming@patchwork.ozlabs.org; Thu, 18 Jun 2015 12:48:38 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5cv4-0007V9-H7 for qemu-devel@nongnu.org; Thu, 18 Jun 2015 12:44:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z5cv2-0000ta-8S for qemu-devel@nongnu.org; Thu, 18 Jun 2015 12:44:02 -0400 Received: from mail-wg0-x233.google.com ([2a00:1450:400c:c00::233]:34262) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z5cv1-0000tT-Sa for qemu-devel@nongnu.org; Thu, 18 Jun 2015 12:44:00 -0400 Received: by wgfq1 with SMTP id q1so22001625wgf.1 for ; Thu, 18 Jun 2015 09:43: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=kX0A3owzMa0oc21OryVDI3ESQEt4+CxassjmSmFYB/Q=; b=EDg/jHgNowgOEMm5tLpoxs2XacFFu91ISOrKyqHKSPssG+efmvWsV1SppC4bSxqKJh IGYKPl5kk4J+KIkaSgfyNZdhGgNNQ5PI5pZXxqRqHj1l1Xe3Z4LUwgKdy/nZOphfh6Vf 5bD5rva1ZHt5i2pJzxqTkpVAFEkxnnFB4Zu7tKJC4xPHLPwkOZAZI6wevcl4NsY+Gifa VNxadNNWF+XxpQUdH4bCkEKt6ZF5BGm7bhNSdXYtvGfVxQiHSqF9tfGX7qZnaRUPOTfp 5bjAbvaJhCbKf9lm+eNL5Db2BYwRZXH/aaiELJnACAP2NzqicC4IwOyAIUDczhcIJqw8 Bkeg== X-Received: by 10.180.208.7 with SMTP id ma7mr67954094wic.0.1434645839134; Thu, 18 Jun 2015 09:43:59 -0700 (PDT) Received: from nullptr.home.dirty-ice.org (178-164-167-192.pool.digikabel.hu. [178.164.167.192]) by mx.google.com with ESMTPSA id fx7sm13006423wjb.10.2015.06.18.09.43.57 (version=TLSv1.2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 18 Jun 2015 09:43: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: Thu, 18 Jun 2015 18:43:47 +0200 Message-Id: X-Mailer: git-send-email 2.4.3 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:c00::233 Cc: Michael Roth , =?utf-8?B?TMOhc3psw7M=?= Ersek , Gerd Hoffmann , Markus Armbruster Subject: [Qemu-devel] [PATCH v3 5/8] 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 previous two commits hopefully ensures that this change doesn't create backward compatibility problems. Signed-off-by: Kővágó, Zoltán --- Change from v2: * only fully qualified paths are allowed qapi/opts-visitor.c | 114 ++++++++++++++++++++++++++------ tests/qapi-schema/qapi-schema-test.json | 9 ++- tests/test-opts-visitor.c | 34 ++++++++++ 3 files changed, 135 insertions(+), 22 deletions(-) diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c index f02059d..7a80442 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,54 @@ 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 = g_strdup(key); + } - list = g_hash_table_lookup(ov->unprocessed_opts, name); if (!list) { error_set(errp, QERR_MISSING_PARAMETER, name); } + + g_free(key); return list; } @@ -218,7 +267,7 @@ 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); + ov->repeated_opts = lookup_distinct(ov, name, &ov->repeated_name, errp); if (ov->repeated_opts != NULL) { ov->list_mode = LM_STARTED; } @@ -254,11 +303,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 +331,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 +374,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 +392,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 +411,15 @@ opts_type_bool(Visitor *v, bool *obj, const char *name, Error **errp) } else { error_set(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 +431,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 +452,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 +479,7 @@ opts_type_int(Visitor *v, int64_t *obj, const char *name, Error **errp) error_set(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 +491,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 +510,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 +535,7 @@ opts_type_uint64(Visitor *v, uint64_t *obj, const char *name, Error **errp) error_set(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 +546,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 +558,13 @@ opts_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp) if (val < 0 || *endptr) { error_set(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 +575,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 +586,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 +631,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/test-opts-visitor.c b/tests/test-opts-visitor.c index ebeee5d..9199141 100644 --- a/tests/test-opts-visitor.c +++ b/tests/test-opts-visitor.c @@ -177,6 +177,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 @@ -270,6 +298,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; }