> > On 15/03/18 14:20, Christophe Fergeau wrote: > > On Tue, Mar 13, 2018 at 10:37:46AM -0300, Eduardo Lima (Etrunko) wrote: > >> On 13/03/18 04:21, Frediano Ziglio wrote: > >>>> > >>>> This patch makes it clear that this is a configure switch and not a > >>>> variable defined somewhere else in the code. > >>>> > >>> > >>> The code is intended that way to make the compiler always parse > >>> these parts. Note that that define is always defined so your code > >>> is not doing what you are intending. > >> > >> I have sent this patch by mistake, but anyway, the fact of it always > >> being defined is true with autotools, but it is not with meson. > >> > >> Do you think it would make sense to have this patch or is it better to > >> keep as is? If the latter, I think it would be better to keep a static > >> variable and change its value according to the define. > > > > For what it's worth, I tried doing something like what you suggest in > > the past > > https://lists.freedesktop.org/archives/spice-devel/2017-September/039963.html > > but I came to the conclusion that this was not going to work nicely > > https://lists.freedesktop.org/archives/spice-devel/2017-September/039983.html > > I don't fully recall what the problem(s) were though :( > > > > Looks like I missed that discussion, but as there are very few places in > the code that make use of this define, we could make it a static global > to each file that makes the use. > > #ifdef ENABLE_EXTRA_CHECKS > static const int extra_checks = 1; > #else > static const int extra_checks = 0; > #endif > > And then replace accordingly. > What about something like this on spice-common: diff --git a/common/log.h b/common/log.h index 06d48d2..f9e1245 100644 --- a/common/log.h +++ b/common/log.h @@ -93,6 +93,12 @@ void spice_log(GLogLevelFlags log_level, } \ } G_STMT_END +#if ENABLE_EXTRA_CHECKS +enum { extra_checks = 1 }; +#else +enum { extra_checks = 0 }; +#endif + SPICE_END_DECLS #endif /* H_SPICE_LOG */ diff --git a/configure.ac b/configure.ac index 3542161..3da85de 100644 --- a/configure.ac +++ b/configure.ac @@ -27,6 +27,7 @@ fi AM_PROG_CC_C_O SPICE_CHECK_SYSDEPS +SPICE_EXTRA_CHECKS AC_ARG_ENABLE([alignment-checks], AS_HELP_STRING([--enable-alignment-checks], diff --git a/m4/spice-deps.m4 b/m4/spice-deps.m4 index 68e3091..b969c4f 100644 --- a/m4/spice-deps.m4 +++ b/m4/spice-deps.m4 @@ -22,6 +22,14 @@ AC_DEFUN([SPICE_PRINT_MESSAGES],[ IFS="$ac_save_IFS" ]) +AC_DEFUN([SPICE_EXTRA_CHECKS],[ +AC_ARG_ENABLE([extra-checks], + AS_HELP_STRING([--enable-extra-checks=@<:@yes/no@:>@], + [Enable expensive checks @<:@default=no@:>@])) +AM_CONDITIONAL(ENABLE_EXTRA_CHECKS, test "x$enable_extra_checks" = "xyes") +AS_IF([test "x$enable_extra_checks" = "xyes"], + [AC_DEFINE([ENABLE_EXTRA_CHECKS], 1, [Enable extra checks on code])]) +]) # SPICE_CHECK_SYSDEPS() # --------------------- and in spice-server (and possibly spice-gtk too for the configure.ac): diff --git a/configure.ac b/configure.ac index 86383434..2443ccf3 100644 --- a/configure.ac +++ b/configure.ac @@ -71,6 +71,7 @@ esac dnl ========================================================================= dnl Check optional features SPICE_CHECK_SMARTCARD +SPICE_EXTRA_CHECKS AC_ARG_ENABLE(gstreamer, AS_HELP_STRING([--enable-gstreamer=@<:@auto/0.10/1.0/yes/no@:>@], @@ -237,14 +238,6 @@ AC_ARG_ENABLE([statistics], AS_IF([test "$enable_statistics" = "yes"], [AC_DEFINE([RED_STATISTICS], [1], [Enable SPICE statistics])]) -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]) - dnl =========================================================================== dnl check compiler flags diff --git a/server/display-channel.c b/server/display-channel.c index 6dc10ee7..6511419d 100644 --- a/server/display-channel.c +++ b/server/display-channel.c @@ -89,7 +89,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) { unsigned int count; _Drawable *drawable; VideoStream *stream; - the usage of the enum make sure extra_checks is a compile time constant; - the macro/constant can be used also in spice-common; - the --enable-extra-checks is shared between different projects; - ENABLE_EXTRA_CHECKS is defined as 1 or undefined which is more standard; - I think common/log.h is a good place to define that constant as is often used with spice_assert. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel