[Spice-devel] [PATCH v2 17/19] Allows reds_core_timer_remove to accept NULL for timer

[Pavel Grunt]

On Fri, 2016-11-25 at 14:52 +0000, Frediano Ziglio wrote:
> Most of the times the check is done externally
> so moving inside the function reduce the code.
> This is similar to the way free(3) works.
> 
OK

> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  server/char-device.c         | 20 ++++++++------------
>  server/inputs-channel.c      |  5 ++---
>  server/main-channel-client.c |  4 +---
>  server/reds.c                |  4 +++-
>  4 files changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/server/char-device.c b/server/char-device.c
> index 3b70066..45ce548 100644
> --- a/server/char-device.c
> +++ b/server/char-device.c
> @@ -176,10 +176,8 @@ static void
> red_char_device_client_free(RedCharDevice *dev,
>  {
>      GList *l, *next;
>  
> -    if (dev_client->wait_for_tokens_timer) {
> -        reds_core_timer_remove(dev->priv->reds, dev_client-
> >wait_for_tokens_timer);
> -        dev_client->wait_for_tokens_timer = NULL;
> -    }
> +    reds_core_timer_remove(dev->priv->reds, dev_client-
> >wait_for_tokens_timer);
> +    dev_client->wait_for_tokens_timer = NULL;
>  
>      g_queue_free_full(dev_client->send_queue,
> (GDestroyNotify)red_pipe_item_unref);
>  
> @@ -990,10 +988,9 @@ static void
> red_char_device_init_device_instance(RedCharDevice *self)
>  
>      g_return_if_fail(self->priv->reds);
>  
> -    if (self->priv->write_to_dev_timer) {
> -        reds_core_timer_remove(self->priv->reds, self->priv-
> >write_to_dev_timer);
> -        self->priv->write_to_dev_timer = NULL;
> -    }
> +    reds_core_timer_remove(self->priv->reds, self->priv-
> >write_to_dev_timer);
> +    self->priv->write_to_dev_timer = NULL;
> +
>      if (self->priv->sin == NULL) {
>         return;
>      }
> @@ -1081,10 +1078,9 @@ red_char_device_finalize(GObject *object)
>  {
>      RedCharDevice *self = RED_CHAR_DEVICE(object);
>  
> -    if (self->priv->write_to_dev_timer) {
> -        reds_core_timer_remove(self->priv->reds, self->priv-
> >write_to_dev_timer);
> -        self->priv->write_to_dev_timer = NULL;
> -    }
> +    reds_core_timer_remove(self->priv->reds, self->priv-
> >write_to_dev_timer);
> +    self->priv->write_to_dev_timer = NULL;
> +
>      write_buffers_queue_free(&self->priv->write_queue);
>      write_buffers_queue_free(&self->priv->write_bufs_pool);
>      self->priv->cur_pool_size = 0;
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index 99c2888..bea0ddf 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -625,9 +625,8 @@ inputs_channel_finalize(GObject *object)
>      InputsChannel *self = INPUTS_CHANNEL(object);
>      RedsState *reds = red_channel_get_server(RED_CHANNEL(self));
>  
> -    if (self->key_modifiers_timer) {
> -        reds_core_timer_remove(reds, self->key_modifiers_timer);
> -    }
> +    reds_core_timer_remove(reds, self->key_modifiers_timer);
> +
>      G_OBJECT_CLASS(inputs_channel_parent_class)->finalize(object);
>  }
>  
> diff --git a/server/main-channel-client.c b/server/main-channel-
> client.c
> index 576b31f..15e168d 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -189,9 +189,7 @@ static void main_channel_client_finalize(GObject
> *object)
>      RedsState *reds =
>          red_channel_get_server(red_channel_client_get_channel(RED_C
> HANNEL_CLIENT(object)));
>  
> -    if (self->priv->ping_timer) {
> -        reds_core_timer_remove(reds, self->priv->ping_timer);
> -    }
> +    reds_core_timer_remove(reds, self->priv->ping_timer);
>  #endif
>      G_OBJECT_CLASS(main_channel_client_parent_class)-
> >finalize(object);
>  }
> diff --git a/server/reds.c b/server/reds.c
> index 553e7a8..28127e8 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -4214,7 +4214,9 @@ void reds_core_timer_remove(RedsState *reds,
>     g_return_if_fail(reds != NULL);
>     g_return_if_fail(reds->core.timer_remove != NULL);
>  
> -   return reds->core.timer_remove(&reds->core, timer);
> +    if (timer) {
> +        reds->core.timer_remove(&reds->core, timer);
> +    }

I would return early before the g_return_if_fail checks - see fixup

Besides that - Ack

Pavel

>  }
>  
>  void reds_update_client_mouse_allowed(RedsState *reds)