Re: [Qemu-devel] [PATCH 06/12] chardev: remove unused 'sioc' variable & cleanup paths

[Thomas Huth]

On 2019-01-16 06:47, Peter Xu wrote:
> On Wed, Jan 16, 2019 at 06:24:49AM +0100, Thomas Huth wrote:
>> On 2019-01-15 15:52, Daniel P. Berrangé wrote:
>>> The 'sioc' variable in qmp_chardev_open_socket was unused since
>>>
>>>   commit 3e7d4d20d3a528b1ed10b1dc3d83119bfb0c5f24
>>>   Author: Peter Xu <address@hidden>
>>>   Date:   Tue Mar 6 13:33:17 2018 +0800
>>>
>>>     chardev: use chardev's gcontext for async connect
>> [...]
>>> -error:
>>> -    if (sioc) {
>>> -        object_unref(OBJECT(sioc));
>>> -    }
>>
>> So who is doing the object_unref() now in case of errors? That commit
>> did not take care of it ... so it sounds like we could be leaving
>> references behind in case of errors here?
> 
> IMHO it'll be done finally in qemu_chr_socket_connected() no matter
> whether it's succeeded or not:
> 
>     cleanup:
>         object_unref(OBJECT(sioc));
> 
> In other words, I think the old error path is not valid even before
> commit 3e7d4d20d3 because IIUC when reaching the error label the sioc
> should never be set (and if it tries to do an object_unref here it
> would be a real bug).

Right, looking at the qmp_chardev_open_socket() function again (before
the rework in 3e7d4d20d3a5), I see now that all locations that use "goto
error" should have sioc = NULL. So this patch here is fine:

Reviewed-by: Thomas Huth <address@hidden>