On Fri, Nov 08, 2019 at 04:52 PM +0100, Pavel Hrdina <phrdina@xxxxxxxxxx> wrote:
> On Fri, Nov 01, 2019 at 06:35:44PM +0100, Marc Hartmayer wrote:
>> The commit 'close callback: move it to driver' (88f09b75eb99) moved
>> the responsibility for the close callback to the driver. But if the
>> driver doesn't support the connectRegisterCloseCallback API this
>> function does nothing, even no unsupported error report. This behavior
>> may lead to problems, for example memory leaks, as the caller cannot
>> differentiate whether the close callback was 'really' registered or
>> not.
>>
>> Therefore call directly @freecb if the connectRegisterCloseCallback
>> API is not supported by the driver used by the connection.
>>
>> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxx>
>> ---
>> src/libvirt-host.c | 12 ++++++++----
>> 1 file changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/src/libvirt-host.c b/src/libvirt-host.c
>> index 221a1b7a4353..9c3a19f33b12 100644
>> --- a/src/libvirt-host.c
>> +++ b/src/libvirt-host.c
>> @@ -1398,7 +1398,7 @@ virConnectIsAlive(virConnectPtr conn)
>> * @conn: pointer to connection object
>> * @cb: callback to invoke upon close
>> * @opaque: user data to pass to @cb
>> - * @freecb: callback to free @opaque
>> + * @freecb: callback to free @opaque when not used anymore
>> *
>> * Registers a callback to be invoked when the connection
>> * is closed. This callback is invoked when there is any
>> @@ -1428,9 +1428,13 @@ virConnectRegisterCloseCallback(virConnectPtr conn,
>> virCheckConnectReturn(conn, -1);
>> virCheckNonNullArgGoto(cb, error);
>>
>> - if (conn->driver->connectRegisterCloseCallback &&
>> - conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb) < 0)
>> - goto error;
>> + if (conn->driver->connectRegisterCloseCallback) {
>> + if (conn->driver->connectRegisterCloseCallback(conn, cb, opaque, freecb) < 0)
>> + goto error;
>> + } else {
>> + if (freecb)
>> + freecb(opaque);
>> + }
>
> Looks good but I think we should improve the documentation of this API
> to explicitly state that the @freecb is called only on success and if
> the API fails the caller is responsible to free the opaque data.
Will change it in v4.
>
> Pavel
--
Kind regards / Beste Grüße
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Matthias Hartmann
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
