Re: [PATCH] reset pointer to RedCharDeviceWriteBuffer calling red_char_device_write_buffer_release

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx>

On Mon, May 09, 2016 at 02:21:04PM +0100, Frediano Ziglio wrote:
> 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@xxxxxxxxxx>
> ---
>  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
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]