From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44618) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTkVW-000628-St for qemu-devel@nongnu.org; Tue, 08 Oct 2013 23:32:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VTkVN-00055w-Po for qemu-devel@nongnu.org; Tue, 08 Oct 2013 23:32:18 -0400 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:50786) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VTkVN-00055b-3t for qemu-devel@nongnu.org; Tue, 08 Oct 2013 23:32:09 -0400 Received: from /spool/local by e28smtp06.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Wed, 9 Oct 2013 09:02:03 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp02.in.ibm.com (Postfix) with ESMTP id 37890394004D for ; Wed, 9 Oct 2013 09:01:43 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r993Vt2G39256144 for ; Wed, 9 Oct 2013 09:01:58 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id r993Vvpd000424 for ; Wed, 9 Oct 2013 09:01:57 +0530 Message-ID: <5254CE2A.1050502@linux.vnet.ibm.com> Date: Wed, 09 Oct 2013 11:31:54 +0800 From: Mark Wu MIME-Version: 1.0 References: <1381286396-30051-1-git-send-email-wudxw@linux.vnet.ibm.com> <5254C63C.7040704@redhat.com> In-Reply-To: <5254C63C.7040704@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3] Add interface to traverse the qmp command list by QmpCommand List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: Michael Roth , qemu-devel@nongnu.org, Luiz Capitulino Thanks a lot for your all kind and helpful comments. I would fix it by myself to save the time of maintainer :) Then I have to beg your another review. BTW, I would like to ask if there's good tools or practice to perform an incremental review in qemu patch works. Thanks. Mark. On Wed 09 Oct 2013 10:58:04 AM CST, Eric Blake wrote: > On 10/08/2013 08:39 PM, Mark Wu wrote: >> In the original code, qmp_get_command_list is used to construct >> a list of all commands' name. To get the information of all qga >> commands, it traverses the name list and search the command info >> with its name. So it can cause O(n^2) in the number of commands. >> >> This patch adds an interface to traverse the qmp command list by >> QmpCommand to replace qmp_get_command_list. It can decrease the >> complexity from O(n^2) to O(n). >> >> Signed-off-by: Mark Wu >> --- >> v3: >> Add an accessor for cmd->name to avoid exposing internals of QmpCommand > > As your two patches are related, I'd send them threaded under one cover > letter if you have to respin. But maybe we don't need that... > >> v2: >> 1. Keep the signature of qmp_command_is_enabled (per Eric and Michael) >> 2. Remove the unnecessary pointer castings (per Eric) >> >> include/qapi/qmp/dispatch.h | 6 ++-- >> qapi/qmp-registry.c | 30 +++++------------- >> qga/commands.c | 38 +++++++++-------------- >> qga/main.c | 75 ++++++++++++++++++--------------------------- >> 4 files changed, 57 insertions(+), 92 deletions(-) >> > >> -struct GuestAgentInfo *qmp_guest_info(Error **err) >> +static void qmp_command_info(QmpCommand *cmd, void *opaque) >> { > >> - while (*cmd_list) { >> - cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo)); >> - cmd_info->name = g_strdup(*cmd_list); >> - cmd_info->enabled = qmp_command_is_enabled(cmd_info->name); >> + cmd_info = g_malloc0(sizeof(GuestAgentCommandInfo)); >> + cmd_info->name = g_strdup(cmd->name); > > Oops, not using qmp_command_name(). > > Seems minor enough that if it is the only change after this much churn, > and the maintainer is willing to fix it on your behalf, I could live with: > > Reviewed-by: Eric Blake >