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

[Markus Armbruster]

Wei Yang <address@hidden> 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 <address@hidden>
>>---
>> 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