[Spice-devel] [spice-gtk v1 03/10] test-file-transfer: Don't leak SpiceFileTransferTask

Victor Toso lists at victortoso.com
Mon Aug 1 12:54:31 UTC 2016


Hi,

On Mon, Aug 01, 2016 at 12:27:17PM +0200, Christophe Fergeau wrote:
> On Sat, Jul 30, 2016 at 12:26:24AM +0200, Victor Toso wrote:
> > The xfer-task reference given by hash table belongs to the caller so
> 
> "stored in the hash table returned by
> spice_file_transfer_task_create_tasks"
> 
> > we need to release it. Releasing on "finish" callback the same way
> > channel-main does but is possible to do on teardown function too.
> 
> "This commit releases it in the "finish" callback ... but it is possible
> to do it in the teardown function too"

Thanks, I did not pay much attention while writing this.
>
> Not directly related to this commit, but are there places in the code
> that rely on removing an item from the hash table and expecting to own
> a ref on that item?

No

> I could not find such a place. In other words, I don't understand why
> the g_object_unref() is not passed as a parameter to
> g_hash_table_new_full().

I'll change it ;)

>
> Reviewed-by: Christophe Fergeau <cfergeau at redhat.com>
> 
> Christophe
> 
> > ---
> >  tests/file-transfer.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/tests/file-transfer.c b/tests/file-transfer.c
> > index a27b9cf..bf959ac 100644
> > --- a/tests/file-transfer.c
> > +++ b/tests/file-transfer.c
> > @@ -78,6 +78,12 @@ transfer_xfer_task_on_finished(SpiceFileTransferTask *xfer_task,
> >  {
> >      Fixture *f = user_data;
> >  
> > +    if (xfer_task != NULL) {
> > +        guint32 task_id = spice_file_transfer_task_get_id(xfer_task);
> > +        g_hash_table_remove(f->xfer_tasks, GUINT_TO_POINTER(task_id));
> > +        g_object_unref(xfer_task);
> > +    }
> > +
> >      f->num_files_done++;
> >      if (f->num_files == f->num_files_done)
> >          g_main_loop_quit(f->loop);
> > -- 
> > 2.7.4
> > 
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/spice-devel



> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel



More information about the Spice-devel mailing list