On Fri, 2015-11-20 at 09:12 -0500, Frediano Ziglio wrote: > > On Fri, 2015-11-20 at 10:50 +0100, Fabiano Fidêncio wrote: > > > On Fri, Nov 20, 2015 at 10:08 AM, Pavel Grunt <pgrunt@xxxxxxxxxx> wrote: > > > > Signed-off-by: Pavel Grunt <pgrunt@xxxxxxxxxx> > > > > --- > > > > Seems that free functions haven't been used since there were introduced > > > > [1] > > > > > > > > [1] > > > > http://lists.freedesktop.org/archives/spice-devel/2015-November/023893.h > > > > tml > > > > --- > > > > server/dcc-encoders.c | 11 +++++++++++ > > > > server/dcc-encoders.h | 1 + > > > > server/red_worker.c | 1 + > > > > 3 files changed, 13 insertions(+) > > > > > > > > diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c > > > > index 90d0ce0..6b764ba 100644 > > > > --- a/server/dcc-encoders.c > > > > +++ b/server/dcc-encoders.c > > > > @@ -406,6 +406,17 @@ void dcc_encoders_init(DisplayChannelClient *dcc) > > > > dcc->zlib_level = ZLIB_DEFAULT_COMPRESSION_LEVEL; > > > > } > > > > > > > > +void dcc_encoders_free(DisplayChannelClient *dcc) > > > > +{ > > > > + quic_destroy(dcc->quic); > > > > + lz_destroy(dcc->lz); > > > > + jpeg_encoder_destroy(dcc->jpeg); > > > > +#ifdef USE_LZ4 > > > > + lz4_encoder_destroy(dcc->lz4); > > > > +#endif > > > > + zlib_encoder_destroy(dcc->zlib); > > > > +} > > > > + > > > > static void marshaller_compress_buf_free(uint8_t *data, void *opaque) > > > > { > > > > compress_buf_free((RedCompressBuf *) opaque); > > > > diff --git a/server/dcc-encoders.h b/server/dcc-encoders.h > > > > index cdb44ac..30f54d5 100644 > > > > --- a/server/dcc-encoders.h > > > > +++ b/server/dcc-encoders.h > > > > @@ -36,6 +36,7 @@ typedef struct RedCompressBuf RedCompressBuf; > > > > typedef struct GlzDrawableInstanceItem GlzDrawableInstanceItem; > > > > > > > > void dcc_encoders_init > > > > (DisplayChanne > > > > lClient *dcc); > > > > +void dcc_encoders_free > > > > (DisplayChanne > > > > lClient *dcc); > > > > void dcc_free_glz_drawable_instance > > > > (DisplayChanne > > > > lClient *dcc, > > > > > > > > GlzDrawableIn > > > > stanceItem *item); > > > > void marshaller_add_compressed > > > > (SpiceMarshall > > > > er *m, > > > > diff --git a/server/red_worker.c b/server/red_worker.c > > > > index 32612d5..cca7f8c 100644 > > > > --- a/server/red_worker.c > > > > +++ b/server/red_worker.c > > > > @@ -5360,6 +5360,7 @@ static void > > > > display_channel_client_on_disconnect(RedChannelClient *rcc) > > > > free(dcc->send_data.stream_outbuf); > > > > free(dcc->send_data.free_list.res); > > > > dcc_destroy_stream_agents(dcc); > > > > + dcc_encoders_free(dcc); > > > > > > > > // this was the last channel client > > > > spice_debug("#draw=%d, #red_draw=%d, #glz_draw=%d", > > > > -- > > > > 2.5.0 > > > > > > > > _______________________________________________ > > > > Spice-devel mailing list > > > > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > > > > http://lists.freedesktop.org/mailman/listinfo/spice-devel > > > > > > Nice catch, ACK. > > > > Pushed, thanks > > > > Pavel > > > > I don't know but I feel some itch... could you set pointers to NULL > after the _destroy ? > It's quite strange as the destruction is not actually perfectly > symmetric. On dcc_new the encoders are created but freed on disconnect. > This lead the paths from on_disconnect to red_channel_client_unref (which > are actually a lot) with dandling pointers. > > I'm missing a virtual destructor here... Remember, at some point later in the branch, ChannelClients are converted to GObject, so there will be a destructor introduced eventually. > > Frediano > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel