[Spice-devel] [PATCH v3 0/9] Refactor red_channel_client_init_send_data()

[Frediano Ziglio]

> 
> A series of patches refactoring the somewhat-confusing
> red_channel_client_init_send_data() function. The third argument to this
> function is a RedPipeItem and it was never very obvious when or why we should
> pass an item in this parameter. Sometimes callers passed NULL, and sometimes
> they passed an item. It turns out that it's only necessary to pass the item
> to
> this parameter if the pipe item needs to be kept alive until we guarantee
> that
> the message has been sent. We only need to do this when we have marshalled
> some
> data by reference, and that data is owned by the RedPipeItem. To make this
> more
> obvious, I've attempted to refactor things so that we no longer rely on this
> mechanism to keep the data alive, but instead reference the pipe item (or
> other
> structure that owns the referenced data) call _add_by_ref_full() to
> unreference
> the data after the message has been sent.
> 
> Changes in v2:
>  - removed one merged patch
>  - this one compiles
> 
> Changes in v3:
>  - introduce a new patch (#1) that ensures that marshaller data is freed as
> soon as possible after the message is sent. This fixes an assert that
> Frediano
> ran into  on the original patchset because some data was kept alive longer
> than
> expected.
>  - Use Frediano's SET/INIT patch (4) instead of my original approach
>  - squashed a fix (removing unused struct field) from Frediano into last
>  patch.
>  - removed another ACKed and pushed patch
> 

I would ack the entire series beside:
- I cannot ack my own patch;
- I'm not again moving marshaller_free_pipe_item into red-pipe-item.c;
  it's basically an utility for RedPipeItem. RedPipeItem, as the name
  suggest are though to be put into the pipe and we know this pipe
  it's the pipe that goes to the client so has to be marshaled.
  Perhaps marshaller_unref_pipe_item instead of marshaller_free_pipe_item
  would also make sense (I noted it calls a unref and that you
  added also a marshaller_unref_drawable).

> 
> 
> 
> Frediano Ziglio (1):
>   Refactor cursor marshalling for SET, INIT
> 
> Jonathon Jongsma (8):
>   Reset marshaller as soon as a message is sent
>   Avoid passing pipe item to red_channel_client_init_send_data()
>   MainChannel: remove another init_send_data arg
>   Smartcard: Don't pass pipe item to _init_send_data()
>   Spicevmc: don't pass pipe item to init_send_data()
>   DCC: remove more init_send_data() arguments
>   DCC: change how fill_bits() marshalls data by reference
>   Remove third argument from red_channel_client_init_send_data()
> 
>  server/cursor-channel.c             |  44 ++++++---------
>  server/dcc-send.c                   | 108
>  +++++++++++++++++++++++-------------
>  server/inputs-channel-client.c      |   2 +-
>  server/inputs-channel.c             |   6 +-
>  server/main-channel-client.c        |  42 ++++++++------
>  server/red-channel-client-private.h |   2 -
>  server/red-channel-client.c         |  33 +++--------
>  server/red-channel-client.h         |   2 +-
>  server/smartcard-channel-client.c   |  14 ++++-
>  server/smartcard.c                  |   2 +-
>  server/spicevmc.c                   |  20 +++++--
>  11 files changed, 150 insertions(+), 125 deletions(-)
> 

As said, beside by patch

Acked-by: Frediano Ziglio <fziglio at redhat.com>

Frediano