[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] qapi: Fix regression with '-netdev ?'
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2] qapi: Fix regression with '-netdev ?' |
Date: |
Thu, 15 Oct 2015 08:24:17 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux) |
Eric Blake <address@hidden> writes:
> Commit e36c714e causes 'qemu -netdev ?' to dump core, because the
> call to visit_end_union() is no longer conditional on whether
> *obj was allocated.
>
> Reported by Marc-André Lureau <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
> ---
> v2: don't depend on unreleased patches
>
> scripts/qapi-visit.py | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
> index 2a9fab8..d0759d7 100644
> --- a/scripts/qapi-visit.py
> +++ b/scripts/qapi-visit.py
> @@ -301,7 +301,9 @@ void visit_type_%(c_name)s(Visitor *v, %(c_name)s **obj,
> const char *name, Error
> out_obj:
> error_propagate(errp, err);
> err = NULL;
> - visit_end_union(v, !!(*obj)->data, &err);
> + if (*obj) {
> + visit_end_union(v, !!(*obj)->data, &err);
> + }
> error_propagate(errp, err);
> err = NULL;
> visit_end_struct(v, &err);
Let's see.
Before commit e36c714e, we generated
visit_start_struct(v, (void **)obj, "%(name)s", name,
sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
if (*obj) {
[...]
out_obj:
error_propagate(errp, err);
err = NULL;
visit_end_union(v, !!(*obj)->data, &err);
error_propagate(errp, err);
err = NULL;
}
visit_end_struct(v, &err);
out:
Since then
visit_start_struct(v, (void **)obj, "%(name)s", name,
sizeof(%(c_name)s), &err);
if (err) {
goto out;
}
if (!*obj) {
goto out_obj; // goto out_end would've been faithful trafo
}
[...]
out_obj:
error_propagate(errp, err); //
err = NULL; // This code became
visit_end_union(v, !!(*obj)->data, &err); // accidentally
error_propagate(errp, err); // unconditional
err = NULL; //
// out_end:
visit_end_struct(v, &err);
out:
error_propagate(errp, err);
We screwed up the if !*obj. Instead of correcting the goto, you exploit
that err is null, and thus the accidentally unconditional code is a
no-op except for the visit_end_union(), so you protect that. Okay.
In case anyone thinks correcting the goto would be nicer: the
visit_end_union() will go away soon.
I'll take this through my tree. Expect a pull request today. Thanks!