Re: [PATCH for-5.0] xen-block: Fix uninitialized variable

[Markus Armbruster <armbru@xxxxxxxxxx>]

Anthony PERARD <anthony.perard@xxxxxxxxxx> writes:

> On Mon, Apr 06, 2020 at 06:50:41PM +0200, Philippe Mathieu-Daudé wrote:
>> On 4/6/20 6:42 PM, Anthony PERARD wrote:
>> > Since 7f5d9b206d1e ("object-add: don't create return value if
>> > failed"), qmp_object_add() don't write any value in 'ret_data', thus
>> > has random data. Then qobject_unref() fails and abort().
>> > 
>> > Fix by initialising 'ret_data' properly.
>> 
>> Or move qobject_unref() after the error check?
>> 
>> -- >8 --
>> diff --git a/hw/block/xen-block.c b/hw/block/xen-block.c
>> index 07bb32e22b..f3f1cbef65 100644
>> --- a/hw/block/xen-block.c
>> +++ b/hw/block/xen-block.c
>> @@ -869,7 +869,6 @@ static XenBlockIOThread *xen_block_iothread_create(const
>> char *id,
>>      qdict_put_str(opts, "id", id);
>>      qmp_object_add(opts, &ret_data, &local_err);
>>      qobject_unref(opts);
>> -    qobject_unref(ret_data);
>> 
>>      if (local_err) {
>>          error_propagate(errp, local_err);
>> @@ -878,6 +877,7 @@ static XenBlockIOThread *xen_block_iothread_create(const
>> char *id,
>>          g_free(iothread);
>>          return NULL;
>>      }
>> +    qobject_unref(ret_data);
>
> That won't help, qmp_object_add() doesn't change the value of ret_data
> at all. The other users of qmp_object_add() passes an initialised
> 'ret_data', so we should do the same I think.

Since the QMP core does it, other callers should do it, too.

For QAPI commands that don't return anything, the marshaller should not
use @ret_data, let alone store through it.  qmp_object_add() complies.
Thus, assert(!ret_data) would do.  qobject_unref(ret_data) is also
correct.

Reviewed-by: Markus Armbruster <armbru@xxxxxxxxxx>