Re: [spice-server] reds: Call sasl_server_done() when destroying SpiceServer instance

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 19, 2018 at 10:18:47AM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > This call will free most of the memory allocated by sasl_server_init().
> > > It's refcounted so should be safe to call from a library.
> > > ---
> > >  server/reds.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/server/reds.c b/server/reds.c
> > > index 85043a88d..e195ce611 100644
> > > --- a/server/reds.c
> > > +++ b/server/reds.c
> > > @@ -3682,6 +3682,7 @@ SPICE_GNUC_VISIBLE void
> > > spice_server_destroy(SpiceServer *reds)
> > >          g_object_unref(reds->main_dispatcher);
> > >      }
> > >      reds_cleanup_net(reds);
> > > +    sasl_server_done();
> > >      g_clear_object(&reds->agent_dev);
> > >  
> > >      // NOTE: don't replace with g_list_free_full as this function that
> > >      passed callback
> > 
> > Would be great if they document this, see
> > https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_done.html
> > 
> > Note that sasl_server_done is called during spice_server_destroy while
> > sasl_server_done is called during spice_server_init so in case of
> > new + destroy (which should be correct) you just call sasl_server_done
> > without any call to spice_server_init.
> > 
> > Had a brief look at sasl_server_init/sasl_server_done the the counter
> > is not updated atomically, potentially can be racy but is really
> > unlikely and is not up to us to fix it.
> > 
> 
> I agree with the refcount implementation, however at
> https://www.cyrusimap.org/sasl/sasl/reference/manpages/library/sasl_server_init.html#std:saslman-sasl_server_init(3)
> they state
> 
> "sasl_server_init() initializes SASL. It must be called before any calls to sasl_server_start, and only once per process."
> 
> Which seems to indicate the opposite.

Oh well, no big deal, I'm fine with dropping the patch as this does not
show up as 'definitely leaked' in valgrind anyway. 

Christophe

Attachment: signature.asc
Description: PGP signature

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

[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]