Message ID | 1455303747-19776-3-git-send-email-ehabkost@redhat.com |
---|---|
State | New |
Headers | show |
On 02/12/2016 09:02 PM, Eduardo Habkost wrote: > After looping through all command-line arguments, error location > info becomes obsolete, and any function calling error_report() > will print misleading information. This breaks error reporting > for some option handling, like: > > $ qemu-system-x86_64 -icount rr=x -vnc :0 > qemu-system-x86_64: -vnc :0: Invalid icount rr option: x > > $ qemu-system-x86_64 -m size= -vnc :0 > qemu-system-x86_64: -vnc :0: missing 'size' option value > > Fix this by resetting location info as soon as we exit the > command-line handling loop. > > With this, replay_configure() and set_memory_options() won't > print any location info yet, but at least they won't print > incorrect information. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > vl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/vl.c b/vl.c > index afbf13f..50cd018 100644 > --- a/vl.c > +++ b/vl.c > @@ -4053,14 +4053,14 @@ int main(int argc, char **argv, char **envp) > } > } > > + loc_set_none(); > + > replay_configure(icount_opts); > > set_machine_options(&machine_class); > > set_memory_options(&ram_slots, &maxram_size, machine_class); > > - loc_set_none(); > - > os_daemonize(); > > if (qemu_init_main_loop(&main_loop_err)) { > Reviewed-by: Marcel Apfelbaum <marcel@redhat.com> Thanks, Marcel
Eduardo Habkost <ehabkost@redhat.com> writes: > After looping through all command-line arguments, error location > info becomes obsolete, and any function calling error_report() > will print misleading information. This breaks error reporting > for some option handling, like: > > $ qemu-system-x86_64 -icount rr=x -vnc :0 > qemu-system-x86_64: -vnc :0: Invalid icount rr option: x > > $ qemu-system-x86_64 -m size= -vnc :0 > qemu-system-x86_64: -vnc :0: missing 'size' option value > > Fix this by resetting location info as soon as we exit the > command-line handling loop. > > With this, replay_configure() and set_memory_options() won't > print any location info yet, but at least they won't print > incorrect information. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > vl.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/vl.c b/vl.c > index afbf13f..50cd018 100644 > --- a/vl.c > +++ b/vl.c > @@ -4053,14 +4053,14 @@ int main(int argc, char **argv, char **envp) > } > } > > + loc_set_none(); > + When I added loc_set_none() in commit 0f0bc3f, I intentionally put no empty line between the loop's closing brace and the loc_set_none(), to reduce the chance of people sticking something in between unthinkingly. It failed. Let's try again with a billboard: } /* * Clear error location left behind by the loop. * Best done right after the loop. Do not insert code here! */ loc_set_none() Can do that when I apply to error-next. > replay_configure(icount_opts); > > set_machine_options(&machine_class); > > set_memory_options(&ram_slots, &maxram_size, machine_class); > > - loc_set_none(); > - > os_daemonize(); > > if (qemu_init_main_loop(&main_loop_err)) {
On Mon, Feb 15, 2016 at 11:29:44AM +0100, Markus Armbruster wrote: > Eduardo Habkost <ehabkost@redhat.com> writes: > > > After looping through all command-line arguments, error location > > info becomes obsolete, and any function calling error_report() > > will print misleading information. This breaks error reporting > > for some option handling, like: > > > > $ qemu-system-x86_64 -icount rr=x -vnc :0 > > qemu-system-x86_64: -vnc :0: Invalid icount rr option: x > > > > $ qemu-system-x86_64 -m size= -vnc :0 > > qemu-system-x86_64: -vnc :0: missing 'size' option value > > > > Fix this by resetting location info as soon as we exit the > > command-line handling loop. > > > > With this, replay_configure() and set_memory_options() won't > > print any location info yet, but at least they won't print > > incorrect information. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > --- > > vl.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/vl.c b/vl.c > > index afbf13f..50cd018 100644 > > --- a/vl.c > > +++ b/vl.c > > @@ -4053,14 +4053,14 @@ int main(int argc, char **argv, char **envp) > > } > > } > > > > + loc_set_none(); > > + > > When I added loc_set_none() in commit 0f0bc3f, I intentionally put no > empty line between the loop's closing brace and the loc_set_none(), to > reduce the chance of people sticking something in between unthinkingly. > It failed. Let's try again with a billboard: > > } > /* > * Clear error location left behind by the loop. > * Best done right after the loop. Do not insert code here! > */ > loc_set_none() ACK.
diff --git a/vl.c b/vl.c index afbf13f..50cd018 100644 --- a/vl.c +++ b/vl.c @@ -4053,14 +4053,14 @@ int main(int argc, char **argv, char **envp) } } + loc_set_none(); + replay_configure(icount_opts); set_machine_options(&machine_class); set_memory_options(&ram_slots, &maxram_size, machine_class); - loc_set_none(); - os_daemonize(); if (qemu_init_main_loop(&main_loop_err)) {
After looping through all command-line arguments, error location info becomes obsolete, and any function calling error_report() will print misleading information. This breaks error reporting for some option handling, like: $ qemu-system-x86_64 -icount rr=x -vnc :0 qemu-system-x86_64: -vnc :0: Invalid icount rr option: x $ qemu-system-x86_64 -m size= -vnc :0 qemu-system-x86_64: -vnc :0: missing 'size' option value Fix this by resetting location info as soon as we exit the command-line handling loop. With this, replay_configure() and set_memory_options() won't print any location info yet, but at least they won't print incorrect information. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- vl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)