Re: [Qemu-devel] [PATCH v2 1/3] qom: Don't keep error value between object_property_parse() calls

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2229 bytes --]

On 1/9/19 8:02 PM, Eduardo Habkost wrote:
> When handling errp==NULL at object_apply_global_props(), we are
> leaving the old error value in `err` after printing a warning.
> This makes QEMU crash if two global properties generate warnings:
> 
>   $ echo device_add rtl8139 | qemu-system-x86_64 -monitor stdio -global rtl8139.xxx=yyy -global rtl8139.xxx=zzz
>   warning: can't apply global rtl8139.xxx=yyy: Property '.xxx' not found
>   qemu-system-x86_64: util/error.c:57: error_setv: Assertion `*errp == NULL' failed.
>   Aborted (core dumped)
> 
> Fix that by making `err` go out of scope immediately after the
> warn_report_err() call.
> 
> Fixes: 50545b2cc029 "qdev-props: call object_apply_global_props()"
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  qom/object.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/qom/object.c b/qom/object.c
> index aa6f3a2a71..4e5226ca12 100644
> --- a/qom/object.c
> +++ b/qom/object.c
> @@ -372,7 +372,6 @@ static void object_post_init_with_type(Object *obj, TypeImpl *ti)
>  
>  void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp)
>  {
> -    Error *err = NULL;

Could also have been fixed by leaving this line at this scope,...

>      int i;
>  
>      if (!props) {
> @@ -381,6 +380,7 @@ void object_apply_global_props(Object *obj, const GPtrArray *props, Error **errp
>  
>      for (i = 0; i < props->len; i++) {
>          GlobalProperty *p = g_ptr_array_index(props, i);
> +        Error *err = NULL;
>  
>          if (object_dynamic_cast(obj, p->driver) == NULL) {
>              continue;
> 

...and doing 'err = NULL;' after warn_report_err().  That is, it's not
the going out of scope that fixes it per se, but the fact that you
changed to resetting it to NULL on each loop invocation rather than
leaving it pointing at freed memory.  Whether you set to NULL by a
tighter scope initializer or by an assignment doesn't matter, so no need
to respin since your way works.

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]