On Sat, Dec 5, 2015 at 2:48 PM, Brian Paul <bri...@vmware.com> wrote:
> On 12/05/2015 12:17 PM, Ilia Mirkin wrote:
>>
>> On Sat, Dec 5, 2015 at 2:10 PM, Brian Paul <bri...@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
>> <imir...@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?
No, but I see what you're saying. This is all so insignificant, so do
whatever you think is best.
Cheers,
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
