I still don't understand why this case in src/include/pgstat.h
is different from cases elsewhere in the code. Taken from
typedef struct xl_heap_header
#define SizeOfHeapHeader (offsetof(xl_heap_header, t_hoff) + sizeof(uint8))
Now, if somebody changed t_hoff to be a uint16, that SizeOfHeapHeader
macro would be wrong. Should we put a static assert in the code for that?
I have no objection, but it seems you like the static assert in one place
and not the other, and (perhaps due to some incredible ignorance on my
part) I cannot see why. I tried looking for an assert of this kind already in
the code. The use of this macro is in src/backend/access/heap/heapam.c,
but I didn't see any asserts for it, though there are lots of asserts for other
stuff. Maybe I just didn't recognize it?
On Thursday, January 2, 2014 2:05 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> It occurs to me that, rather than trying to improve the struct definition
> methodology, maybe we should just add static asserts to catch any
> inconsistency here. It wouldn't be that hard:
> #define PGSTAT_MAX_MSG_SIZE 1000
> #define PGSTAT_MSG_PAYLOAD (PGSTAT_MAX_MSG_SIZE - sizeof(PgStat_MsgHdr))
> ... all else in pgstat.h as before ...
> StaticAssertStmt(sizeof(PgStat_MsgTabstat) <= PGSTAT_MAX_MSG_SIZE,
> 'bad PgStat_MsgTabstat size');
> ... and similarly for other pgstat message structs ...
After further thought it seems to me that this is a desirable approach,
because it is a direct check of the property we want, and will complain
about *any* mistake that results in too-large struct sizes. The other
ideas that were kicked around upthread still left a lot of ways to mess up
the array size calculations, for instance referencing the wrong array
element type in the sizeof calculation. So unless anyone has a concrete
objection, I'll go put in something like this along with Mark's fix.
regards, tom lane
Sent via pgsql-hackers mailing list (email@example.com)
To make changes to your subscription: