On Tue, 2016-09-20 at 08:18 -0400, Frediano Ziglio wrote:
> >
> >
> > On Mon, 2016-09-19 at 18:35 +0200, Victor Toso wrote:
> > >
> > > Hi,
> > >
> > > On Fri, Sep 16, 2016 at 12:32:54PM +0100, Frediano Ziglio wrote:
> > > >
> > > >
> > > > Allocations are kept into a GList to be able to free in case
> > > > some
> > > > errors happened.
> > > >
> > > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > > > ---
> > > > server/red-replay-qxl.c | 68
> > > > ++++++++++++++++++++++++++++++++++++++++---------
> > > > 1 file changed, 56 insertions(+), 12 deletions(-)
> > > >
> ...
> >
> > >
> > > >
> > > > @@ -1234,9 +1257,28 @@ SPICE_GNUC_VISIBLE QXLCommandExt*
> > > > spice_replay_next_cmd(SpiceReplay *replay,
> > > > info->id = (uintptr_t)cmd;
> > > > }
> > > >
> > > > + /* all buffer allocated will be used by the caller but
> > > > + * free the list of buffer allocated to avoid to free on
> > > > next
> > > > calls */
> > > > + if (replay->allocated) {
> > > > + g_list_free(replay->allocated);
> > > > + replay->allocated = NULL;
> > > > + }
> > > > +
> > > > replay->counter++;
> > > >
> > > > return cmd;
> > > > +
> > > > +error:
> > > > + /* if there were some allocation during the read free all
> > > > + * buffers allocated to avoid leaks */
> > > > + if (replay->allocated) {
> > > > + g_list_free_full(replay->allocated, free);
> > > > + replay->allocated = NULL;
> > > > + }
> > > > + if (cmd) {
> > > > + g_free(cmd);
> >
> > Shouldn't we use spice_replay_free_cmd() here?
> >
> >
> > >
> > > >
> > > > + }
> > > > + return NULL;
> > > > }
> > > >
>
> No, this would result in a double free.
> The buffers are in an unstable state so you could free data pointer
> not initialized
> (that's the reason of the list).
Yes, I see now. A bit non-obvious, to be honest, but it works. If we
also allocate the the cmd variable with replay_malloc, we could just
omit the extra g_free(), I think. I'll send a possible patch. Feel free
to use it or not.
Jonathon
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel