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 <arm...@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