[prev in list] [next in list] [prev in thread] [next in thread]
List: pgsql-hackers
Subject: Re: [HACKERS] fix_PGSTAT_NUM_TABENTRIES_macro patch
From: Mark Dilger <markdilger () yahoo ! com>
Date: 2013-12-31 18:43:58
Message-ID: 1388515438.43073.YahooMailNeo () web125403 ! mail ! ne1 ! yahoo ! com
[Download RAW message or body]
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@sss.pgh.pa.us> wrote:
Mark Dilger <markdilger@yahoo.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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[Attachment #3 (text/html)]
<html><body><div style="color:#000; background-color:#fff; font-family:HelveticaNeue, \
Helvetica Neue, Helvetica, Arial, Lucida Grande, sans-serif;font-size:12pt">A quick \
grep through the code reveals lots of examples,<br>so I'll just paste the first ones \
I notice. There are <br>references to sizeof(Oid), sizeof(uint32), \
sizeof(bool),<br>and sizeof(uint8) that clearly refer to fields in structs \
that<br>the macros refer to implicitly, but there is no way for the<br>compiler to \
detect if you change the struct but not the<br>macro.<br><br>I see these as similar \
to what I was talking about in<br>src/include/pgstat.h.<br><br><br>Mark \
Dilger<br><br><br>src/include/pg_attribute.h:<br>------------------------------<br><br>#define \
ATTRIBUTE_FIXED_PART_SIZE \<br> \
(offsetof(FormData_pg_attribute,attcollation) + \
sizeof(Oid))<br><br><br>src/include/access/tuptoaster.h:<br>-------------------------------------<br><br>#define
MaximumBytesPerTuple(tuplesPerPage) \<br> MAXALIGN_DOWN((BLCKSZ - \
\<br> \
MAXALIGN(SizeOfPageHeaderData + (tuplesPerPage) * sizeof(ItemIdData))) \
\<br> \
/ (tuplesPerPage))<br><br>#define TOAST_MAX_CHUNK_SIZE \
\<br> (EXTERN_TUPLE_MAX_SIZE \
- \
\<br> MAXALIGN(offsetof(HeapTupleHeaderData, t_bits)) - \
\<br> sizeof(Oid)
- \
\
\<br> sizeof(int32) \
- & \
nbsp; \
\<br> \
VARHDRSZ)<br><br>src/include/access/htup.h:<br>------------------------------<br><br>#define \
HeapTupleHeaderGetOid(tup) \<br>( \<br> ((tup)->t_infomask & \
HEAP_HASOID) ? \<br> *((Oid *) ((char \
*)(tup) + (tup)->t_hoff - sizeof(Oid))) \<br> : \
\<br> InvalidOid \<br>)<br><br>#define \
HeapTupleHeaderSetOid(tup, oid) \<br>do { \<br> \
Assert((tup)->t_infomask & HEAP_HASOID); \<br> *((Oid *) \
((char *)(tup) + (tup)->t_hoff - sizeof(Oid))) = (oid); \<br>} while \
(0)<br><br>#define MINIMAL_TUPLE_OFFSET \<br> \
((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) / MAXIMUM_ALIGNOF * \
MAXIMUM_ALIGNOF)<br>#define MINIMAL_TUPLE_PADDING \<br> \
((offsetof(HeapTupleHeaderData, t_infomask2) - sizeof(uint32)) % \
MAXIMUM_ALIGNOF)<br>#define MINIMAL_TUPLE_DATA_OFFSET \<br> \
offsetof(MinimalTupleData, t_infomask2)<br><br>#define \
SizeOfHeapDelete (offsetof(xl_heap_delete, all_visible_cleared) + \
sizeof(bool))<br><br>#define SizeOfHeapHeader \
(offsetof(xl_heap_header, t_hoff) + \
sizeof(uint8))<br><br><br><br><div><span><br></span></div><div style="display: \
block;" class="yahoo_quoted"> <br> <br> <div style="font-family: HelveticaNeue, \
Helvetica Neue, Helvetica, Arial, Lucida Grande, sans-serif; font-size: 12pt;"> <div \
style="font-family: HelveticaNeue, Helvetica Neue, Helvetica, Arial, Lucida Grande, \
sans-serif; font-size: 12pt;"> <div dir="ltr"> <font face="Arial" size="2"> On \
Tuesday, December 31, 2013 10:26 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:<br> \
</font> </div> <div class="y_msg_container">Mark Dilger <<a shape="rect" \
ymailto="mailto:markdilger@yahoo.com" \
href="mailto:markdilger@yahoo.com">markdilger@yahoo.com</a>> writes:<br \
clear="none">> I don't care for the places in the code that say things like<br \
clear="none"><br clear="none">> foo - sizeof(int)<br \
clear="none"><br clear="none">> where "int" refers implicitly to a specific \
variable or struct field, but<br clear="none">> you have to remember that and \
change it by hand if you change the<br clear="none">> type of the variable or \
struct.<br clear="none"><br clear="none">> But this sort of code is quite common \
in postgres, and not<br clear="none">> at all unique to src/include/pgstat.h.<br \
clear="none"><br clear="none">Really? I think we're using offsetof for this \
type of thing in most<div class="yqt1521000123" id="yqtfd42498"><br \
clear="none">places.<br clear="none"><br clear="none"> \
regards, tom lane<br clear="none"><br \
clear="none"><br clear="none">-- <br clear="none">Sent via pgsql-hackers mailing list \
(<a shape="rect" ymailto="mailto:pgsql-hackers@postgresql.org" \
href="mailto:pgsql-hackers@postgresql.org">pgsql-hackers@postgresql.org</a>)<br \
clear="none">To make changes to your subscription:<br clear="none"><a shape="rect" \
href="http://www.postgresql.org/mailpref/pgsql-hackers" \
target="_blank">http://www.postgresql.org/mailpref/pgsql-hackers</a><br \
clear="none"></div><br><br></div> </div> </div> </div> </div></body></html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic