>
> On Wed, Mar 01, 2017 at 09:57:29AM +0000, Frediano Ziglio wrote:
> > Free security, migration, sasl and name stuff.
> >
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/reds.c | 23 +++++++++++++++++++----
> > 1 file changed, 19 insertions(+), 4 deletions(-)
> >
> > diff --git a/server/reds.c b/server/reds.c
> > index 898be92..eabb63f 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3640,6 +3640,24 @@ SPICE_GNUC_VISIBLE int spice_server_init(SpiceServer
> > *reds, SpiceCoreInterface *
> > return ret;
> > }
> >
> > +static void reds_config_free(SpiceServer *reds)
>
> It's called reds_config_free, so I'd expect to see a RedServerConfig
> argument (you can set reds->config to NULL in the caller).
>
> > +{
> > + reds_mig_release(reds);
yes, this is the reason for the weird argument... I'll try
to change also reds_mig_release.
> > + ChannelSecurityOptions *now = reds->config->channels_security, *next;
>
> I'd rather avoid 2 declarations on the same line, especially if one
> comes with initialization. And I'd keep the variable declarations at the
> top. And let's not introduce new use of 'now', imo it's a very bad name
> for an iterator/the current item (feel free to ignore this last comment
> if you disagree).
>
Usually I use "curr", maybe was some inspiration for some
other code I was looking at that time... I prefer curr, I'll use it.
> Do we have a test case showing at least part of these as leaks?
>
Was thinking to add a unit test. Our tests don't set these stuff.
> Patch looks good to me.
>
> Christophe
>
> > + for (; now; now = next) {
> > + next = now->next;
> > + free(now);
> > + }
> > +#if HAVE_SASL
> > + free(reds->config->sasl_appname);
> > +#endif
> > + free(reds->config->spice_name);
> > + g_array_unref(reds->config->renderers);
> > + g_array_unref(reds->config->video_codecs);
> > + free(reds->config);
> > + reds->config = NULL;
> > +}
> > +
> > SPICE_GNUC_VISIBLE void spice_server_destroy(SpiceServer *reds)
> > {
> > /* remove the server from the list of servers so that we don't attempt
> > to
> > @@ -3668,10 +3686,7 @@ SPICE_GNUC_VISIBLE void
> > spice_server_destroy(SpiceServer *reds)
> > stat_file_free(reds->stat_file);
> > #endif
> >
> > - g_array_unref(reds->config->renderers);
> > - g_array_unref(reds->config->video_codecs);
> > - free(reds->config);
> > -
> > + reds_config_free(reds);
> > free(reds);
> > }
> >
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel