From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([209.51.188.92]:35067) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hC4cv-0005Ub-II for qemu-devel@nongnu.org; Thu, 04 Apr 2019 11:50:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hC4cu-0004lV-IH for qemu-devel@nongnu.org; Thu, 04 Apr 2019 11:50:05 -0400 Received: from mx1.redhat.com ([209.132.183.28]:40484) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1hC4cu-0004jQ-AK for qemu-devel@nongnu.org; Thu, 04 Apr 2019 11:50:04 -0400 From: Markus Armbruster References: <20190311060823.18360-1-richardw.yang@linux.intel.com> <20190402132650.23095-1-armbru@redhat.com> <20190403221003.GA18809@richard> Date: Thu, 04 Apr 2019 17:49:52 +0200 In-Reply-To: <20190403221003.GA18809@richard> (Wei Yang's message of "Thu, 4 Apr 2019 06:10:03 +0800") Message-ID: <87d0m16a8v.fsf@dusky.pond.sub.org> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Wei Yang Cc: pbonzini@redhat.com, qemu-devel@nongnu.org, ehabkost@redhat.com Wei Yang writes: > On Tue, Apr 02, 2019 at 03:26:49PM +0200, Markus Armbruster wrote: >>Since the previous commit, find_machine() and find_default_machine() >>don't have to deallocate on return. This permits further >>simplifications. >> >>Signed-off-by: Markus Armbruster >>--- >> vl.c | 25 +++++++------------------ >> 1 file changed, 7 insertions(+), 18 deletions(-) >> >>diff --git a/vl.c b/vl.c >>index 126081f595..6a31e5bfac 100644 >>--- a/vl.c >>+++ b/vl.c >>@@ -1467,40 +1467,29 @@ MachineState *current_machine; >> static MachineClass *find_machine(const char *name, GSList *machines) >> { >> GSList *el; >>- MachineClass *mc = NULL; >> >> for (el = machines; el; el = el->next) { >>- MachineClass *temp = el->data; >>+ MachineClass *mc = el->data; >> >>- if (!strcmp(temp->name, name)) { >>- mc = temp; >>- break; >>- } >>- if (temp->alias && >>- !strcmp(temp->alias, name)) { >>- mc = temp; >>- break; >>+ if (!strcmp(mc->name, name) || !g_strcmp0(mc->alias, name)) { >>+ return mc; >> } >> } >> >>- return mc; >>+ return NULL; >> } >> >> static MachineClass *find_default_machine(GSList *machines) >> { >> GSList *el; >>- MachineClass *mc = NULL; >> >> for (el = machines; el; el = el->next) { >>- MachineClass *temp = el->data; >>- >>- if (temp->is_default) { >>- mc = temp; >>- break; >>+ if (((MachineClass *)el->data)->is_default) { >>+ return el->data; >> } >> } > > Generally it looks good to me. > > One tiny suggestion here is to define > > MachineClass *mc = el->data; > > just as it does in find_machin() and return mc instead of raw el->data. > > If you agree with that I will modify this at my place. No objection. >> >>- return mc; >>+ return NULL; >> } >> >> MachineInfoList *qmp_query_machines(Error **errp) >>-- >>2.17.2