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

Submitted by Frediano Ziglio on Nov. 25, 2016, 2:52 p.m.

Details

Message ID 7cda5c39ca5923dc8540f961b2710611058a8806.1480085518.git-series.fziglio@redhat.com
State New
Headers show
Series "Start cleaning objects on destruction" ( rev: 3 2 ) in Spice

Not browsing as part of any series.

Commit Message

Frediano Ziglio Nov. 25, 2016, 2:52 p.m.
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.

Signed-off-by: Frediano Ziglio <fziglio@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(-)

Patch hide | download patch | download mbox

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_CHANNEL_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);
+    }
 }
 
 void reds_update_client_mouse_allowed(RedsState *reds)

Comments

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@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)