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 <pet...@redhat.com>
>>> 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 <th...@redhat.com>
