[cairo] [PATCH 6/8] Don't use bare #if on possibly undefined preprocessor symbols

Pavel Roskin proski at gnu.org
Wed Jan 17 08:55:30 PST 2007


Hello, Carl!

On Tue, 2007-01-16 at 16:52 -0800, Carl Worth wrote:
> On Sun, 13 Aug 2006 05:52:03 -0400, Pavel Roskin wrote:
> > On Sun, 2006-08-13 at 04:50 -0400, Behdad Esfahbod wrote:
> > > This one I wanted to do too, but Carl doesn't like it.  The problem with
> > > using ifdef of course is that defining something to zero doesn't turn it
> > > off.
> >
> > Actually, this patch only makes such change for the symbols starting
> > with "HAVE_", which come from Autoconf generated config.h.  It doesn't
> > affect other symbols that may have a different semantic.  I was only
> > getting warnings about "HAVE_" symbols.
> 
> What makes the HAVE_ macros special that they were the only ones you
> were getting warnings about?

I was getting warnings about other symbols as well.  I just wanted to
separate the issues.  It's easier with "HAVE_" symbols - they are 
either defined to 1 or undefined.  I would need to do more checking for
other symbols, so I decided to do it separately.

[skip]
> As Behdad mentioned, I don't want to break the natural means of
> changing that, which would be to leave the #define but change the
> value to 0.
[skip]
> So I see that as a separate problem, with a couple of different
> answers:
> 
> 1. Fix the checkers to restrict the warnings for undefined macros to
>    expressions where the arithmetic value is used, (rather than just
>    the Boolean condition).

I agree.  More precisely, "#if SYMBOL" should be explicitly allowed and
should not trigger any warnings.  It has a specific meaning, and the
other way of achieving the same functionality would be too wordy:

#if defined(SYMBOL) && SYMBOL

Actually, "#if SYMBOL" triggers a warning in gcc itself, but such
warnings need to be specifically requested by -Wundef, which it not
implied by either -Wall or -Wextra.

As for sparse, I'm going to request "#if SYMBOL" to be allowed.  I wrote
some patches for sparse before, so I hope my voice will be heard.

> 2. Avoid using pre-processor arithmetic.
> 
> The pre-processor is a nasty language, there's no doubt. But let's
> bear the pain of that as the developers of the code and not pass it
> on to the users by using #ifdef when we aren't wanting to test whether
> it's defined, but whether it's defined to a non-zero value.

I understand your motives, but users are not supposed to edit config.h
(they should re-run configure), and if they do, they'd better know the
rules.

-- 
Regards,
Pavel Roskin




More information about the cairo mailing list