[libvirt] [PATCH] event: reference state only when virEventAddTimeout success

[Michal Privoznik]

On 9/19/19 10:02 AM, Xu Yandong wrote:
> Reference state is not necessary when virEventAddTimeout failed,
> this may cause a memory leak, so reference state only when
> virEventAddTimeout success.
> 
> Signed-off-by: Xu Yandong <xuyandong2 at huawei.com>
> ---
>   src/conf/object_event.c | 25 +++++++++++++------------
>   1 file changed, 13 insertions(+), 12 deletions(-)
> 
> diff --git a/src/conf/object_event.c b/src/conf/object_event.c
> index 5d84598d59..ee5def5910 100644
> --- a/src/conf/object_event.c
> +++ b/src/conf/object_event.c
> @@ -891,20 +891,21 @@ virObjectEventStateRegisterID(virConnectPtr conn,
>       virObjectLock(state);
>   
>       if ((state->callbacks->count == 0) &&
> -        (state->timer == -1) &&
> -        (state->timer = virEventAddTimeout(-1,
> -                                           virObjectEventTimer,
> -                                           state,
> -                                           virObjectFreeCallback)) < 0) {
> -        virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> -                       _("could not initialize domain event timer"));
> -        goto cleanup;
> +        (state->timer == -1)) {
> +        if ((state->timer = virEventAddTimeout(-1,
> +                                               virObjectEventTimer,
> +                                               state,
> +                                               virObjectFreeCallback)) < 0) {
> +            virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> +                           _("could not initialize domain event timer"));
> +            goto cleanup;
> +        } else {
> +            /* event loop has one reference, but we need one more for the
> +             * timer's opaque argument */
> +            virObjectRef(state);
> +        }

This doesn't have to be under else branch since there's no way the 
control can return from the above branch.

>       }
>   
> -    /* event loop has one reference, but we need one more for the
> -     * timer's opaque argument */
> -    virObjectRef(state);
> -
>       ret = virObjectEventCallbackListAddID(conn, state->callbacks,
>                                             key, filter, filter_opaque,
>                                             klass, eventID,
> 

I'm rewording the commit message a bit, ACKing and pushing.

Congratulations on your first libvirt contribution.

Michal