[Spice-devel] [PATCH spice-server] display-channel: Check that all structure are destroyed during finalize

[Jonathon Jongsma]

On Tue, 2017-08-22 at 08:54 +0100, Frediano Ziglio wrote:
> The leak detector we use currently is not enough to detect
> some kind of leak in DisplayChannel so manully test.
> These tests are enabled only when --enable-extra-checks is passed
> to configure.


Out of curiosity, did you find a bug that made you add this extra
check?

> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
>  configure.ac             |  2 ++
>  server/display-channel.c | 22 ++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index e1e74862..7fec66fc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -247,6 +247,8 @@ AC_ARG_ENABLE([extra-checks],
>                 AS_HELP_STRING([--enable-extra-
> checks=@<:@yes/no@:>@],
>                                [Enable expensive checks
> @<:@default=no@:>@]))
>  AM_CONDITIONAL(ENABLE_EXTRA_CHECKS, test "$enable_extra_checks" =
> "yes")
> +AC_DEFINE_UNQUOTED([ENABLE_EXTRA_CHECKS], [$(test
> "x$enable_extra_checks" = xyes && echo 1 || echo 0)],
> +                   [Define to 1 to enable extra checks on code])
>  
>  dnl
> =====================================================================
> ======
>  dnl check compiler flags
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 924d219b..317b02ef 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -80,6 +80,28 @@ display_channel_finalize(GObject *object)
>  
>      display_channel_destroy_surfaces(self);
>      image_cache_reset(&self->priv->image_cache);
> +
> +    if (ENABLE_EXTRA_CHECKS) {
> +        unsigned cnt;
> +        _Drawable *dr;

I'd really prefer full-name variables: "count", "drawable"

> +        Stream *stream;
> +
> +        for (cnt = 0, dr = self->priv->free_drawables; dr; dr = dr-
> >u.next) {
> +            ++cnt;
> +        }
> +        spice_assert(cnt == NUM_DRAWABLES);
> +
> +        for (cnt = 0, stream = self->priv->free_streams; stream;
> stream = stream->next) {
> +            ++cnt;
> +        }
> +        spice_assert(cnt == NUM_STREAMS);
> +        spice_assert(ring_is_empty(&self->priv->streams));
> +
> +        for (cnt = 0; cnt < NUM_SURFACES; ++cnt) {
> +            spice_assert(self->priv->surfaces[cnt].context.canvas ==
> NULL);
> +        }
> +    }
> +
>      monitors_config_unref(self->priv->monitors_config);
>      g_array_unref(self->priv->video_codecs);
>      g_free(self->priv);


otherwise OK with me

Acked-by: Jonathon Jongsma <jjongsma at redhat.com>