Re: [Qemu-devel] [PATCH 3/4] vl: Clean up after previous commit

[Wei Yang <richardw.yang@linux.intel.com>]

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 <armbru@redhat.com>
>---
> 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.

> 
>-    return mc;
>+    return NULL;
> }
> 
> MachineInfoList *qmp_query_machines(Error **errp)
>-- 
>2.17.2

-- 
Wei Yang
Help you, Help me