Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
On Mon, Oct 08, 2018 at 07:31:12PM +0200, Markus Armbruster wrote: > Calling error_report() in a function that takes an Error ** argument > is suspicious. user_creatable_add_opts_foreach() 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. > > Cc: Daniel P. Berrangé > Signed-off-by: Markus Armbruster > --- > qemu-io.c | 8 +++- > qemu-nbd.c | 8 +++- > qom/object_interfaces.c | 4 +--- > vl.c| 16 ++-- > 4 files changed, 13 insertions(+), 23 deletions(-) Reviewed-by: Daniel P. Berrangé Regards, Daniel -- |: https://berrange.com -o-https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o-https://fstop138.berrange.com :| |: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|
Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Hi
On Mon, Oct 8, 2018 at 9:39 PM Markus Armbruster wrote:
>
> Calling error_report() in a function that takes an Error ** argument
> is suspicious. user_creatable_add_opts_foreach() 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.
>
> Cc: Daniel P. Berrangé
> Signed-off-by: Markus Armbruster
Reviewed-by: Marc-André Lureau
> ---
> qemu-io.c | 8 +++-
> qemu-nbd.c | 8 +++-
> qom/object_interfaces.c | 4 +---
> vl.c| 16 ++--
> 4 files changed, 13 insertions(+), 23 deletions(-)
>
> diff --git a/qemu-io.c b/qemu-io.c
> index 13829f5e21..6df7731af4 100644
> --- a/qemu-io.c
> +++ b/qemu-io.c
> @@ -620,11 +620,9 @@ int main(int argc, char **argv)
> exit(1);
> }
>
> -if (qemu_opts_foreach(_object_opts,
> - user_creatable_add_opts_foreach,
> - NULL, NULL)) {
> -exit(1);
> -}
> +qemu_opts_foreach(_object_opts,
> + user_creatable_add_opts_foreach,
> + NULL, _fatal);
>
> if (!trace_init_backends()) {
> exit(1);
> diff --git a/qemu-nbd.c b/qemu-nbd.c
> index 7874bc973c..ca7109652e 100644
> --- a/qemu-nbd.c
> +++ b/qemu-nbd.c
> @@ -766,11 +766,9 @@ int main(int argc, char **argv)
> exit(EXIT_FAILURE);
> }
>
> -if (qemu_opts_foreach(_object_opts,
> - user_creatable_add_opts_foreach,
> - NULL, NULL)) {
> -exit(EXIT_FAILURE);
> -}
> +qemu_opts_foreach(_object_opts,
> + user_creatable_add_opts_foreach,
> + NULL, _fatal);
>
> if (!trace_init_backends()) {
> exit(1);
> diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
> index 72b97a8bed..4052d6c4a7 100644
> --- a/qom/object_interfaces.c
> +++ b/qom/object_interfaces.c
> @@ -143,7 +143,6 @@ int user_creatable_add_opts_foreach(void *opaque,
> QemuOpts *opts, Error **errp)
> {
> bool (*type_predicate)(const char *) = opaque;
> Object *obj = NULL;
> -Error *err = NULL;
> const char *type;
>
> type = qemu_opt_get(opts, "qom-type");
> @@ -152,9 +151,8 @@ int user_creatable_add_opts_foreach(void *opaque,
> QemuOpts *opts, Error **errp)
> return 0;
> }
>
> -obj = user_creatable_add_opts(opts, );
> +obj = user_creatable_add_opts(opts, errp);
> if (!obj) {
> -error_report_err(err);
> return -1;
> }
> object_unref(obj);
> diff --git a/vl.c b/vl.c
> index 7ce8299d9d..b8576f8f10 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -4181,11 +4181,9 @@ int main(int argc, char **argv, char **envp)
> page_size_init();
> socket_init();
>
> -if (qemu_opts_foreach(qemu_find_opts("object"),
> - user_creatable_add_opts_foreach,
> - object_create_initial, NULL)) {
> -exit(1);
> -}
> +qemu_opts_foreach(qemu_find_opts("object"),
> + user_creatable_add_opts_foreach,
> + object_create_initial, _fatal);
>
> if (qemu_opts_foreach(qemu_find_opts("chardev"),
>chardev_init_func, NULL, NULL)) {
> @@ -4316,11 +4314,9 @@ int main(int argc, char **argv, char **envp)
> exit(1);
> }
>
> -if (qemu_opts_foreach(qemu_find_opts("object"),
> - user_creatable_add_opts_foreach,
> - object_create_delayed, NULL)) {
> -exit(1);
> -}
> +qemu_opts_foreach(qemu_find_opts("object"),
> + user_creatable_add_opts_foreach,
> + object_create_delayed, _fatal);
>
> if (tpm_init() < 0) {
> exit(1);
> --
> 2.17.1
>
>
--
Marc-André Lureau
Re: [Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
On 10/8/18 12:31 PM, Markus Armbruster wrote: Calling error_report() in a function that takes an Error ** argument is suspicious. user_creatable_add_opts_foreach() 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. Cc: Daniel P. Berrangé Signed-off-by: Markus Armbruster --- qemu-io.c | 8 +++- qemu-nbd.c | 8 +++- qom/object_interfaces.c | 4 +--- vl.c| 16 ++-- 4 files changed, 13 insertions(+), 23 deletions(-) While I could take this through my NBD tree, I think it makes more sense to go through your error tree with the rest of the series. Reviewed-by: Eric Blake -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
[Qemu-devel] [PATCH 18/31] qom: Clean up error reporting in user_creatable_add_opts_foreach()
Calling error_report() in a function that takes an Error ** argument
is suspicious. user_creatable_add_opts_foreach() 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.
Cc: Daniel P. Berrangé
Signed-off-by: Markus Armbruster
---
qemu-io.c | 8 +++-
qemu-nbd.c | 8 +++-
qom/object_interfaces.c | 4 +---
vl.c| 16 ++--
4 files changed, 13 insertions(+), 23 deletions(-)
diff --git a/qemu-io.c b/qemu-io.c
index 13829f5e21..6df7731af4 100644
--- a/qemu-io.c
+++ b/qemu-io.c
@@ -620,11 +620,9 @@ int main(int argc, char **argv)
exit(1);
}
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- NULL, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, _fatal);
if (!trace_init_backends()) {
exit(1);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 7874bc973c..ca7109652e 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -766,11 +766,9 @@ int main(int argc, char **argv)
exit(EXIT_FAILURE);
}
-if (qemu_opts_foreach(_object_opts,
- user_creatable_add_opts_foreach,
- NULL, NULL)) {
-exit(EXIT_FAILURE);
-}
+qemu_opts_foreach(_object_opts,
+ user_creatable_add_opts_foreach,
+ NULL, _fatal);
if (!trace_init_backends()) {
exit(1);
diff --git a/qom/object_interfaces.c b/qom/object_interfaces.c
index 72b97a8bed..4052d6c4a7 100644
--- a/qom/object_interfaces.c
+++ b/qom/object_interfaces.c
@@ -143,7 +143,6 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts
*opts, Error **errp)
{
bool (*type_predicate)(const char *) = opaque;
Object *obj = NULL;
-Error *err = NULL;
const char *type;
type = qemu_opt_get(opts, "qom-type");
@@ -152,9 +151,8 @@ int user_creatable_add_opts_foreach(void *opaque, QemuOpts
*opts, Error **errp)
return 0;
}
-obj = user_creatable_add_opts(opts, );
+obj = user_creatable_add_opts(opts, errp);
if (!obj) {
-error_report_err(err);
return -1;
}
object_unref(obj);
diff --git a/vl.c b/vl.c
index 7ce8299d9d..b8576f8f10 100644
--- a/vl.c
+++ b/vl.c
@@ -4181,11 +4181,9 @@ int main(int argc, char **argv, char **envp)
page_size_init();
socket_init();
-if (qemu_opts_foreach(qemu_find_opts("object"),
- user_creatable_add_opts_foreach,
- object_create_initial, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("object"),
+ user_creatable_add_opts_foreach,
+ object_create_initial, _fatal);
if (qemu_opts_foreach(qemu_find_opts("chardev"),
chardev_init_func, NULL, NULL)) {
@@ -4316,11 +4314,9 @@ int main(int argc, char **argv, char **envp)
exit(1);
}
-if (qemu_opts_foreach(qemu_find_opts("object"),
- user_creatable_add_opts_foreach,
- object_create_delayed, NULL)) {
-exit(1);
-}
+qemu_opts_foreach(qemu_find_opts("object"),
+ user_creatable_add_opts_foreach,
+ object_create_delayed, _fatal);
if (tpm_init() < 0) {
exit(1);
--
2.17.1
