SEO

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Oops :-( I just noticed that I removed bufmgr.h from bufpage.h, which
> is a change you had objected to previously :-(
> http://archives.postgresql.org/pgsql...4/msg00149.php

Hmm. I did notice that the patch seemed to need to add bufmgr.h
inclusions to an awful lot of files, but I'd forgotten the argument
about the bufpage.h macros needing it. Do you want to undo that
aspect of it?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@commandprompt.com> writes:
> However, it seems the right fix is to move BufferGetPageSize and
> BufferGetPage from bufpage.h to bufmgr.h.

That sounds sane if it fixes the problem.

> (Digging further, it seems like bufpage.h should also include transam.h
> in order to get TransactionIdIsNormal ... I start to wonder how many
> problems of this nature we have on our headers. Without having a way to
> detect whether the defined macros are valid, it seems hard to check
> programatically, however.)

Yeah, I'm certain there's some other problems of the same kind, but I
don't see any easy way to find 'em.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > However, it seems the right fix is to move BufferGetPageSize and
> > BufferGetPage from bufpage.h to bufmgr.h.
>
> That sounds sane if it fixes the problem.

Actually it's not, because bufmgr.h does not include bufpage.h, and it
knows nothing about "pages".

However, I think most additions of bufmgr.h to source files are not all
that bogus -- a lot of the warnings I got were about
BufferAccessStrategyType. I'm still checking what's the best way to
deal with this, and if I can't find anything else I'll re-add bufmgr.h
to bufpage.h.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera wrote:
> Alvaro Herrera wrote:
>
>> Oops :-( I just noticed that I removed bufmgr.h from bufpage.h, which
>> is a change you had objected to previously :-(
>
> However, it seems the right fix is to move BufferGetPageSize and
> BufferGetPage from bufpage.h to bufmgr.h.

+1 It makes more sense.

> (Digging further, it seems like bufpage.h should also include transam.h
> in order to get TransactionIdIsNormal ... I start to wonder how many
> problems of this nature we have on our headers. Without having a way to
> detect whether the defined macros are valid, it seems hard to check
> programatically, however.)
>

I attached script which should check it. In first step it runs C preprocessor on
each header (postgres.h is included as well). The output from first step is
processed again with preprocessor and define.h is included. Define.h contains
"all" used macros in following format:

#define SIGABRT "NOT_EXPANDED_SIGABRT"

Main problem is how to generate define.h. I used following magic:

grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1 -d"("

but it generates some noise as well. Maybe some Perl or AWK magic should be better.

Zdenek


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Oops :-( I just noticed that I removed bufmgr.h from bufpage.h, which
> > is a change you had objected to previously :-(
> > http://archives.postgresql.org/pgsql...4/msg00149.php
>
> Hmm. I did notice that the patch seemed to need to add bufmgr.h
> inclusions to an awful lot of files, but I'd forgotten the argument
> about the bufpage.h macros needing it. Do you want to undo that
> aspect of it?

Done -- I put back bufmgr.h into bufpage.h. Surely there is a better
structure for this, perhaps involving more header files, but I don't
have time for that ATM.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Zdenek Kotala wrote:
> Alvaro Herrera wrote:

>> (Digging further, it seems like bufpage.h should also include transam.h
>> in order to get TransactionIdIsNormal ... I start to wonder how many
>> problems of this nature we have on our headers. Without having a way to
>> detect whether the defined macros are valid, it seems hard to check
>> programatically, however.)
>
> I attached script which should check it. In first step it runs C
> preprocessor on each header (postgres.h is included as well). The output
> from first step is processed again with preprocessor and define.h is
> included. Define.h contains "all" used macros in following format:
>
> #define SIGABRT "NOT_EXPANDED_SIGABRT"
>
> Main problem is how to generate define.h. I used following magic:
>
> grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1 -d"("
>
> but it generates some noise as well. Maybe some Perl or AWK magic should be better.

So were you able to detect anything bogus with this technique?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> Alvaro Herrera wrote:
>
>>> (Digging further, it seems like bufpage.h should also include transam.h
>>> in order to get TransactionIdIsNormal ... I start to wonder how many
>>> problems of this nature we have on our headers. Without having a way to
>>> detect whether the defined macros are valid, it seems hard to check
>>> programatically, however.)
>> I attached script which should check it. In first step it runs C
>> preprocessor on each header (postgres.h is included as well). The output
>> from first step is processed again with preprocessor and define.h is
>> included. Define.h contains "all" used macros in following format:
>>
>> #define SIGABRT "NOT_EXPANDED_SIGABRT"
>>
>> Main problem is how to generate define.h. I used following magic:
>>
>> grep "^#define" `find . -name "*.h"` | cut -d" " -f 2 | cut -f 1 | cut -f 1 -d"("
>>
>> but it generates some noise as well. Maybe some Perl or AWK magic should be better.
>
> So were you able to detect anything bogus with this technique?
>

No, everything looks OK.

Zdenek

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Zdenek Kotala wrote:
> Alvaro Herrera napsal(a):
>> Zdenek Kotala wrote:

>>> I attached script which should check it. In first step it runs C
>>> preprocessor on each header (postgres.h is included as well). The
>>> output from first step is processed again with preprocessor and
>>> define.h is included.

>> So were you able to detect anything bogus with this technique?
>
> No, everything looks OK.

Well, then it is not working, because transam.h is missing from
bufpage.h ...

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera napsal(a):
> Zdenek Kotala wrote:
>> Alvaro Herrera napsal(a):
>>> Zdenek Kotala wrote:
>
>>>> I attached script which should check it. In first step it runs C
>>>> preprocessor on each header (postgres.h is included as well). The
>>>> output from first step is processed again with preprocessor and
>>>> define.h is included.
>
>>> So were you able to detect anything bogus with this technique?
>> No, everything looks OK.
>
> Well, then it is not working, because transam.h is missing from
> bufpage.h ...
>

It tried catch problems with defines not with datatypes. If you mean that
TransactionID is not defined.

I try to play with lint now if it gets better results.

Zdenek





--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Zdenek Kotala wrote:
> Alvaro Herrera napsal(a):

>> Well, then it is not working, because transam.h is missing from
>> bufpage.h ...
>
> It tried catch problems with defines not with datatypes. If you mean that
> TransactionID is not defined.

No, I mean that TransactionIdIsNormal() is used in PageSetPrunable().

> I try to play with lint now if it gets better results.

Ok, good.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers