[Mesa-dev] [PATCH 1/5] intel: Add a macro for printing a debug warning once.

[Jason Wood]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 10/15/2012 10:41 AM, Eric Anholt wrote:
> Jason Wood <sandain at hotmail.com> writes:
> 
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>> 
>> On 10/12/2012 04:30 PM, Eric Anholt wrote:
>>> There are a number of places where some obscure piece of the
>>> code is not currently worth fixing, and we have some workaround
>>> behavior available.  It's nicer for users to do some lame
>>> workaround than to just assert, but without asserts we never
>>> knew when the workaround was at fault.
>>> 
>>> This should give us a nice compromise: Execute the workaround,
>>> but mention that the obscure workaround was hit. --- 
>>> src/mesa/drivers/dri/intel/intel_context.h |   11 +++++++++++
>>> 1 file changed, 11 insertions(+)
>>> 
>>> diff --git a/src/mesa/drivers/dri/intel/intel_context.h 
>>> b/src/mesa/drivers/dri/intel/intel_context.h index
>>> b732696..eeefadf 100644 ---
>>> a/src/mesa/drivers/dri/intel/intel_context.h +++ 
>>> b/src/mesa/drivers/dri/intel/intel_context.h @@ -483,6 +483,17
>>> @@ extern int INTEL_DEBUG; dbg_printf(__VA_ARGS__);			\ }
>>> while(0)
>>> 
>>> +#define WARN_ONCE(cond, fmt...) do {
>>> \ +   if (unlikely(cond)) {
>>> \ +      static bool _warned = false;
>>> \ +      if (!_warned) {
>>> \
>> 
>> I could be wrong here because I'm not super familiar with how
>> macros work..
>> 
>> It looks like this if statement will _always_ evaluate to true
>> given that you just initialized the value in the previous line.
>> I assume this is not the behavior that you were after.
> 
> The "static" keyword on _warned means that it will have persistant 
> storage that contains the initializer value on library load.
> 

Have you actually tried this to see if your code only warns once?
When I write a little test program to see how this code actually
functions, I get multiple warnings unless I move the
definition/initialization of _warned outside of the WARN_ONCE define
block:

static bool _warned = false;

#define WARN_ONCE(cond, fmt...) do {                            \
   if (unlikely(cond)) {                                        \
      if (!_warned) {                                           \
         fprintf(stderr, "WARNING: ");                          \
         fprintf(stderr, fmt);                                  \
         _warned = true;                                        \
      }                                                         \
   }                                                            \
} while (0)


I admit that as a Perl programmer my C fu is a little weak, so I may
just be adding noise to this list.  If this is the case, I apologize.

On a side note, the "do { ... } while(0)" portion of the code looks
funny to me, and I have to assume that it is some oddball compiler
workaround.  GCC works just fine when this code is wrapped in just
curly braces.

Jason Wood
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://www.enigmail.net/

iQEcBAEBAgAGBQJQfHzPAAoJEKeoMFRQQB/oR0cH/idAP3l7G4j5M3sHAhFzZd3K
LwFJObeuoPQqQ5cqMuV2oobl0UCQm6vOjp2omHhfkoMzY0txXwQZ/mPlozcIKomW
hzwhD/LLPkuemL/FTBUFSNUs+nNJxx86D2mNEq/JF/hijVWN0eXC1yFK6dF6SMLC
PGfSV+c3yVaPoZpH3x1fwfA5fv1WaF3UEgdk/8CLOMbwg2v2anWx+P76eAfhAAfe
sg8Qlty6cKhoFS4MYMG+VnALhsVR8XbZw255i4IpfB/KdJzkac7kA8Aphc7APhM/
UQOR20+ZOnLNlkIqBbPctGwnyqHw8SbfRooEOThmeYw+FjPEUuagJu/EO5mbYdE=
=AgMW
-----END PGP SIGNATURE-----