From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:33527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SApUZ-0006vo-4c for qemu-devel@nongnu.org; Thu, 22 Mar 2012 17:24:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SApUW-0006bl-TM for qemu-devel@nongnu.org; Thu, 22 Mar 2012 17:24:18 -0400 Received: from mail-pb0-f45.google.com ([209.85.160.45]:46829) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SApUW-0006b2-Kx for qemu-devel@nongnu.org; Thu, 22 Mar 2012 17:24:16 -0400 Received: by pbcuo5 with SMTP id uo5so2264484pbc.4 for ; Thu, 22 Mar 2012 14:24:14 -0700 (PDT) Sender: Paolo Bonzini Message-ID: <4F6B9874.4020108@redhat.com> Date: Thu, 22 Mar 2012 22:24:04 +0100 From: Paolo Bonzini MIME-Version: 1.0 References: <1332417072-20329-1-git-send-email-pbonzini@redhat.com> <1332417072-20329-7-git-send-email-pbonzini@redhat.com> <4F6B8A1C.3080209@codemonkey.ws> In-Reply-To: <4F6B8A1C.3080209@codemonkey.ws> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 06/10] qapi: simplify qmp_input_next_list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Anthony Liguori Cc: anthony@codemonkey.vs, lcapitulino@redhat.com, eblake@redhat.com, qemu-devel@nongnu.org, mdroth@linux.vnet.ibm.com Il 22/03/2012 21:22, Anthony Liguori ha scritto: >> >> Signed-off-by: Paolo Bonzini > > I've been staring a this patch for the past 5 minutes and I can't figure > out what's going on here. > > Maybe the code was too obscure to begin with. Could you enhance the > commit message a bit with what's going on here? There's three possible questions about what's going on: 1) What's going on before the patch with so->entry 2) What's going on after the patch with so->entry 3) What's going on with *list. The patch doesn't change this, it just unties it with so->entry, but it's the most puzzling part so the question is a good one. :) First of all, so->entry here is advanced and also used to figure out whether we have a next element. It is then accessed in qmp_input_get_obj. The caller must: * call start_list * call next_list for each element *including the first* * on the first call to next_list, the result is the head of the list (works for both input and output visitor). Before: so->entry is initialized to qlist_first on start_list. The first call will have *list == NULL, so it skips the qlist_next. The caller assigns the result and makes *list not NULL, so that the next iteration will advance so->entry and modify (*list)->next. After: so->entry is initialized to NULL on start_list. The first call sees so->entry == NULL and does qlist_first; subsequent calls do qlist_next. *list is handled same as above: assignment done by the caller on the first call, done by next_list on the next ones. Thanks for making me write all this down, because it looks like there is still room for further simplifying the handling of *list (for example, do not have the caller write the list head). I'll submit a v2. Depending on the amount of simplification I'll document the result in either a code comment or the commit message. Luckily this code is very well tested. It broke all the time for me. :P Paolo