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

[Brian Paul]

On 12/05/2015 12:17 PM, Ilia Mirkin wrote:
> 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?

I kept it in case the function were called directly (maybe because a 
non-default ID was desired).


> 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.

I wasn't aware of that.  I can undo that part of the patch if you prefer.


>
> 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.

Do you ever expect someone to call the function directly w/out the macro 
wrapper?

-Brian