[Spice-devel] [PATCH spice-common 1/2] Add --enable-extra-checks option

[Christophe de Dinechin]

> On 19 Mar 2018, at 14:46, Frediano Ziglio <fziglio at redhat.com> wrote:
> 
> Allow to enable code to do additional or expensive checks.

“Allow to enable…” -> “Add configuration option enabling expensive checks”

> The option should be used by higher level libraries.
> By default the option is disabled.
> 
> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> ---
> common/log.h     |  6 ++++++
> configure.ac     |  1 +
> m4/spice-deps.m4 | 14 ++++++++++++++
> 3 files changed, 21 insertions(+)
> 
> diff --git a/common/log.h b/common/log.h
> index 06d48d2..e0fd34b 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 { spice_extra_checks = 1 };

I would suggest renaming “extra_checks” to “expensive_checks” if that’s really the intent.
And then, I would avoid bracketing inexpensive checks (like simple asserts) with it, because otherwise it introduces confusion.

Alternatively, if we think that asserts are expensive, then all spice_assert checks should be disabled by this. But I’d rather not.

(I already suggested a spice_hot_assert for assertions in hot code as a better way to express the intent than an if around spice_assert).

> +#else
> +enum { spice_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..a6f4b7b 100644
> --- a/m4/spice-deps.m4
> +++ b/m4/spice-deps.m4
> @@ -23,6 +23,20 @@ AC_DEFUN([SPICE_PRINT_MESSAGES],[
> ])
> 
> 
> +# SPICE_EXTRA_CHECKS()
> +# --------------------
> +# Check for --enable-extra-checks option
> +# --------------------
> +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()
> # ---------------------
> # Checks for header files and library functions needed by spice-common.
> -- 
> 2.14.3
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel