[Spice-devel] [PATCH spice-gtk 1/2] display: factor out initialization of stream array

Frediano Ziglio fziglio at redhat.com
Fri Jul 14 16:04:33 UTC 2017


> 
> From: Victor Toso <me at victortoso.com>
> 
> Including some comment about current implementation of stream-id value
> in the Spice.
> 
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
>  src/channel-display.c | 42 +++++++++++++++++++++++++++++++-----------
>  1 file changed, 31 insertions(+), 11 deletions(-)
> 
> diff --git a/src/channel-display.c b/src/channel-display.c
> index 06c503c..9ae2851 100644
> --- a/src/channel-display.c
> +++ b/src/channel-display.c
> @@ -105,6 +105,7 @@ static void channel_set_handlers(SpiceChannelClass
> *klass);
>  
>  static void clear_surfaces(SpiceChannel *channel, gboolean keep_primary);
>  static void clear_streams(SpiceChannel *channel);
> +static void streams_check_init(SpiceChannel *channel, guint stream_id);
>  static display_surface *find_surface(SpiceDisplayChannelPrivate *c, guint32
>  surface_id);
>  static void spice_display_channel_reset(SpiceChannel *channel, gboolean
>  migrating);
>  static void spice_display_channel_reset_capabilities(SpiceChannel *channel);
> @@ -1233,17 +1234,7 @@ static void display_handle_stream_create(SpiceChannel
> *channel, SpiceMsgIn *in)
>  
>      CHANNEL_DEBUG(channel, "%s: id %u", __FUNCTION__, op->id);
>  
> -    if (op->id >= c->nstreams) {
> -        int n = c->nstreams;
> -        if (!c->nstreams) {
> -            c->nstreams = 1;
> -        }
> -        while (op->id >= c->nstreams) {
> -            c->nstreams *= 2;
> -        }
> -        c->streams = realloc(c->streams, c->nstreams *
> sizeof(c->streams[0]));
> -        memset(c->streams + n, 0, (c->nstreams - n) *
> sizeof(c->streams[0]));
> -    }
> +    streams_check_init(channel, op->id);
>      g_return_if_fail(c->streams[op->id] == NULL);
>  
>      c->streams[op->id] = display_stream_create(channel, op->surface_id,
> @@ -1593,6 +1584,35 @@ static void clear_streams(SpiceChannel *channel)
>      c->nstreams = 0;
>  }
>  
> +/* Always called on SPICE_MSG_DISPLAY_STREAM_CREATE to verify if the
> c->streams
> + * array is big enough to handle the new stream. This obviously takes in
> + * consideration that the stream_id could grow overtime from 0 to
> MAX_STREAMS
> + * value but on server side, the id is picked in descending order, starting
> with
> + * (MAX_STREAMS - 1). As the id itself is not enforced by the protocol, we
> + * should keep the current check to avoid regressions or other unknown
> issues.
> + */

MAX_STREAMS? Are you speaking of NUM_STREAMS in spice-server code?
Could be hard to maintain the consistency here if the server code changes.
Also is it worth documenting the allocation policy here? As far as
the client is concerned is not that useful, the only important thing is
that is unique.

> +static void streams_check_init(SpiceChannel *channel, guint stream_id)
> +{
> +    SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> +    gint old, new;
> +
> +    if (stream_id < c->nstreams) {
> +        /* No need to increase c->streams */
> +        return;
> +    }
> +
> +    old = c->nstreams;
> +    new = (c->nstreams == 0) ? 1 : c->nstreams;
> +
> +    while (stream_id > new) {
> +        new *= 2;
> +    }
> +
> +    c->nstreams = new;
> +    c->streams = realloc(c->streams, new * sizeof(c->streams[0]));
> +    memset(c->streams + old, 0, (new - old) * sizeof(c->streams[0]));
> +}
> +
>  /* coroutine context */
>  static void display_handle_stream_destroy(SpiceChannel *channel, SpiceMsgIn
>  *in)
>  {

Frediano


More information about the Spice-devel mailing list