[Spice-devel] [PATCH] reset pointer to RedCharDeviceWriteBuffer calling red_char_device_write_buffer_release

[Frediano Ziglio]

This code make easier to be sure we don't have dangling pointers
resetting in the function which free the structure.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
 server/char-device.c | 21 ++++++++++-----------
 server/char-device.h |  2 +-
 server/reds.c        |  3 ++-
 server/smartcard.c   |  3 +--
 server/spicevmc.c    | 17 ++++++-----------
 5 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/server/char-device.c b/server/char-device.c
index 055b232..b67e192 100644
--- a/server/char-device.c
+++ b/server/char-device.c
@@ -506,9 +506,7 @@ static int red_char_device_write_to_device(RedCharDevice *dev)
         total += n;
         write_len -= n;
         if (!write_len) {
-            RedCharDeviceWriteBuffer *release_buf = dev->priv->cur_write_buf;
-            dev->priv->cur_write_buf = NULL;
-            red_char_device_write_buffer_release(dev, release_buf);
+            red_char_device_write_buffer_release(dev, &dev->priv->cur_write_buf);
             continue;
         }
         dev->priv->cur_write_buf_pos += n;
@@ -650,8 +648,14 @@ void red_char_device_write_buffer_add(RedCharDevice *dev,
 }
 
 void red_char_device_write_buffer_release(RedCharDevice *dev,
-                                          RedCharDeviceWriteBuffer *write_buf)
+                                          RedCharDeviceWriteBuffer **p_write_buf)
 {
+    RedCharDeviceWriteBuffer *write_buf = *p_write_buf;
+    if (!write_buf) {
+        return;
+    }
+    *p_write_buf = NULL;
+
     int buf_origin = write_buf->origin;
     uint32_t buf_token_price = write_buf->token_price;
     RedClient *client = write_buf->client;
@@ -835,14 +839,9 @@ void red_char_device_reset(RedCharDevice *dev)
         ring_remove(item);
         buf = SPICE_CONTAINEROF(item, RedCharDeviceWriteBuffer, link);
         /* tracking the tokens */
-        red_char_device_write_buffer_release(dev, buf);
-    }
-    if (dev->priv->cur_write_buf) {
-        RedCharDeviceWriteBuffer *release_buf = dev->priv->cur_write_buf;
-
-        dev->priv->cur_write_buf = NULL;
-        red_char_device_write_buffer_release(dev, release_buf);
+        red_char_device_write_buffer_release(dev, &buf);
     }
+    red_char_device_write_buffer_release(dev, &dev->priv->cur_write_buf);
 
     RING_FOREACH(client_item, &dev->priv->clients) {
         RedCharDeviceClient *dev_client;
diff --git a/server/char-device.h b/server/char-device.h
index d05b1fd..ff43baa 100644
--- a/server/char-device.h
+++ b/server/char-device.h
@@ -228,7 +228,7 @@ RedCharDeviceWriteBuffer *red_char_device_write_buffer_get_server_no_token(
 void red_char_device_write_buffer_add(RedCharDevice *dev,
                                         RedCharDeviceWriteBuffer *write_buf);
 void red_char_device_write_buffer_release(RedCharDevice *dev,
-                                            RedCharDeviceWriteBuffer *write_buf);
+                                          RedCharDeviceWriteBuffer **p_write_buf);
 
 /* api for specific char devices */
 
diff --git a/server/reds.c b/server/reds.c
index f54534a..f0ebf0c 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1131,9 +1131,10 @@ void reds_release_agent_data_buffer(RedsState *reds, uint8_t *buf)
     }
 
     spice_assert(buf == dev->priv->recv_from_client_buf->buf + sizeof(VDIChunkHeader));
+    /* if we pushed the buffer the buffer is attached to the channel so don't free it */
     if (!dev->priv->recv_from_client_buf_pushed) {
         red_char_device_write_buffer_release(RED_CHAR_DEVICE(reds->agent_dev),
-                                             dev->priv->recv_from_client_buf);
+                                             &dev->priv->recv_from_client_buf);
     }
     dev->priv->recv_from_client_buf = NULL;
     dev->priv->recv_from_client_buf_pushed = FALSE;
diff --git a/server/smartcard.c b/server/smartcard.c
index a42bcd8..e483eec 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -420,8 +420,7 @@ static void smartcard_channel_release_msg_rcv_buf(RedChannelClient *rcc,
     } else {
         if (scc->write_buf) { /* msg hasn't been pushed to the guest */
             spice_assert(scc->write_buf->buf == msg);
-            red_char_device_write_buffer_release(RED_CHAR_DEVICE(scc->smartcard), scc->write_buf);
-            scc->write_buf = NULL;
+            red_char_device_write_buffer_release(RED_CHAR_DEVICE(scc->smartcard), &scc->write_buf);
         }
     }
 }
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 283a8f0..bdaa3c3 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -224,10 +224,8 @@ static void spicevmc_red_channel_client_on_disconnect(RedChannelClient *rcc)
 
     state = spicevmc_red_channel_client_get_state(rcc);
 
-    if (state->recv_from_client_buf) { /* partial message which wasn't pushed to device */
-        red_char_device_write_buffer_release(state->chardev, state->recv_from_client_buf);
-        state->recv_from_client_buf = NULL;
-    }
+    /* partial message which wasn't pushed to device */
+    red_char_device_write_buffer_release(state->chardev, &state->recv_from_client_buf);
 
     if (state->chardev) {
         if (red_char_device_client_exists(state->chardev, rcc->client)) {
@@ -349,10 +347,8 @@ static void spicevmc_red_channel_release_msg_rcv_buf(RedChannelClient *rcc,
 
     switch (type) {
     case SPICE_MSGC_SPICEVMC_DATA:
-        if (state->recv_from_client_buf) { /* buffer wasn't pushed to device */
-            red_char_device_write_buffer_release(state->chardev, state->recv_from_client_buf);
-            state->recv_from_client_buf = NULL;
-        }
+        /* buffer wasn't pushed to device */
+        red_char_device_write_buffer_release(state->chardev, &state->recv_from_client_buf);
         break;
     default:
         free(msg);
@@ -545,9 +541,8 @@ void spicevmc_device_disconnect(RedsState *reds, SpiceCharDeviceInstance *sin)
     /* FIXME */
     state = (SpiceVmcState *)red_char_device_opaque_get((RedCharDevice*)sin->st);
 
-    if (state->recv_from_client_buf) {
-        red_char_device_write_buffer_release(state->chardev, state->recv_from_client_buf);
-    }
+    red_char_device_write_buffer_release(state->chardev, &state->recv_from_client_buf);
+
     /* FIXME */
     red_char_device_destroy((RedCharDevice*)sin->st);
     state->chardev = NULL;
-- 
2.5.5