[Spice-devel] [spice-server] build-sys: Improve ENABLE_EXTRA_CHECK setting

[Christophe Fergeau]

On Tue, Sep 12, 2017 at 06:06:29AM -0400, Frediano Ziglio wrote:
> > 
> > Currently, ENABLE_EXTRA_CHECK is defined to be 0 or 1, and has to be
> > defined this way, otherwise this will break display-channel.c
> > compilation.
> > This is different from most AC_DEFINE() preprocessor constants which are
> > usually either set to 1, or unset.
> > 
> > This commit switches ENABLE_EXTRA_CHECK setting in configure.ac to the
> > usual way, and defines a second constant to 0/1 depending on whether
> > it's set or not in display-channel.c.
> > 
> > Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
> > ---
> >  configure.ac             | 6 +++---
> >  server/display-channel.c | 8 +++++++-
> >  2 files changed, 10 insertions(+), 4 deletions(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 483dbfdf8..c0ea12b5a 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -237,9 +237,9 @@ 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 otherwise
> > define to 0])
> > +AS_IF([test "x$enable_extra_checks" = xyes],
> > +      AC_DEFINE([ENABLE_EXTRA_CHECKS], [1],
> > +                [Define to 1 to enable extra checks on code otherwise define
> > to 0]))
> 
> comment should be updated
> 
> >  
> >  dnl
> >  ===========================================================================
> >  dnl check compiler flags
> > diff --git a/server/display-channel.c b/server/display-channel.c
> > index f7e36dbb4..5cfc151cd 100644
> > --- a/server/display-channel.c
> > +++ b/server/display-channel.c
> > @@ -80,6 +80,12 @@ display_channel_set_property(GObject *object,
> >      }
> >  }
> >  
> > +#ifdef HAVE_EXTRA_CHECKS
> > +#define EXTRA_CHECKS_ENABLED 1
> > +#else
> > +#define EXTRA_CHECKS_ENABLED 0
> > +#endif
> > +
> 
> 
> This should be defined in a more global header to get reused.

At the moment this is only used in one place, if this gets reused, it
will be a good time to move it.

> 
> >  static void
> >  display_channel_finalize(GObject *object)
> >  {
> > @@ -88,7 +94,7 @@ display_channel_finalize(GObject *object)
> >      display_channel_destroy_surfaces(self);
> >      image_cache_reset(&self->priv->image_cache);
> >  
> > -    if (ENABLE_EXTRA_CHECKS) {
> > +    if (EXTRA_CHECKS_ENABLED) {
> >          unsigned int count;
> >          _Drawable *drawable;
> >          Stream *stream;
> 
> Why not using the same mnemonic? I found confusing having 2 names.

Do you mean redefining it if it's not set?

Christophe