Hi,
On Thu, Apr 19, 2018 at 03:29:00AM -0400, Frediano Ziglio wrote:
> >
> > From: Victor Toso <me@xxxxxxxxxxxxxx>
> >
> > Using the limit SPICE_MAX_NUM_STREAMS introduced in spice-protocol
> > 0.12.14 to fix the amount of streams we might be working currently.
> >
> > With this change, let's create the streams array on _init() and free
> > it on _finalize() to avoid dealing with too many checks everywhere.
> >
> > Note that this patch removes the nstreams as the array size is fixed.
> >
> > Signed-off-by: Victor Toso <victortoso@xxxxxxxxxx>
> > ---
> > configure.ac | 2 +-
> > src/channel-display.c | 28 +++++++---------------------
> > 2 files changed, 8 insertions(+), 22 deletions(-)
> >
> > diff --git a/configure.ac b/configure.ac
> > index a9a7eb9..768e180 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -86,7 +86,7 @@ AC_CHECK_LIBM
> > AC_SUBST(LIBM)
> >
> > AC_CONFIG_SUBDIRS([spice-common])
> > -PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.13])
> > +PKG_CHECK_MODULES([SPICE_PROTOCOL], [spice-protocol >= 0.12.14])
> >
> > COMMON_CFLAGS='-I${top_builddir}/spice-common/ -I${top_srcdir}/spice-common/
> > ${SPICE_PROTOCOL_CFLAGS}'
> > AC_SUBST(COMMON_CFLAGS)
> > diff --git a/src/channel-display.c b/src/channel-display.c
> > index 5329ed0..50c896d 100644
> > --- a/src/channel-display.c
> > +++ b/src/channel-display.c
> > @@ -64,7 +64,6 @@ struct _SpiceDisplayChannelPrivate {
> > SpiceImageSurfaces image_surfaces;
> > SpiceGlzDecoderWindow *glz_window;
> > display_stream **streams;
> > - int nstreams;
> > gboolean mark;
> > guint mark_false_event_id;
> > GArray *monitors;
> > @@ -169,6 +168,7 @@ static void spice_display_channel_finalize(GObject
> > *object)
> > clear_surfaces(SPICE_CHANNEL(object), FALSE);
> > g_hash_table_unref(c->surfaces);
> > clear_streams(SPICE_CHANNEL(object));
> > + g_clear_pointer(&c->streams, g_free);
> > g_clear_pointer(&c->palettes, cache_free);
> >
> > if (G_OBJECT_CLASS(spice_display_channel_parent_class)->finalize)
> > @@ -862,6 +862,7 @@ static void
> > spice_display_channel_init(SpiceDisplayChannel *channel)
> >
> > c = channel->priv = SPICE_DISPLAY_CHANNEL_GET_PRIVATE(channel);
> >
> > + c->streams = g_new0(display_stream *, SPICE_MAX_NUM_STREAMS);
> > c->surfaces = g_hash_table_new_full(NULL, NULL, NULL, destroy_surface);
> > c->image_cache.ops = &image_cache_ops;
> > c->palette_cache.ops = &palette_cache_ops;
> > @@ -1209,8 +1210,7 @@ static display_stream *get_stream_by_id(SpiceChannel
> > *channel, uint32_t id)
> > {
> > SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> >
> > - if (c != NULL && c->streams != NULL && id < c->nstreams &&
> > - c->streams[id] != NULL) {
> > + if (c != NULL && id < SPICE_MAX_NUM_STREAMS && c->streams[id] != NULL) {
> > return c->streams[id];
> > }
> >
> > @@ -1263,7 +1263,7 @@ static void destroy_stream(SpiceChannel *channel, int
> > id)
> >
> > g_return_if_fail(c != NULL);
> > g_return_if_fail(c->streams != NULL);
> > - g_return_if_fail(c->nstreams > id);
> > + g_return_if_fail(id < SPICE_MAX_NUM_STREAMS);
>
> As int could potentially also be <0, maybe better change these
> IDs to make sure there are unsigned?
Oh, well, yes. Let me do another patch for that, I'll check if
something else might need to be changed too.
Cheers,
toso
>
> >
> > g_clear_pointer(&c->streams[id], display_stream_destroy);
> > }
> > @@ -1274,18 +1274,7 @@ static void display_handle_stream_create(SpiceChannel
> > *channel, SpiceMsgIn *in)
> > SpiceMsgDisplayStreamCreate *op = spice_msg_in_parsed(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]));
> > - }
> > + g_return_if_fail(op->id < SPICE_MAX_NUM_STREAMS);
> > g_return_if_fail(c->streams[op->id] == NULL);
> >
> > c->streams[op->id] = display_stream_create(channel, op->id,
> > op->surface_id,
> > @@ -1474,7 +1463,7 @@ static void
> > display_session_mm_time_reset_cb(SpiceSession *session, gpointer dat
> >
> > CHANNEL_DEBUG(channel, "%s", __FUNCTION__);
> >
> > - for (i = 0; i < c->nstreams; i++) {
> > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
> > display_stream *st;
> >
> > if (c->streams[i] == NULL) {
> > @@ -1625,14 +1614,11 @@ static void display_stream_destroy(gpointer
> > st_pointer)
> >
> > static void clear_streams(SpiceChannel *channel)
> > {
> > - SpiceDisplayChannelPrivate *c = SPICE_DISPLAY_CHANNEL(channel)->priv;
> > int i;
> >
> > - for (i = 0; i < c->nstreams; i++) {
> > + for (i = 0; i < SPICE_MAX_NUM_STREAMS; i++) {
> > destroy_stream(channel, i);
> > }
> > - g_clear_pointer(&c->streams, g_free);
> > - c->nstreams = 0;
> > }
> >
> > /* coroutine context */
>
> Frediano
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel