| Message ID | 20181008173125.19678-20-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | Replace some unwise uses of error_report() & friends | expand |
On 08/10/2018 19:31, Markus Armbruster wrote: > Calling error_report() in a function that takes an Error ** argument > is suspicious. chardev_init_func() does that, and then fails without > setting an error. Its caller main(), via qemu_opts_foreach(), is fine > with it, but clean it up anyway. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> > --- > vl.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/vl.c b/vl.c > index b8576f8f10..ecb70f87d8 100644 > --- a/vl.c > +++ b/vl.c > @@ -2239,7 +2239,7 @@ static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp) > > if (!qemu_chr_new_from_opts(opts, &local_err)) { > if (local_err) { > - error_report_err(local_err); > + error_propagate(errp, local_err); > return -1; > } > exit(0); > @@ -4185,10 +4185,8 @@ int main(int argc, char **argv, char **envp) > user_creatable_add_opts_foreach, > object_create_initial, &error_fatal); > > - if (qemu_opts_foreach(qemu_find_opts("chardev"), > - chardev_init_func, NULL, NULL)) { > - exit(1); > - } > + qemu_opts_foreach(qemu_find_opts("chardev"), > + chardev_init_func, NULL, &error_fatal); > > #ifdef CONFIG_VIRTFS > if (qemu_opts_foreach(qemu_find_opts("fsdev"), >
Hi On Mon, Oct 8, 2018 at 9:51 PM Markus Armbruster <armbru@redhat.com> wrote: > > Calling error_report() in a function that takes an Error ** argument > is suspicious. chardev_init_func() does that, and then fails without > setting an error. Its caller main(), via qemu_opts_foreach(), is fine > with it, but clean it up anyway. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> commit title is wrong, apart from that: Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > vl.c | 8 +++----- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/vl.c b/vl.c > index b8576f8f10..ecb70f87d8 100644 > --- a/vl.c > +++ b/vl.c > @@ -2239,7 +2239,7 @@ static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp) > > if (!qemu_chr_new_from_opts(opts, &local_err)) { > if (local_err) { > - error_report_err(local_err); > + error_propagate(errp, local_err); > return -1; > } > exit(0); > @@ -4185,10 +4185,8 @@ int main(int argc, char **argv, char **envp) > user_creatable_add_opts_foreach, > object_create_initial, &error_fatal); > > - if (qemu_opts_foreach(qemu_find_opts("chardev"), > - chardev_init_func, NULL, NULL)) { > - exit(1); > - } > + qemu_opts_foreach(qemu_find_opts("chardev"), > + chardev_init_func, NULL, &error_fatal); > > #ifdef CONFIG_VIRTFS > if (qemu_opts_foreach(qemu_find_opts("fsdev"), > -- > 2.17.1 > >
Marc-André Lureau <marcandre.lureau@gmail.com> writes: > Hi > > On Mon, Oct 8, 2018 at 9:51 PM Markus Armbruster <armbru@redhat.com> wrote: >> >> Calling error_report() in a function that takes an Error ** argument >> is suspicious. chardev_init_func() does that, and then fails without >> setting an error. Its caller main(), via qemu_opts_foreach(), is fine >> with it, but clean it up anyway. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> > > commit title is wrong, apart from that: Fixing... > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Thanks!
diff --git a/vl.c b/vl.c index b8576f8f10..ecb70f87d8 100644 --- a/vl.c +++ b/vl.c @@ -2239,7 +2239,7 @@ static int chardev_init_func(void *opaque, QemuOpts *opts, Error **errp) if (!qemu_chr_new_from_opts(opts, &local_err)) { if (local_err) { - error_report_err(local_err); + error_propagate(errp, local_err); return -1; } exit(0); @@ -4185,10 +4185,8 @@ int main(int argc, char **argv, char **envp) user_creatable_add_opts_foreach, object_create_initial, &error_fatal); - if (qemu_opts_foreach(qemu_find_opts("chardev"), - chardev_init_func, NULL, NULL)) { - exit(1); - } + qemu_opts_foreach(qemu_find_opts("chardev"), + chardev_init_func, NULL, &error_fatal); #ifdef CONFIG_VIRTFS if (qemu_opts_foreach(qemu_find_opts("fsdev"),
Calling error_report() in a function that takes an Error ** argument is suspicious. chardev_init_func() does that, and then fails without setting an error. Its caller main(), via qemu_opts_foreach(), is fine with it, but clean it up anyway. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- vl.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)