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;
}
}
- return mc;
+ return NULL;
}
MachineInfoList *qmp_query_machines(Error **errp)
--
2.17.2
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
Wei Yang <richardw.yang@linux.intel.com> 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 <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.
No objection.
>>
>>- return mc;
>>+ return NULL;
>> }
>>
>> MachineInfoList *qmp_query_machines(Error **errp)
>>--
>>2.17.2
Exploit that argument @name is nerver null. Check is_help_option()
first, because that's what we do elsewhere.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
vl.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/vl.c b/vl.c
index 6a31e5bfac..da1af3e10d 100644
--- a/vl.c
+++ b/vl.c
@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
static MachineClass *machine_parse(const char *name, GSList *machines)
{
- MachineClass *mc = NULL;
+ MachineClass *mc;
GSList *el;
- if (name) {
- mc = find_machine(name, machines);
- }
- if (mc) {
- return mc;
- }
- if (name && !is_help_option(name)) {
- error_report("unsupported machine type");
- error_printf("Use -machine help to list supported machines\n");
- } else {
+ if (is_help_option(name)) {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
+ exit(0);
}
-
- exit(!name || !is_help_option(name));
+
+ mc = find_machine(name, machines);
+ if (!mc) {
+ error_report("unsupported machine type");
+ error_printf("Use -machine help to list supported machines\n");
+ exit(1);
+ }
+ return mc;
}
void qemu_add_exit_notifier(Notifier *notify)
--
2.17.2
On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>Exploit that argument @name is nerver null. Check is_help_option()
>first, because that's what we do elsewhere.
>
>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>---
> vl.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
>diff --git a/vl.c b/vl.c
>index 6a31e5bfac..da1af3e10d 100644
>--- a/vl.c
>+++ b/vl.c
>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
>- MachineClass *mc = NULL;
>+ MachineClass *mc;
> GSList *el;
>
>- if (name) {
>- mc = find_machine(name, machines);
>- }
>- if (mc) {
>- return mc;
>- }
>- if (name && !is_help_option(name)) {
>- error_report("unsupported machine type");
>- error_printf("Use -machine help to list supported machines\n");
>- } else {
>+ if (is_help_option(name)) {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
>+ exit(0);
> }
>-
>- exit(!name || !is_help_option(name));
>+
>+ mc = find_machine(name, machines);
>+ if (!mc) {
>+ error_report("unsupported machine type");
>+ error_printf("Use -machine help to list supported machines\n");
>+ exit(1);
>+ }
>+ return mc;
This change looks changed the original behavior.
In original logic, if mc is not NULL, there is no message printed. While now
it rely on is_help_option(). And no it exit when !is_help_option(), while
before this change it exit when is_help_option().
I don't understand the reason behind this. My suggestion is you may split this
patch into two:
1. remove check on name
2. refine the logic with explanations.
> }
>
> void qemu_add_exit_notifier(Notifier *notify)
>--
>2.17.2
--
Wei Yang
Help you, Help me
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>Exploit that argument @name is nerver null. Check is_help_option()
>>first, because that's what we do elsewhere.
>>
>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>---
>> vl.c | 24 +++++++++++-------------
>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>
>>diff --git a/vl.c b/vl.c
>>index 6a31e5bfac..da1af3e10d 100644
>>--- a/vl.c
>>+++ b/vl.c
>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>>- MachineClass *mc = NULL;
>>+ MachineClass *mc;
>> GSList *el;
>>
>>- if (name) {
>>- mc = find_machine(name, machines);
>>- }
>>- if (mc) {
>>- return mc;
>>- }
>>- if (name && !is_help_option(name)) {
>>- error_report("unsupported machine type");
>>- error_printf("Use -machine help to list supported machines\n");
>>- } else {
>>+ if (is_help_option(name)) {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>>+ exit(0);
>> }
>>-
>>- exit(!name || !is_help_option(name));
>>+
>>+ mc = find_machine(name, machines);
>>+ if (!mc) {
>>+ error_report("unsupported machine type");
>>+ error_printf("Use -machine help to list supported machines\n");
>>+ exit(1);
>>+ }
>>+ return mc;
>
> This change looks changed the original behavior.
>
> In original logic, if mc is not NULL, there is no message printed. While now
> it rely on is_help_option(). And no it exit when !is_help_option(), while
> before this change it exit when is_help_option().
>
> I don't understand the reason behind this. My suggestion is you may split this
> patch into two:
>
> 1. remove check on name
> 2. refine the logic with explanations.
Cases:
(1) User asks for help, i.e. is_help_option(name)
(1a) and no machine named @name exists, i.e.
is_help_option(name) && !find_machine(name, machines)
(1b) and a machine named @name exists
is_help_option(name) && find_machine(name, machines)
(2) User asks for a machine that doesn't exist, i.e.
!is_help_option(name) && !find_machine(name, machines)
(3) User asks for a machine that exists, i.e.
!is_help_option(name) && find_machine(name, machines)
Since no machines are called "help" or "?", case (1b) is not actually
possible.
Old code:
static MachineClass *machine_parse(const char *name, GSList *machines)
{
MachineClass *mc = NULL;
GSList *el;
if (name) {
mc = find_machine(name, machines);
}
if (mc) {
return mc;
}
if (name && !is_help_option(name)) {
error_report("unsupported machine type");
error_printf("Use -machine help to list supported machines\n");
} else {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
MachineClass *mc = el->data;
if (mc->alias) {
printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
}
printf("%-20s %s%s%s\n", mc->name, mc->desc,
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
}
exit(!name || !is_help_option(name));
}
Case (1a): print help, exit(0)
Case (1b): return find_machine()
Case (2): report error, exit(1)
Case (3): return find_machine()
New code:
static MachineClass *machine_parse(const char *name, GSList *machines)
{
MachineClass *mc;
GSList *el;
if (is_help_option(name)) {
printf("Supported machines are:\n");
machines = g_slist_sort(machines, machine_class_cmp);
for (el = machines; el; el = el->next) {
MachineClass *mc = el->data;
if (mc->alias) {
printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
}
printf("%-20s %s%s%s\n", mc->name, mc->desc,
mc->is_default ? " (default)" : "",
mc->deprecation_reason ? " (deprecated)" : "");
}
exit(0);
}
mc = find_machine(name, machines);
if (!mc) {
error_report("unsupported machine type");
error_printf("Use -machine help to list supported machines\n");
exit(1);
}
return mc;
}
Case (1a): print help, exit(0)
Case (1b): print help, exit(0)
Case (2): report error, exit(1)
Case (3): return find_machine()
The patch changes "impossible" case (1b). That's intentional (but my
commit message could explain it better).
On Thu, Apr 04, 2019 at 06:05:25PM +0200, Markus Armbruster wrote:
>Wei Yang <richardw.yang@linux.intel.com> writes:
>
>> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>>Exploit that argument @name is nerver null. Check is_help_option()
>>>first, because that's what we do elsewhere.
>>>
>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>---
>>> vl.c | 24 +++++++++++-------------
>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>
>>>diff --git a/vl.c b/vl.c
>>>index 6a31e5bfac..da1af3e10d 100644
>>>--- a/vl.c
>>>+++ b/vl.c
>>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>>
>>> static MachineClass *machine_parse(const char *name, GSList *machines)
>>> {
>>>- MachineClass *mc = NULL;
>>>+ MachineClass *mc;
>>> GSList *el;
>>>
>>>- if (name) {
>>>- mc = find_machine(name, machines);
>>>- }
>>>- if (mc) {
>>>- return mc;
>>>- }
>>>- if (name && !is_help_option(name)) {
>>>- error_report("unsupported machine type");
>>>- error_printf("Use -machine help to list supported machines\n");
>>>- } else {
>>>+ if (is_help_option(name)) {
>>> printf("Supported machines are:\n");
>>> machines = g_slist_sort(machines, machine_class_cmp);
>>> for (el = machines; el; el = el->next) {
>>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>>> mc->is_default ? " (default)" : "",
>>> mc->deprecation_reason ? " (deprecated)" : "");
>>> }
>>>+ exit(0);
>>> }
>>>-
>>>- exit(!name || !is_help_option(name));
>>>+
>>>+ mc = find_machine(name, machines);
>>>+ if (!mc) {
>>>+ error_report("unsupported machine type");
>>>+ error_printf("Use -machine help to list supported machines\n");
>>>+ exit(1);
>>>+ }
>>>+ return mc;
>>
>> This change looks changed the original behavior.
>>
>> In original logic, if mc is not NULL, there is no message printed. While now
>> it rely on is_help_option(). And no it exit when !is_help_option(), while
>> before this change it exit when is_help_option().
>>
>> I don't understand the reason behind this. My suggestion is you may split this
>> patch into two:
>>
>> 1. remove check on name
>> 2. refine the logic with explanations.
>
>Cases:
>
>(1) User asks for help, i.e. is_help_option(name)
>
>(1a) and no machine named @name exists, i.e.
> is_help_option(name) && !find_machine(name, machines)
>
>(1b) and a machine named @name exists
> is_help_option(name) && find_machine(name, machines)
>
>(2) User asks for a machine that doesn't exist, i.e.
> !is_help_option(name) && !find_machine(name, machines)
>
>(3) User asks for a machine that exists, i.e.
> !is_help_option(name) && find_machine(name, machines)
>
>Since no machines are called "help" or "?", case (1b) is not actually
>possible.
>
>Old code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc = NULL;
> GSList *el;
>
> if (name) {
> mc = find_machine(name, machines);
> }
> if (mc) {
> return mc;
> }
> if (name && !is_help_option(name)) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> } else {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> }
>
> exit(!name || !is_help_option(name));
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): return find_machine()
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>New code:
>
> static MachineClass *machine_parse(const char *name, GSList *machines)
> {
> MachineClass *mc;
> GSList *el;
>
> if (is_help_option(name)) {
> printf("Supported machines are:\n");
> machines = g_slist_sort(machines, machine_class_cmp);
> for (el = machines; el; el = el->next) {
> MachineClass *mc = el->data;
> if (mc->alias) {
> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
> }
> printf("%-20s %s%s%s\n", mc->name, mc->desc,
> mc->is_default ? " (default)" : "",
> mc->deprecation_reason ? " (deprecated)" : "");
> }
> exit(0);
> }
>
> mc = find_machine(name, machines);
> if (!mc) {
> error_report("unsupported machine type");
> error_printf("Use -machine help to list supported machines\n");
> exit(1);
> }
> return mc;
> }
>
>Case (1a): print help, exit(0)
>
>Case (1b): print help, exit(0)
>
>Case (2): report error, exit(1)
>
>Case (3): return find_machine()
>
>The patch changes "impossible" case (1b). That's intentional (but my
>commit message could explain it better).
This looks better. Would you mind refine it so that I could send all these
patches in v2.
Or you prefer send it out by our self?
--
Wei Yang
Help you, Help me
Wei Yang <richardw.yang@linux.intel.com> writes:
> On Thu, Apr 04, 2019 at 06:05:25PM +0200, Markus Armbruster wrote:
>>Wei Yang <richardw.yang@linux.intel.com> writes:
>>
>>> On Tue, Apr 02, 2019 at 03:26:50PM +0200, Markus Armbruster wrote:
>>>>Exploit that argument @name is nerver null. Check is_help_option()
>>>>first, because that's what we do elsewhere.
>>>>
>>>>Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>---
>>>> vl.c | 24 +++++++++++-------------
>>>> 1 file changed, 11 insertions(+), 13 deletions(-)
>>>>
>>>>diff --git a/vl.c b/vl.c
>>>>index 6a31e5bfac..da1af3e10d 100644
>>>>--- a/vl.c
>>>>+++ b/vl.c
>>>>@@ -2573,19 +2573,10 @@ static gint machine_class_cmp(gconstpointer a, gconstpointer b)
>>>>
>>>> static MachineClass *machine_parse(const char *name, GSList *machines)
>>>> {
>>>>- MachineClass *mc = NULL;
>>>>+ MachineClass *mc;
>>>> GSList *el;
>>>>
>>>>- if (name) {
>>>>- mc = find_machine(name, machines);
>>>>- }
>>>>- if (mc) {
>>>>- return mc;
>>>>- }
>>>>- if (name && !is_help_option(name)) {
>>>>- error_report("unsupported machine type");
>>>>- error_printf("Use -machine help to list supported machines\n");
>>>>- } else {
>>>>+ if (is_help_option(name)) {
>>>> printf("Supported machines are:\n");
>>>> machines = g_slist_sort(machines, machine_class_cmp);
>>>> for (el = machines; el; el = el->next) {
>>>>@@ -2597,9 +2588,16 @@ static MachineClass *machine_parse(const char *name, GSList *machines)
>>>> mc->is_default ? " (default)" : "",
>>>> mc->deprecation_reason ? " (deprecated)" : "");
>>>> }
>>>>+ exit(0);
>>>> }
>>>>-
>>>>- exit(!name || !is_help_option(name));
>>>>+
>>>>+ mc = find_machine(name, machines);
>>>>+ if (!mc) {
>>>>+ error_report("unsupported machine type");
>>>>+ error_printf("Use -machine help to list supported machines\n");
>>>>+ exit(1);
>>>>+ }
>>>>+ return mc;
>>>
>>> This change looks changed the original behavior.
>>>
>>> In original logic, if mc is not NULL, there is no message printed. While now
>>> it rely on is_help_option(). And no it exit when !is_help_option(), while
>>> before this change it exit when is_help_option().
>>>
>>> I don't understand the reason behind this. My suggestion is you may split this
>>> patch into two:
>>>
>>> 1. remove check on name
>>> 2. refine the logic with explanations.
>>
>>Cases:
>>
>>(1) User asks for help, i.e. is_help_option(name)
>>
>>(1a) and no machine named @name exists, i.e.
>> is_help_option(name) && !find_machine(name, machines)
>>
>>(1b) and a machine named @name exists
>> is_help_option(name) && find_machine(name, machines)
>>
>>(2) User asks for a machine that doesn't exist, i.e.
>> !is_help_option(name) && !find_machine(name, machines)
>>
>>(3) User asks for a machine that exists, i.e.
>> !is_help_option(name) && find_machine(name, machines)
>>
>>Since no machines are called "help" or "?", case (1b) is not actually
>>possible.
>>
>>Old code:
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>> MachineClass *mc = NULL;
>> GSList *el;
>>
>> if (name) {
>> mc = find_machine(name, machines);
>> }
>> if (mc) {
>> return mc;
>> }
>> if (name && !is_help_option(name)) {
>> error_report("unsupported machine type");
>> error_printf("Use -machine help to list supported machines\n");
>> } else {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>> MachineClass *mc = el->data;
>> if (mc->alias) {
>> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
>> }
>> printf("%-20s %s%s%s\n", mc->name, mc->desc,
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>> }
>>
>> exit(!name || !is_help_option(name));
>> }
>>
>>Case (1a): print help, exit(0)
>>
>>Case (1b): return find_machine()
>>
>>Case (2): report error, exit(1)
>>
>>Case (3): return find_machine()
>>
>>New code:
>>
>> static MachineClass *machine_parse(const char *name, GSList *machines)
>> {
>> MachineClass *mc;
>> GSList *el;
>>
>> if (is_help_option(name)) {
>> printf("Supported machines are:\n");
>> machines = g_slist_sort(machines, machine_class_cmp);
>> for (el = machines; el; el = el->next) {
>> MachineClass *mc = el->data;
>> if (mc->alias) {
>> printf("%-20s %s (alias of %s)\n", mc->alias, mc->desc, mc->name);
>> }
>> printf("%-20s %s%s%s\n", mc->name, mc->desc,
>> mc->is_default ? " (default)" : "",
>> mc->deprecation_reason ? " (deprecated)" : "");
>> }
>> exit(0);
>> }
>>
>> mc = find_machine(name, machines);
>> if (!mc) {
>> error_report("unsupported machine type");
>> error_printf("Use -machine help to list supported machines\n");
>> exit(1);
>> }
>> return mc;
>> }
>>
>>Case (1a): print help, exit(0)
>>
>>Case (1b): print help, exit(0)
>>
>>Case (2): report error, exit(1)
>>
>>Case (3): return find_machine()
>>
>>The patch changes "impossible" case (1b). That's intentional (but my
>>commit message could explain it better).
>
> This looks better.
Amending my commit message to explain things better:
vl: Simplify machine_parse()
Exploit that argument @name is nerver null. Check is_help_option()
first, because that's what we do elsewhere. If we (foolishly!)
defined a machine named "help", -machine help would now print help
instead of selecting the machine named "help".
> Would you mind refine it so that I could send all these
> patches in v2.
>
> Or you prefer send it out by our self?
Please send v2.
© 2016 - 2025 Red Hat, Inc.