Re: fix_PGSTAT_NUM_TABENTRIES_macro patch

From: Mark Dilger <markdilger(at)yahoo(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: fix_PGSTAT_NUM_TABENTRIES_macro patch
Date: 2013-12-31 18:43:58
Message-ID: 1388515438.43073.YahooMailNeo@web125403.mail.ne1.yahoo.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

A quick grep through the code reveals lots of examples,
so I'll just paste the first ones I notice.  There are
references to sizeof(Oid), sizeof(uint32), sizeof(bool),
and sizeof(uint8) that clearly refer to fields in structs that
the macros refer to implicitly, but there is no way for the
compiler to detect if you change the struct but not the
macro.

I see these as similar to what I was talking about in
src/include/pgstat.h.

Mark Dilger

src/include/pg_attribute.h:
------------------------------

#define ATTRIBUTE_FIXED_PART_SIZE \
    (offsetof(FormData_pg_attribute,attcollation) + sizeof(Oid))

src/include/access/tuptoaster.h:
-------------------------------------

#define MaximumBytesPerTuple(tuplesPerPage) \
    MAXALIGN_DOWN((BLCKSZ - \
                   MAXALIGN(SizeOfPageHeaderData + (tuplesPerPage) * sizeof(ItemIdData))) \
                  / (tuplesPerPage))

#define TOAST_MAX_CHUNK_SIZE    \
    (EXTERN_TUPLE_MAX_SIZE -                            \
     MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) -  \
     sizeof(Oid) -                                      \
     sizeof(int32) -                                    \
     VARHDRSZ)

src/include/access/htup.h:
------------------------------

#define HeapTupleHeaderGetOid(tup) \
( \
    ((tup)->t_infomask & HEAP_HASOID) ? \
        *((Oid *) ((char *)(tup) + (tup)->t_hoff - sizeof(Oid))) \
    : \
        InvalidOid \
)

#define HeapTupleHeaderSetOid(tup, oid) \
do { \
    Assert((tup)->t_infomask & HEAP_HASOID); \
    *((Oid *) ((char *)(tup) + (tup)->t_hoff - sizeof(Oid))) = (oid); \
} while (0)

#define MINIMAL_TUPLE_OFFSET \
    ((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) / MAXIMUM_ALIGNOF * MAXIMUM_ALIGNOF)
#define MINIMAL_TUPLE_PADDING \
    ((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) % MAXIMUM_ALIGNOF)
#define MINIMAL_TUPLE_DATA_OFFSET \
    offsetof(MinimalTupleData, t_infomask2)

#define SizeOfHeapDelete    (offsetof(xl_heap_delete, all_visible_cleared) + sizeof(bool))

#define SizeOfHeapHeader    (offsetof(xl_heap_header, t_hoff) + sizeof(uint8))

On Tuesday, December 31, 2013 10:26 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

Mark Dilger <markdilger(at)yahoo(dot)com> writes:
> I don't care for the places in the code that say things like

>     foo - sizeof(int)

> where "int" refers implicitly to a specific variable or struct field, but
> you have to remember that and change it by hand if you change the
> type of the variable or struct.

> But this sort of code is quite common in postgres, and not
> at all unique to src/include/pgstat.h.

Really?  I think we're using offsetof for this type of thing in most

places.

            regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2013-12-31 18:45:59 Re: proposal: persistent plpgsql plugin info - field plugin_info for plpgsql_function structure
Previous Message Pavel Stehule 2013-12-31 18:34:01 proposal: persistent plpgsql plugin info - field plugin_info for plpgsql_function structure