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.
Pavel
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
