From patchwork Tue Feb 28 22:11:31 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Markus Armbruster X-Patchwork-Id: 9597015 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1E11B60471 for ; Tue, 28 Feb 2017 22:12:00 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1009F1FF29 for ; Tue, 28 Feb 2017 22:12:00 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 04FB620223; Tue, 28 Feb 2017 22:12:00 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 6EEEA1FF29 for ; Tue, 28 Feb 2017 22:11:59 +0000 (UTC) Received: from localhost ([::1]:37303 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cipzy-0006W8-DU for patchwork-qemu-devel@patchwork.kernel.org; Tue, 28 Feb 2017 17:11:58 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55720) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cipzg-0006UM-R2 for qemu-devel@nongnu.org; Tue, 28 Feb 2017 17:11:42 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cipzf-00036T-Jy for qemu-devel@nongnu.org; Tue, 28 Feb 2017 17:11:40 -0500 Received: from mx1.redhat.com ([209.132.183.28]:43192) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1cipza-000353-Ph; Tue, 28 Feb 2017 17:11:34 -0500 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C12E47F0A3; Tue, 28 Feb 2017 22:11:34 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-55.ams2.redhat.com [10.36.116.55]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id v1SMBW0v017747 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Tue, 28 Feb 2017 17:11:34 -0500 Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 645ED1138647; Tue, 28 Feb 2017 23:11:31 +0100 (CET) From: Markus Armbruster To: Kevin Wolf References: <1488317230-26248-1-git-send-email-armbru@redhat.com> <1488317230-26248-25-git-send-email-armbru@redhat.com> <20170228220241.GD4090@noname.redhat.com> Date: Tue, 28 Feb 2017 23:11:31 +0100 In-Reply-To: <20170228220241.GD4090@noname.redhat.com> (Kevin Wolf's message of "Tue, 28 Feb 2017 23:02:41 +0100") Message-ID: <87tw7e2cfg.fsf@dusky.pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 28 Feb 2017 22:11:34 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: Re: [Qemu-devel] [PATCH v2 24/24] keyval: Support lists X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: pkrempa@redhat.com, qemu-devel@nongnu.org, qemu-block@nongnu.org, mdroth@linux.vnet.ibm.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP Kevin Wolf writes: > Am 28.02.2017 um 22:27 hat Markus Armbruster geschrieben: >> Additionally permit non-negative integers as key components. A >> dictionary's keys must either be all integers or none. If all keys >> are integers, convert the dictionary to a list. The set of keys must >> be [0,N]. >> >> Examples: >> >> * list.1=goner,list.0=null,list.1=eins,list.2=zwei >> is equivalent to JSON [ "null", "eins", "zwei" ] >> >> * a.b.c=1,a.b.0=2 >> is inconsistent: a.b.c clashes with a.b.0 >> >> * list.0=null,list.2=eins,list.2=zwei >> has a hole: list.1 is missing >> >> Similar design flaw as for objects: there is no way to denote an empty >> list. While interpreting "key absent" as empty list seems natural >> (removing a list member from the input string works when there are >> multiple ones, so why not when there's just one), it doesn't work: >> "key absent" already means "optional list absent", which isn't the >> same as "empty list present". >> >> Update the keyval object visitor to use this a.0 syntax in error >> messages rather than the usual a[0]. >> >> Signed-off-by: Markus Armbruster > >> +/* >> + * Listify @cur recursively. >> + * Replace QDicts whose keys are all valid list indexes by QLists. >> + * @key_of_cur is the list of key fragments leading up to @cur. >> + * On success, return either @cur or its replacement. >> + * On failure, store an error through @errp and return NULL. >> + */ >> +static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error **errp) >> +{ >> + GSList key_node; >> + bool has_index, has_member; >> + const QDictEntry *ent; >> + QDict *qdict; >> + QObject *val; >> + char *key; >> + size_t nelt; >> + QObject **elt; >> + int index, max_index, i; >> + QList *list; >> + >> + key_node.next = key_of_cur; >> + >> + /* >> + * Recursively listify @cur's members, and figure out whether @cur >> + * itself is to be listified. >> + */ >> + has_index = false; >> + has_member = false; >> + for (ent = qdict_first(cur); ent; ent = qdict_next(cur, ent)) { >> + if (key_to_index(ent->key, NULL) >= 0) { >> + has_index = true; >> + } else { >> + has_member = true; >> + } >> + >> + qdict = qobject_to_qdict(ent->value); >> + if (!qdict) { >> + continue; >> + } >> + >> + key_node.data = ent->key; >> + val = keyval_listify(qdict, &key_node, errp); >> + if (!val) { >> + return NULL; >> + } >> + if (val != ent->value) { >> + qdict_put_obj(cur, ent->key, val); >> + } >> + } >> + >> + if (has_index && has_member) { >> + key = reassemble_key(key_of_cur); >> + error_setg(errp, "Parameters '%s*' used inconsistently", key); >> + g_free(key); >> + return NULL; >> + } >> + if (!has_index) { >> + return QOBJECT(cur); >> + } >> + >> + /* Copy @cur's values to @elt[] */ >> + nelt = qdict_size(cur) + 1; /* one extra, for use as sentinel */ >> + elt = g_new0(QObject *, nelt); >> + max_index = -1; >> + for (ent = qdict_first(cur); ent; ent = qdict_next(cur, ent)) { >> + index = key_to_index(ent->key, NULL); >> + assert(index >= 0); >> + if (index > max_index) { >> + max_index = index; >> + } >> + /* >> + * We iterate @nelt times. If we get one exceeding @nelt >> + * here, we will put less than @nelt values into @elt[], >> + * triggering the error in the next loop. >> + */ >> + if ((size_t)index >= nelt) { > > I think this needs to be index >= nelt - 1 now... > >> + continue; >> + } >> + /* Even though dict keys are distinct, indexes need not be */ >> + elt[index] = ent->value; >> + } >> + >> + /* >> + * Make a list from @elt[], reporting any missing elements. >> + * If we dropped an index >= nelt in the previous loop, this loop >> + * will run into the sentinel and report index @nelt missing. >> + */ >> + list = qlist_new(); >> + assert(!elt[nelt-1]); /* need the sentinel to be null */ > > ...or this assertion can fail. Yup. Appended fix squashed in and pushed to my public repo. R-by? >> + for (i = 0; i < MIN(nelt, max_index + 1); i++) { >> + if (!elt[i]) { >> + key = reassemble_key(key_of_cur); >> + error_setg(errp, "Parameter '%s%d' missing", key, i); >> + g_free(key); >> + g_free(elt); >> + QDECREF(list); >> + return NULL; >> + } >> + qobject_incref(elt[i]); >> + qlist_append_obj(list, elt[i]); >> + } >> + >> + g_free(elt); >> + return QOBJECT(list); >> +} > > Kevin Reviewed-by: Kevin Wolf diff --git a/util/keyval.c b/util/keyval.c index b57a742..c316aaa 100644 --- a/util/keyval.c +++ b/util/keyval.c @@ -329,7 +329,7 @@ static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error **errp) * here, we will put less than @nelt values into @elt[], * triggering the error in the next loop. */ - if ((size_t)index >= nelt) { + if ((size_t)index >= nelt - 1) { continue; } /* Even though dict keys are distinct, indexes need not be */