Re: src/tools/pginclude considered harmful (was Re: [PATCHES] - Mailing list pgsql-hackers

From Tom Lane
Subject Re: src/tools/pginclude considered harmful (was Re: [PATCHES]
Date
Msg-id 4724.1152908699@sss.pgh.pa.us
Whole thread Raw
In response to Re: src/tools/pginclude considered harmful (was Re: [PATCHES]  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: src/tools/pginclude considered harmful (was Re: [PATCHES]  (Andrew Dunstan <andrew@dunslane.net>)
Re: src/tools/pginclude considered harmful (was Re: [PATCHES]  (Martijn van Oosterhout <kleptog@svana.org>)
Re: src/tools/pginclude considered harmful (was Re:  (Bruce Momjian <bruce@momjian.us>)
List pgsql-hackers
Andrew Dunstan <andrew@dunslane.net> writes:
> I agree with reverting. The tool looks pretty broken anyway, with 
> hardcoded paths and all sorts of stuff quite apart from logic problems.

Well, it's only intended to work on Bruce's system, so until someone
else takes over the position of chief gruntwork-doer I don't think the
portability issues are much of a factor.  What I'm worried about at the
moment is the correctness issue.

After some reflection it seems that there is only one case where removal
of a needed include file would not lead to a compiler error or warning,
assuming gcc with ordinary -W settings (notably -Wmissing-prototypes).
That case is exactly what Kris found: removal of a #define that is
tested via #ifdef or #if defined().  (Can anyone think of other cases?)

It's not hard to imagine getting burnt by this through ordinary manual
rearrangement or cleanup of inclusions, so I'm thinking that blaming
pgrminclude may be too hasty.  It'd be worth our time to make a tool to
check for it.  I'm going to work on a Perl script that sucks up all the
#defines in all our .h files and looks for "#ifdef foo" or "defined foo"
where foo is defined in a file not included by the referencing file
(gcc -H looks like the right tool for checking that).  Another thing
such a script could easily do is check for inclusion of system headers
before c.h.
        regards, tom lane


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [PATCHES] putting CHECK_FOR_INTERRUPTS in qsort_comparetup()
Next
From: Zoltan Boszormenyi
Date:
Subject: Re: Three weeks left until feature freeze