Re: pgindent weirdness

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pgindent weirdness
Date: 2011-04-20 16:23:11
Message-ID: 4DAF086F.6050305@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 04/20/2011 11:43 AM, Tom Lane wrote:
> Andrew Dunstan<andrew(at)dunslane(dot)net> writes:
>> On 04/20/2011 05:48 AM, Bruce Momjian wrote:
>>> BulkInsertStateData is not listed in the typedef list supplied by
>>> Andrew; see src/tools/pgindent/typedefs.list. CC'ing him. This might
>>> be because the typdef is listed in two files:
>> It's tagged as a structure type by objdump, but not as a typedef:
> Hmm. hio.h clearly declares it as both, but many object files probably
> include only heapam.h, which exposes only the struct name. I'm guessing
> that you are merging the results from objdump'ing different files in a
> way that fails to consider the possibility of some files knowing more
> versions of a symbol than others.

We don't run objdump against individual object files, we run it against
the linked binaries, i.e. the contents of the installed bin and lib
directories.

But in any case, *none* of the individual files knows about
BulkInsertStateData as a typedef:

[andrew(at)emma backend]$ for f in ./access/heap/hio.o
./access/heap/heapam.o ./commands/tablecmds.o ./commands/copy.o
./executor/execMain.o ; do objdump -W $f ; done | egrep -A3
DW_TAG_typedef | grep BulkInsertState
<1811> DW_AT_name : (indirect string, offset: 0x1cc9):
BulkInsertState
<1f3c> DW_AT_name : (indirect string, offset: 0x296c):
BulkInsertState
<1fac> DW_AT_name : (indirect string, offset: 0x5c5b):
BulkInsertState
<211b> DW_AT_name : (indirect string, offset: 0x35ad):
BulkInsertState
<2530> DW_AT_name : (indirect string, offset: 0x3c93):
BulkInsertState

And the reason is actually fairly obvious on closer inspection. The only
place we actually use the BulkInsertStateData typedef (as opposed to the
struct declaration) is here:

./backend/access/heap/heapam.c: bistate = (BulkInsertState)
palloc(sizeof(BulkInsertStateData));

and that sizeof operation will be resolved at compile time and never hit
the symbol table.

So I suspect that the typedef finding code is actually working just fine.

It looks like the real problem is in how pgindent handles the use of
'struct foo' in various places.

cheers

andrew

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2011-04-20 16:29:02 Re: pgindent weirdness
Previous Message Tom Lane 2011-04-20 16:21:49 Re: Formatting Curmudgeons WAS: MMAP Buffers