Re: src/tools/pginclude considered harmful (was Re:

From: Bruce Momjian <bruce(at)momjian(dot)us>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, pgsql-hackers(at)postgresql(dot)org, Kris Jurka <books(at)ejurka(dot)com>
Subject: Re: src/tools/pginclude considered harmful (was Re:
Date: 2006-07-17 19:43:14
Message-ID: 200607171943.k6HJhEr01850@momjian.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


FYI, I updated pginclude/README to explain the complexity of
removing includes from include files:

pgfixinclude
sort include references
run multiple times:
pgcompinclude
pgrminclude /src/include
pgrminclude /
pgcheckdefines

There is a complexity when modifying /src/include. If include file 1
includes file 2, and file 2 includes file 3, then when file 1 is
processed, it needs only file 2, not file 3. However, if later, include
file 2 is processed, and file 3 is not needed by file 2 and is removed,
file 1 might then need to include file 3. For this reason, the
pgcompinclude and pgrminclude /src/include steps must be run several
times until all includes compile cleanly.

I followed this process, but didn't full understand why multiple include
runs were necessary until I thought about it.

FYI, 527 include were removed from non-header C files in this run. That
is not something that can be easily done manually.

---------------------------------------------------------------------------

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)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

--
Bruce Momjian bruce(at)momjian(dot)us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zdenek Kotala 2006-07-17 19:48:33 Re: TODO: Mark change-on-restart-only values in
Previous Message Josh Berkus 2006-07-17 19:36:44 Re: plPHP and plRuby

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2006-07-17 21:27:36 Re: Possible explanation for Win32 stats regression test
Previous Message korry 2006-07-17 16:33:56 Re: Possible explanation for Win32 stats regression test