[Mesa-dev] [PATCH 1/2] gallium/util: check callback pointers for non-null in pipe_debug_message()

[Ilia Mirkin]

On Sat, Dec 5, 2015 at 2:10 PM, Brian Paul <brianp at vmware.com> wrote:
> So the callers don't have to do it.
>
> v2: also check cb!=NULL in the macro and move the conditional in
> _pipe_debug_message() to enclose all code.
> ---
>  src/gallium/auxiliary/util/u_debug.c | 9 +++++----
>  src/gallium/auxiliary/util/u_debug.h | 8 +++++---
>  2 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/src/gallium/auxiliary/util/u_debug.c b/src/gallium/auxiliary/util/u_debug.c
> index 7029536..2aa75b4 100644
> --- a/src/gallium/auxiliary/util/u_debug.c
> +++ b/src/gallium/auxiliary/util/u_debug.c
> @@ -77,11 +77,12 @@ _pipe_debug_message(
>     enum pipe_debug_type type,
>     const char *fmt, ...)
>  {
> -   va_list args;
> -   va_start(args, fmt);
> -   if (cb && cb->debug_message)
> +   if (cb && cb->debug_message) {

Shouldn't this if () just have been removed? Separately, I've been
told that the va_* "functions" shouldn't be inside of control flow,
since they're often funky macros, although it does seem like this
ought to work even in that scenario.

Either way, this patch is Reviewed-by: Ilia Mirkin
<imirkin at alum.mit.edu> but with a minor preference to removing the
check entirely here now that it's in the macro.

> +      va_list args;
> +      va_start(args, fmt);
>        cb->debug_message(cb->data, id, type, fmt, args);
> -   va_end(args);
> +      va_end(args);
> +   }
>  }
>
>
> diff --git a/src/gallium/auxiliary/util/u_debug.h b/src/gallium/auxiliary/util/u_debug.h
> index 9900703..5307072 100644
> --- a/src/gallium/auxiliary/util/u_debug.h
> +++ b/src/gallium/auxiliary/util/u_debug.h
> @@ -268,9 +268,11 @@ void _debug_assert_fail(const char *expr,
>   */
>  #define pipe_debug_message(cb, type, fmt, ...) do { \
>     static unsigned id = 0; \
> -   _pipe_debug_message(cb, &id, \
> -                       PIPE_DEBUG_TYPE_ ## type, \
> -                       fmt, ##__VA_ARGS__); \
> +   if ((cb) && (cb)->debug_message) { \
> +      _pipe_debug_message(cb, &id, \
> +                          PIPE_DEBUG_TYPE_ ## type, \
> +                          fmt, ##__VA_ARGS__); \
> +   } \
>  } while (0)
>
>  struct pipe_debug_callback;
> --
> 1.9.1
>