[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Thomas Huth
Subject: Re: [Qemu-devel] [PATCH 06/12] chardev: remove unused 'sioc' variable & cleanup paths
Date: Wed, 16 Jan 2019 07:01:46 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1

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>

reply via email to

[Prev in Thread] Current Thread [Next in Thread]