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

[Victor Toso]

Hi,

On Fri, Jul 14, 2017 at 12:04:33PM -0400, Frediano Ziglio wrote:
> > 
> > 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.

Sure

> 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.

That depend on how you define 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20170716/993f5bff/attachment.sig>