From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47490) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGvvQ-0005iD-C9 for qemu-devel@nongnu.org; Fri, 21 Feb 2014 14:38:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WGvvK-0001QX-A1 for qemu-devel@nongnu.org; Fri, 21 Feb 2014 14:38:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:18356) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WGvvK-0001QF-1P for qemu-devel@nongnu.org; Fri, 21 Feb 2014 14:38:14 -0500 Received: from int-mx12.intmail.prod.int.phx2.redhat.com (int-mx12.intmail.prod.int.phx2.redhat.com [10.5.11.25]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s1LJcCkb016454 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 21 Feb 2014 14:38:13 -0500 Message-ID: <5307ABC5.7040906@redhat.com> Date: Fri, 21 Feb 2014 20:40:53 +0100 From: Max Reitz MIME-Version: 1.0 References: <1393006301-22514-1-git-send-email-mreitz@redhat.com> <1393006301-22514-3-git-send-email-mreitz@redhat.com> <53079CE0.1040504@redhat.com> In-Reply-To: <53079CE0.1040504@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/3] qdict: Extract non-QDicts in qdict_array_split() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake , qemu-devel@nongnu.org Cc: Kevin Wolf , Stefan Hajnoczi On 21.02.2014 19:37, Eric Blake wrote: > On 02/21/2014 11:11 AM, Max Reitz wrote: >> Currently, qdict_array_split() only splits off entries with a key prefix >> of "%u.", packing them into a new QDict. This patch makes it support >> entries with the plain key "%u" as well, directly putting them into the >> new QList without creating a QDict. >> >> If there is both an entry with a key of "%u" and other entries with keys >> prefixed "%u." (for the same index), the function simply terminates. >> >> To do this, this patch also adds a static function which tests whether a >> given QDict contains any keys with the given prefix. This is used to test >> whether entries with a key prefixed "%u." do exist in the source QDict >> without modifying it. >> >> Signed-off-by: Max Reitz >> --- >> qobject/qdict.c | 60 +++++++++++++++++++++++++++++++++++++++++++-------------- >> 1 file changed, 46 insertions(+), 14 deletions(-) >> >> +static bool qdict_has_prefixed_entries(const QDict *src, const char *start) >> +{ >> + const QDictEntry *entry; >> + >> + for (entry = qdict_first(src); entry; entry = qdict_next(src, entry)) { >> + if (strstart(entry->key, start, NULL)) { >> + return true; > Note that if called with start="1" and the dict contains a key "10", > this would return true. > >> @@ -617,19 +632,36 @@ void qdict_array_split(QDict *src, QList **dst) >> *dst = qlist_new(); >> >> for (i = 0; i < UINT_MAX; i++) { >> + QObject *subqobj; >> + bool is_subqdict; >> QDict *subqdict; >> - char prefix[32]; >> + char indexstr[32], prefix[32]; >> size_t snprintf_ret; >> >> + snprintf_ret = snprintf(indexstr, 32, "%u", i); >> + assert(snprintf_ret < 32); > This assertion is redundant... Not really, as... >> + >> + subqobj = qdict_get(src, indexstr); >> + ...this qdict_get() may break if we haven't asserted that indexstr is indeed 0-terminated at this point (e.g. by testing that snprintf() didn't use the whole space, as I did). Max >> snprintf_ret = snprintf(prefix, 32, "%u.", i); >> assert(snprintf_ret < 32); > ...if this assertion about a longer string holds true. But it doesn't > hurt my feelings to leave it in. > >> >> - qdict_extract_subqdict(src, &subqdict, prefix); >> - if (!qdict_size(subqdict)) { >> - QDECREF(subqdict); >> + is_subqdict = qdict_has_prefixed_entries(src, prefix); > Thankfully you always test a prefix with a trailing '.', so this is not > a problem in your usage. > > Reviewed-by: Eric Blake >