On Thu, Feb 19, 2015 at 2:58 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: >> 1-2) sizeof(ParamListInfoData) is present in a couple of places, >> assuming that sizeof(ParamListInfoData) has the equivalent of 1 >> parameter, like prepare.c, functions.c, spi.c and postgres.c: >> - /* sizeof(ParamListInfoData) includes the first array element */ >> paramLI = (ParamListInfo) >> palloc(sizeof(ParamListInfoData) + >> - (num_params - 1) * sizeof(ParamExternData)); >> + num_params * sizeof(ParamExternData)); >> 1-3) FuncCandidateList in namespace.c (thanks Andres!): >> newResult = (FuncCandidateList) >> - palloc(sizeof(struct _FuncCandidateList) - >> sizeof(Oid) >> - + effective_nargs * sizeof(Oid)); >> + palloc(sizeof(struct _FuncCandidateList) + >> + effective_nargs * sizeof(Oid)); >> I imagine that we do not want for those palloc calls to use ifdef >> FLEXIBLE_ARRAY_MEMBER to save some memory for code readability even if >> compiler does not support flexible-array length, right? > > These are just wrong. As a general rule, we do not want to *ever* take > sizeof() a struct that contains a flexible array: the results will not > be consistent across platforms. The right thing is to use offsetof() > instead. See the helpful comment autoconf provides: > > [...]
And I had this one in front of my eyes a couple of hours ago... Thanks. > This point is actually the main reason we've not done this change long > since. People did not feel like running around to make sure there were > no overlooked uses of sizeof(). Thanks for the clarifications and the review. Attached is a new set. -- Michael
From ca92019afc1db4da147ebb5c436164a787f7fa62 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Mon, 16 Feb 2015 02:44:10 +0900 Subject: [PATCH 1/4] First cut with FLEXIBLE_ARRAY_MEMBER This is targetted to prevent false positive errors that static code analyzers may do by assuming that some structures have an unvarying size. --- contrib/cube/cubedata.h | 7 +++---- contrib/intarray/_int.h | 4 ++-- contrib/ltree/ltree.h | 14 +++++++------- contrib/pg_trgm/trgm.h | 2 +- src/backend/catalog/namespace.c | 4 ++-- src/backend/commands/prepare.c | 6 +++--- src/backend/executor/functions.c | 6 +++--- src/backend/executor/spi.c | 5 ++--- src/backend/nodes/params.c | 5 ++--- src/backend/tcop/postgres.c | 5 ++--- src/bin/pg_dump/dumputils.c | 3 +-- src/bin/pg_dump/dumputils.h | 2 +- src/include/access/gin_private.h | 2 +- src/include/access/gist_private.h | 5 +++-- src/include/access/heapam_xlog.h | 2 +- src/include/access/spgist_private.h | 10 +++++----- src/include/access/xact.h | 8 ++++---- src/include/c.h | 4 ++-- src/include/catalog/namespace.h | 4 ++-- src/include/commands/dbcommands.h | 4 ++-- src/include/commands/tablespace.h | 2 +- src/include/executor/hashjoin.h | 2 +- src/include/nodes/bitmapset.h | 4 ++-- src/include/nodes/params.h | 2 +- src/include/nodes/tidbitmap.h | 2 +- src/include/postgres.h | 9 +++++---- src/include/postmaster/syslogger.h | 2 +- src/include/replication/walsender_private.h | 2 +- src/include/storage/bufpage.h | 3 ++- src/include/storage/fsm_internals.h | 2 +- src/include/storage/standby.h | 4 ++-- src/include/tsearch/dicts/regis.h | 2 +- src/include/tsearch/dicts/spell.h | 6 +++--- src/include/tsearch/ts_type.h | 6 +++--- src/include/utils/catcache.h | 4 ++-- src/include/utils/datetime.h | 5 +++-- src/include/utils/geo_decls.h | 5 +++-- src/include/utils/jsonb.h | 2 +- src/include/utils/relmapper.h | 2 +- src/include/utils/varbit.h | 3 ++- 40 files changed, 86 insertions(+), 85 deletions(-) diff --git a/contrib/cube/cubedata.h b/contrib/cube/cubedata.h index 5d44e11..3535847 100644 --- a/contrib/cube/cubedata.h +++ b/contrib/cube/cubedata.h @@ -23,11 +23,10 @@ typedef struct NDBOX unsigned int header; /* - * Variable length array. The lower left coordinates for each dimension - * come first, followed by upper right coordinates unless the point flag - * is set. + * The lower left coordinates for each dimension come first, followed + * by upper right coordinates unless the point flag is set. */ - double x[1]; + double x[FLEXIBLE_ARRAY_MEMBER]; } NDBOX; #define POINT_BIT 0x80000000 diff --git a/contrib/intarray/_int.h b/contrib/intarray/_int.h index 7f93206..d524f0f 100644 --- a/contrib/intarray/_int.h +++ b/contrib/intarray/_int.h @@ -73,7 +73,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 flag; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } GISTTYPE; #define ALLISTRUE 0x04 @@ -133,7 +133,7 @@ typedef struct QUERYTYPE { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 size; /* number of ITEMs */ - ITEM items[1]; /* variable length array */ + ITEM items[FLEXIBLE_ARRAY_MEMBER]; } QUERYTYPE; #define HDRSIZEQT offsetof(QUERYTYPE, items) diff --git a/contrib/ltree/ltree.h b/contrib/ltree/ltree.h index 1b1305b..c604357 100644 --- a/contrib/ltree/ltree.h +++ b/contrib/ltree/ltree.h @@ -10,7 +10,7 @@ typedef struct { uint16 len; - char name[1]; + char name[FLEXIBLE_ARRAY_MEMBER]; } ltree_level; #define LEVEL_HDRSIZE (offsetof(ltree_level,name)) @@ -20,7 +20,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ uint16 numlevel; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } ltree; #define LTREE_HDRSIZE MAXALIGN( offsetof(ltree, data) ) @@ -34,7 +34,7 @@ typedef struct int32 val; uint16 len; uint8 flag; - char name[1]; + char name[FLEXIBLE_ARRAY_MEMBER]; } lquery_variant; #define LVAR_HDRSIZE MAXALIGN(offsetof(lquery_variant, name)) @@ -51,7 +51,7 @@ typedef struct uint16 numvar; uint16 low; uint16 high; - char variants[1]; + char variants[FLEXIBLE_ARRAY_MEMBER]; } lquery_level; #define LQL_HDRSIZE MAXALIGN( offsetof(lquery_level,variants) ) @@ -72,7 +72,7 @@ typedef struct uint16 numlevel; uint16 firstgood; uint16 flag; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } lquery; #define LQUERY_HDRSIZE MAXALIGN( offsetof(lquery, data) ) @@ -107,7 +107,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 size; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } ltxtquery; #define HDRSIZEQT MAXALIGN(VARHDRSZ + sizeof(int32)) @@ -208,7 +208,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ uint32 flag; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } ltree_gist; #define LTG_ONENODE 0x01 diff --git a/contrib/pg_trgm/trgm.h b/contrib/pg_trgm/trgm.h index ed649b8..f030558 100644 --- a/contrib/pg_trgm/trgm.h +++ b/contrib/pg_trgm/trgm.h @@ -63,7 +63,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ uint8 flag; - char data[1]; + char data[FLEXIBLE_ARRAY_MEMBER]; } TRGM; #define TRGMHDRSIZE (VARHDRSZ + sizeof(uint8)) diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index bfb4fdc..d47b523 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -1075,8 +1075,8 @@ FuncnameGetCandidates(List *names, int nargs, List *argnames, */ effective_nargs = Max(pronargs, nargs); newResult = (FuncCandidateList) - palloc(sizeof(struct _FuncCandidateList) - sizeof(Oid) - + effective_nargs * sizeof(Oid)); + palloc(offsetof(struct _FuncCandidateList, args) + + effective_nargs * sizeof(Oid)); newResult->pathpos = pathpos; newResult->oid = HeapTupleGetOid(proctup); newResult->nargs = effective_nargs; diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index 71b08f0..36e21c1 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -383,10 +383,10 @@ EvaluateParams(PreparedStatement *pstmt, List *params, /* Prepare the expressions for execution */ exprstates = (List *) ExecPrepareExpr((Expr *) params, estate); - /* sizeof(ParamListInfoData) includes the first array element */ paramLI = (ParamListInfo) - palloc(sizeof(ParamListInfoData) + - (num_params - 1) * sizeof(ParamExternData)); + palloc(offsetof(ParamListInfoData, params) + + num_params * sizeof(ParamExternData)); + /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c index 84be37c..6c3eff7 100644 --- a/src/backend/executor/functions.c +++ b/src/backend/executor/functions.c @@ -896,9 +896,9 @@ postquel_sub_params(SQLFunctionCachePtr fcache, if (fcache->paramLI == NULL) { - /* sizeof(ParamListInfoData) includes the first array element */ - paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) + - (nargs - 1) * sizeof(ParamExternData)); + paramLI = (ParamListInfo) + palloc(offsetof(ParamListInfoData, params) + + nargs * sizeof(ParamExternData)); /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c index 4b86e91..d101fcd 100644 --- a/src/backend/executor/spi.c +++ b/src/backend/executor/spi.c @@ -2290,9 +2290,8 @@ _SPI_convert_params(int nargs, Oid *argtypes, { int i; - /* sizeof(ParamListInfoData) includes the first array element */ - paramLI = (ParamListInfo) palloc(sizeof(ParamListInfoData) + - (nargs - 1) * sizeof(ParamExternData)); + paramLI = (ParamListInfo) palloc(offsetof(ParamListInfoData, params) + + nargs * sizeof(ParamExternData)); /* we have static list of params, so no hooks needed */ paramLI->paramFetch = NULL; paramLI->paramFetchArg = NULL; diff --git a/src/backend/nodes/params.c b/src/backend/nodes/params.c index 2f2f5ed..fb803f8 100644 --- a/src/backend/nodes/params.c +++ b/src/backend/nodes/params.c @@ -40,9 +40,8 @@ copyParamList(ParamListInfo from) if (from == NULL || from->numParams <= 0) return NULL; - /* sizeof(ParamListInfoData) includes the first array element */ - size = sizeof(ParamListInfoData) + - (from->numParams - 1) * sizeof(ParamExternData); + size = offsetof(ParamListInfoData, params) + + from->numParams * sizeof(ParamExternData); retval = (ParamListInfo) palloc(size); retval->paramFetch = NULL; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 28af40c..33720e8 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -1619,9 +1619,8 @@ exec_bind_message(StringInfo input_message) { int paramno; - /* sizeof(ParamListInfoData) includes the first array element */ - params = (ParamListInfo) palloc(sizeof(ParamListInfoData) + - (numParams - 1) * sizeof(ParamExternData)); + params = (ParamListInfo) palloc(offsetof(ParamListInfoData, params) + + numParams * sizeof(ParamExternData)); /* we have static list of params, so no hooks needed */ params->paramFetch = NULL; params->paramFetchArg = NULL; diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 095c507..892d3fa 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -1217,8 +1217,7 @@ simple_string_list_append(SimpleStringList *list, const char *val) SimpleStringListCell *cell; /* this calculation correctly accounts for the null trailing byte */ - cell = (SimpleStringListCell *) - pg_malloc(sizeof(SimpleStringListCell) + strlen(val)); + cell = (SimpleStringListCell *) pg_malloc(sizeof(SimpleStringListCell)); cell->next = NULL; strcpy(cell->val, val); diff --git a/src/bin/pg_dump/dumputils.h b/src/bin/pg_dump/dumputils.h index a39c1b6..94476a9 100644 --- a/src/bin/pg_dump/dumputils.h +++ b/src/bin/pg_dump/dumputils.h @@ -38,7 +38,7 @@ typedef struct SimpleOidList typedef struct SimpleStringListCell { struct SimpleStringListCell *next; - char val[1]; /* VARIABLE LENGTH FIELD */ + char val[FLEXIBLE_ARRAY_MEMBER]; } SimpleStringListCell; typedef struct SimpleStringList diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h index bda7c28..d3abbc0 100644 --- a/src/include/access/gin_private.h +++ b/src/include/access/gin_private.h @@ -389,7 +389,7 @@ typedef struct { ItemPointerData first; /* first item in this posting list (unpacked) */ uint16 nbytes; /* number of bytes that follow */ - unsigned char bytes[1]; /* varbyte encoded items (variable length) */ + unsigned char bytes[FLEXIBLE_ARRAY_MEMBER]; /* varbyte encoded items */ } GinPostingList; #define SizeOfGinPostingList(plist) (offsetof(GinPostingList, bytes) + SHORTALIGN((plist)->nbytes) ) diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h index 382826e..36f5257 100644 --- a/src/include/access/gist_private.h +++ b/src/include/access/gist_private.h @@ -47,7 +47,7 @@ typedef struct { BlockNumber prev; uint32 freespace; - char tupledata[1]; + char tupledata[FLEXIBLE_ARRAY_MEMBER]; } GISTNodeBufferPage; #define BUFFER_PAGE_DATA_OFFSET MAXALIGN(offsetof(GISTNodeBufferPage, tupledata)) @@ -131,7 +131,8 @@ typedef struct GISTSearchItem /* we must store parentlsn to detect whether a split occurred */ GISTSearchHeapItem heap; /* heap info, if heap tuple */ } data; - double distances[1]; /* array with numberOfOrderBys entries */ + double distances[FLEXIBLE_ARRAY_MEMBER]; /* array with numberOfOrderBys + * entries */ } GISTSearchItem; #define GISTSearchItemIsHeap(item) ((item).blkno == InvalidBlockNumber) diff --git a/src/include/access/heapam_xlog.h b/src/include/access/heapam_xlog.h index a2ed2a0..f0f89de 100644 --- a/src/include/access/heapam_xlog.h +++ b/src/include/access/heapam_xlog.h @@ -132,7 +132,7 @@ typedef struct xl_heap_multi_insert { uint8 flags; uint16 ntuples; - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } xl_heap_multi_insert; #define SizeOfHeapMultiInsert offsetof(xl_heap_multi_insert, offsets) diff --git a/src/include/access/spgist_private.h b/src/include/access/spgist_private.h index f11d8ef..0492ef6 100644 --- a/src/include/access/spgist_private.h +++ b/src/include/access/spgist_private.h @@ -426,7 +426,7 @@ typedef struct spgxlogMoveLeafs * the dead tuple from the source *---------- */ - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } spgxlogMoveLeafs; #define SizeOfSpgxlogMoveLeafs offsetof(spgxlogMoveLeafs, offsets) @@ -534,7 +534,7 @@ typedef struct spgxlogPickSplit * list of leaf tuples, length nInsert (unaligned!) *---------- */ - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } spgxlogPickSplit; #define SizeOfSpgxlogPickSplit offsetof(spgxlogPickSplit, offsets) @@ -558,7 +558,7 @@ typedef struct spgxlogVacuumLeaf * tuple numbers to insert in nextOffset links *---------- */ - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } spgxlogVacuumLeaf; #define SizeOfSpgxlogVacuumLeaf offsetof(spgxlogVacuumLeaf, offsets) @@ -571,7 +571,7 @@ typedef struct spgxlogVacuumRoot spgxlogState stateSrc; /* offsets of tuples to delete follow */ - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } spgxlogVacuumRoot; #define SizeOfSpgxlogVacuumRoot offsetof(spgxlogVacuumRoot, offsets) @@ -583,7 +583,7 @@ typedef struct spgxlogVacuumRedirect TransactionId newestRedirectXid; /* newest XID of removed redirects */ /* offsets of redirect tuples to make placeholders follow */ - OffsetNumber offsets[1]; + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } spgxlogVacuumRedirect; #define SizeOfSpgxlogVacuumRedirect offsetof(spgxlogVacuumRedirect, offsets) diff --git a/src/include/access/xact.h b/src/include/access/xact.h index 8205504..9b95f10 100644 --- a/src/include/access/xact.h +++ b/src/include/access/xact.h @@ -118,7 +118,7 @@ typedef struct xl_xact_assignment { TransactionId xtop; /* assigned XID's top-level XID */ int nsubxacts; /* number of subtransaction XIDs */ - TransactionId xsub[1]; /* assigned subxids */ + TransactionId xsub[FLEXIBLE_ARRAY_MEMBER]; /* assigned subxids */ } xl_xact_assignment; #define MinSizeOfXactAssignment offsetof(xl_xact_assignment, xsub) @@ -128,7 +128,7 @@ typedef struct xl_xact_commit_compact TimestampTz xact_time; /* time of commit */ int nsubxacts; /* number of subtransaction XIDs */ /* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */ - TransactionId subxacts[1]; /* VARIABLE LENGTH ARRAY */ + TransactionId subxacts[FLEXIBLE_ARRAY_MEMBER]; } xl_xact_commit_compact; #define MinSizeOfXactCommitCompact offsetof(xl_xact_commit_compact, subxacts) @@ -143,7 +143,7 @@ typedef struct xl_xact_commit Oid dbId; /* MyDatabaseId */ Oid tsId; /* MyDatabaseTableSpace */ /* Array of RelFileNode(s) to drop at commit */ - RelFileNode xnodes[1]; /* VARIABLE LENGTH ARRAY */ + RelFileNode xnodes[FLEXIBLE_ARRAY_MEMBER]; /* ARRAY OF COMMITTED SUBTRANSACTION XIDs FOLLOWS */ /* ARRAY OF SHARED INVALIDATION MESSAGES FOLLOWS */ } xl_xact_commit; @@ -171,7 +171,7 @@ typedef struct xl_xact_abort int nrels; /* number of RelFileNodes */ int nsubxacts; /* number of subtransaction XIDs */ /* Array of RelFileNode(s) to drop at abort */ - RelFileNode xnodes[1]; /* VARIABLE LENGTH ARRAY */ + RelFileNode xnodes[FLEXIBLE_ARRAY_MEMBER]; /* ARRAY OF ABORTED SUBTRANSACTION XIDs FOLLOWS */ } xl_xact_abort; diff --git a/src/include/c.h b/src/include/c.h index b187520..bbd0d53 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -424,7 +424,7 @@ typedef struct Oid elemtype; int dim1; int lbound1; - int16 values[1]; /* VARIABLE LENGTH ARRAY */ + int16 values[FLEXIBLE_ARRAY_MEMBER]; } int2vector; /* VARIABLE LENGTH STRUCT */ typedef struct @@ -435,7 +435,7 @@ typedef struct Oid elemtype; int dim1; int lbound1; - Oid values[1]; /* VARIABLE LENGTH ARRAY */ + Oid values[FLEXIBLE_ARRAY_MEMBER]; } oidvector; /* VARIABLE LENGTH STRUCT */ /* diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index d2e5198..cf5f7d0 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -34,8 +34,8 @@ typedef struct _FuncCandidateList int nvargs; /* number of args to become variadic array */ int ndargs; /* number of defaulted args */ int *argnumbers; /* args' positional indexes, if named call */ - Oid args[1]; /* arg types --- VARIABLE LENGTH ARRAY */ -} *FuncCandidateList; /* VARIABLE LENGTH STRUCT */ + Oid args[FLEXIBLE_ARRAY_MEMBER]; /* arg types */ +} *FuncCandidateList; /* * Structure for xxxOverrideSearchPath functions diff --git a/src/include/commands/dbcommands.h b/src/include/commands/dbcommands.h index cb7cc0e..be1cac2 100644 --- a/src/include/commands/dbcommands.h +++ b/src/include/commands/dbcommands.h @@ -26,7 +26,7 @@ typedef struct xl_dbase_create_rec_old { /* Records copying of a single subdirectory incl. contents */ Oid db_id; - char src_path[1]; /* VARIABLE LENGTH STRING */ + char src_path[FLEXIBLE_ARRAY_MEMBER]; /* dst_path follows src_path */ } xl_dbase_create_rec_old; @@ -34,7 +34,7 @@ typedef struct xl_dbase_drop_rec_old { /* Records dropping of a single subdirectory incl. contents */ Oid db_id; - char dir_path[1]; /* VARIABLE LENGTH STRING */ + char dir_path[FLEXIBLE_ARRAY_MEMBER]; } xl_dbase_drop_rec_old; typedef struct xl_dbase_create_rec diff --git a/src/include/commands/tablespace.h b/src/include/commands/tablespace.h index e8b9bc4..9e40e31 100644 --- a/src/include/commands/tablespace.h +++ b/src/include/commands/tablespace.h @@ -25,7 +25,7 @@ typedef struct xl_tblspc_create_rec { Oid ts_id; - char ts_path[1]; /* VARIABLE LENGTH STRING */ + char ts_path[FLEXIBLE_ARRAY_MEMBER]; } xl_tblspc_create_rec; typedef struct xl_tblspc_drop_rec diff --git a/src/include/executor/hashjoin.h b/src/include/executor/hashjoin.h index e79df71..71099b1 100644 --- a/src/include/executor/hashjoin.h +++ b/src/include/executor/hashjoin.h @@ -114,7 +114,7 @@ typedef struct HashMemoryChunkData struct HashMemoryChunkData *next; /* pointer to the next chunk (linked list) */ - char data[1]; /* buffer allocated at the end */ + char data[FLEXIBLE_ARRAY_MEMBER]; /* buffer allocated at the end */ } HashMemoryChunkData; typedef struct HashMemoryChunkData *HashMemoryChunk; diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h index 5f45f4d..3a556ee 100644 --- a/src/include/nodes/bitmapset.h +++ b/src/include/nodes/bitmapset.h @@ -32,8 +32,8 @@ typedef int32 signedbitmapword; /* must be the matching signed type */ typedef struct Bitmapset { int nwords; /* number of words in array */ - bitmapword words[1]; /* really [nwords] */ -} Bitmapset; /* VARIABLE LENGTH STRUCT */ + bitmapword words[FLEXIBLE_ARRAY_MEMBER]; /* really [nwords] */ +} Bitmapset; /* result of bms_subset_compare */ diff --git a/src/include/nodes/params.h b/src/include/nodes/params.h index 5b096c5..a0f7dd0 100644 --- a/src/include/nodes/params.h +++ b/src/include/nodes/params.h @@ -71,7 +71,7 @@ typedef struct ParamListInfoData ParserSetupHook parserSetup; /* parser setup hook */ void *parserSetupArg; int numParams; /* number of ParamExternDatas following */ - ParamExternData params[1]; /* VARIABLE LENGTH ARRAY */ + ParamExternData params[FLEXIBLE_ARRAY_MEMBER]; } ParamListInfoData; diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h index fb62c9e..bfbc0fb 100644 --- a/src/include/nodes/tidbitmap.h +++ b/src/include/nodes/tidbitmap.h @@ -41,7 +41,7 @@ typedef struct int ntuples; /* -1 indicates lossy result */ bool recheck; /* should the tuples be rechecked? */ /* Note: recheck is always true if ntuples < 0 */ - OffsetNumber offsets[1]; /* VARIABLE LENGTH ARRAY */ + OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER]; } TBMIterateResult; /* VARIABLE LENGTH STRUCT */ /* function prototypes in nodes/tidbitmap.c */ diff --git a/src/include/postgres.h b/src/include/postgres.h index 082c75b..482e676 100644 --- a/src/include/postgres.h +++ b/src/include/postgres.h @@ -117,20 +117,20 @@ typedef union struct /* Normal varlena (4-byte length) */ { uint32 va_header; - char va_data[1]; + char va_data[FLEXIBLE_ARRAY_MEMBER]; } va_4byte; struct /* Compressed-in-line format */ { uint32 va_header; uint32 va_rawsize; /* Original data size (excludes header) */ - char va_data[1]; /* Compressed data */ + char va_data[FLEXIBLE_ARRAY_MEMBER]; /* Compressed data */ } va_compressed; } varattrib_4b; typedef struct { uint8 va_header; - char va_data[1]; /* Data begins here */ + char va_data[FLEXIBLE_ARRAY_MEMBER]; /* Data begins here */ } varattrib_1b; /* TOAST pointers are a subset of varattrib_1b with an identifying tag byte */ @@ -138,7 +138,8 @@ typedef struct { uint8 va_header; /* Always 0x80 or 0x01 */ uint8 va_tag; /* Type of datum */ - char va_data[1]; /* Data (of the type indicated by va_tag) */ + char va_data[FLEXIBLE_ARRAY_MEMBER]; /* Data (of the type + * indicated by va_tag) */ } varattrib_1b_e; /* diff --git a/src/include/postmaster/syslogger.h b/src/include/postmaster/syslogger.h index 602b13c..89a535c 100644 --- a/src/include/postmaster/syslogger.h +++ b/src/include/postmaster/syslogger.h @@ -48,7 +48,7 @@ typedef struct int32 pid; /* writer's pid */ char is_last; /* last chunk of message? 't' or 'f' ('T' or * 'F' for CSV case) */ - char data[1]; /* data payload starts here */ + char data[FLEXIBLE_ARRAY_MEMBER]; /* data payload starts here */ } PipeProtoHeader; typedef union diff --git a/src/include/replication/walsender_private.h b/src/include/replication/walsender_private.h index 8867750..02faad8 100644 --- a/src/include/replication/walsender_private.h +++ b/src/include/replication/walsender_private.h @@ -88,7 +88,7 @@ typedef struct */ bool sync_standbys_defined; - WalSnd walsnds[1]; /* VARIABLE LENGTH ARRAY */ + WalSnd walsnds[FLEXIBLE_ARRAY_MEMBER]; } WalSndCtlData; extern WalSndCtlData *WalSndCtl; diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index f693032..b7dbd6f 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -156,7 +156,8 @@ typedef struct PageHeaderData LocationIndex pd_special; /* offset to start of special space */ uint16 pd_pagesize_version; TransactionId pd_prune_xid; /* oldest prunable XID, or zero if none */ - ItemIdData pd_linp[1]; /* beginning of line pointer array */ + ItemIdData pd_linp[FLEXIBLE_ARRAY_MEMBER]; /* beginning of line pointer + * array */ } PageHeaderData; typedef PageHeaderData *PageHeader; diff --git a/src/include/storage/fsm_internals.h b/src/include/storage/fsm_internals.h index 1decd90..26340b4 100644 --- a/src/include/storage/fsm_internals.h +++ b/src/include/storage/fsm_internals.h @@ -39,7 +39,7 @@ typedef struct * NonLeafNodesPerPage elements are upper nodes, and the following * LeafNodesPerPage elements are leaf nodes. Unused nodes are zero. */ - uint8 fp_nodes[1]; + uint8 fp_nodes[FLEXIBLE_ARRAY_MEMBER]; } FSMPageData; typedef FSMPageData *FSMPage; diff --git a/src/include/storage/standby.h b/src/include/storage/standby.h index c32c963..7626c4c 100644 --- a/src/include/storage/standby.h +++ b/src/include/storage/standby.h @@ -60,7 +60,7 @@ extern void StandbyReleaseOldLocks(int nxids, TransactionId *xids); typedef struct xl_standby_locks { int nlocks; /* number of entries in locks array */ - xl_standby_lock locks[1]; /* VARIABLE LENGTH ARRAY */ + xl_standby_lock locks[FLEXIBLE_ARRAY_MEMBER]; } xl_standby_locks; /* @@ -75,7 +75,7 @@ typedef struct xl_running_xacts TransactionId oldestRunningXid; /* *not* oldestXmin */ TransactionId latestCompletedXid; /* so we can set xmax */ - TransactionId xids[1]; /* VARIABLE LENGTH ARRAY */ + TransactionId xids[FLEXIBLE_ARRAY_MEMBER]; } xl_running_xacts; #define MinSizeOfXactRunningXacts offsetof(xl_running_xacts, xids) diff --git a/src/include/tsearch/dicts/regis.h b/src/include/tsearch/dicts/regis.h index 081a502..ddf5b60 100644 --- a/src/include/tsearch/dicts/regis.h +++ b/src/include/tsearch/dicts/regis.h @@ -21,7 +21,7 @@ typedef struct RegisNode len:16, unused:14; struct RegisNode *next; - unsigned char data[1]; + unsigned char data[FLEXIBLE_ARRAY_MEMBER]; } RegisNode; #define RNHDRSZ (offsetof(RegisNode,data)) diff --git a/src/include/tsearch/dicts/spell.h b/src/include/tsearch/dicts/spell.h index a75552b..e512532 100644 --- a/src/include/tsearch/dicts/spell.h +++ b/src/include/tsearch/dicts/spell.h @@ -49,7 +49,7 @@ typedef struct typedef struct SPNode { uint32 length; - SPNodeData data[1]; + SPNodeData data[FLEXIBLE_ARRAY_MEMBER]; } SPNode; #define SPNHDRSZ (offsetof(SPNode,data)) @@ -70,7 +70,7 @@ typedef struct spell_struct int len; } d; } p; - char word[1]; /* variable length, null-terminated */ + char word[FLEXIBLE_ARRAY_MEMBER]; } SPELL; #define SPELLHDRSZ (offsetof(SPELL, word)) @@ -120,7 +120,7 @@ typedef struct AffixNode { uint32 isvoid:1, length:31; - AffixNodeData data[1]; + AffixNodeData data[FLEXIBLE_ARRAY_MEMBER]; } AffixNode; #define ANHRDSZ (offsetof(AffixNode, data)) diff --git a/src/include/tsearch/ts_type.h b/src/include/tsearch/ts_type.h index 1cdfa82..ce919a2 100644 --- a/src/include/tsearch/ts_type.h +++ b/src/include/tsearch/ts_type.h @@ -63,7 +63,7 @@ typedef uint16 WordEntryPos; typedef struct { uint16 npos; - WordEntryPos pos[1]; /* variable length */ + WordEntryPos pos[FLEXIBLE_ARRAY_MEMBER]; } WordEntryPosVector; @@ -82,7 +82,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 size; - WordEntry entries[1]; /* variable length */ + WordEntry entries[FLEXIBLE_ARRAY_MEMBER]; /* lexemes follow the entries[] array */ } TSVectorData; @@ -233,7 +233,7 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 size; /* number of QueryItems */ - char data[1]; /* data starts here */ + char data[FLEXIBLE_ARRAY_MEMBER]; /* data starts here */ } TSQueryData; typedef TSQueryData *TSQuery; diff --git a/src/include/utils/catcache.h b/src/include/utils/catcache.h index 8084785..da261e8 100644 --- a/src/include/utils/catcache.h +++ b/src/include/utils/catcache.h @@ -147,8 +147,8 @@ typedef struct catclist uint32 hash_value; /* hash value for lookup keys */ HeapTupleData tuple; /* header for tuple holding keys */ int n_members; /* number of member tuples */ - CatCTup *members[1]; /* members --- VARIABLE LENGTH ARRAY */ -} CatCList; /* VARIABLE LENGTH STRUCT */ + CatCTup *members[FLEXIBLE_ARRAY_MEMBER]; /* members */ +} CatCList; typedef struct catcacheheader diff --git a/src/include/utils/datetime.h b/src/include/utils/datetime.h index 8912ba5..d36f297 100644 --- a/src/include/utils/datetime.h +++ b/src/include/utils/datetime.h @@ -219,7 +219,7 @@ typedef struct TimeZoneAbbrevTable { Size tblsize; /* size in bytes of TimeZoneAbbrevTable */ int numabbrevs; /* number of entries in abbrevs[] array */ - datetkn abbrevs[1]; /* VARIABLE LENGTH ARRAY */ + datetkn abbrevs[FLEXIBLE_ARRAY_MEMBER]; /* DynamicZoneAbbrev(s) may follow the abbrevs[] array */ } TimeZoneAbbrevTable; @@ -227,7 +227,8 @@ typedef struct TimeZoneAbbrevTable typedef struct DynamicZoneAbbrev { pg_tz *tz; /* NULL if not yet looked up */ - char zone[1]; /* zone name (var length, NUL-terminated) */ + char zone[FLEXIBLE_ARRAY_MEMBER]; /* zone name (var length, + * NUL-terminated) */ } DynamicZoneAbbrev; diff --git a/src/include/utils/geo_decls.h b/src/include/utils/geo_decls.h index 0b6d3c3..a0f779b 100644 --- a/src/include/utils/geo_decls.h +++ b/src/include/utils/geo_decls.h @@ -80,7 +80,8 @@ typedef struct int32 npts; int32 closed; /* is this a closed polygon? */ int32 dummy; /* padding to make it double align */ - Point p[1]; /* variable length array of POINTs */ + Point p[FLEXIBLE_ARRAY_MEMBER]; /* variable length array + * of POINTs */ } PATH; @@ -115,7 +116,7 @@ typedef struct int32 vl_len_; /* varlena header (do not touch directly!) */ int32 npts; BOX boundbox; - Point p[1]; /* variable length array of POINTs */ + Point p[FLEXIBLE_ARRAY_MEMBER]; } POLYGON; /*--------------------------------------------------------------------- diff --git a/src/include/utils/jsonb.h b/src/include/utils/jsonb.h index 887eb9b..9d1770e 100644 --- a/src/include/utils/jsonb.h +++ b/src/include/utils/jsonb.h @@ -194,7 +194,7 @@ typedef struct JsonbContainer { uint32 header; /* number of elements or key/value pairs, and * flags */ - JEntry children[1]; /* variable length */ + JEntry children[FLEXIBLE_ARRAY_MEMBER]; /* the data for each child node follows. */ } JsonbContainer; diff --git a/src/include/utils/relmapper.h b/src/include/utils/relmapper.h index 420310d..73b4905 100644 --- a/src/include/utils/relmapper.h +++ b/src/include/utils/relmapper.h @@ -29,7 +29,7 @@ typedef struct xl_relmap_update Oid dbid; /* database ID, or 0 for shared map */ Oid tsid; /* database's tablespace, or pg_global */ int32 nbytes; /* size of relmap data */ - char data[1]; /* VARIABLE LENGTH ARRAY */ + char data[FLEXIBLE_ARRAY_MEMBER]; } xl_relmap_update; #define MinSizeOfRelmapUpdate offsetof(xl_relmap_update, data) diff --git a/src/include/utils/varbit.h b/src/include/utils/varbit.h index 8afc3b1..bd5bf52 100644 --- a/src/include/utils/varbit.h +++ b/src/include/utils/varbit.h @@ -26,7 +26,8 @@ typedef struct { int32 vl_len_; /* varlena header (do not touch directly!) */ int32 bit_len; /* number of valid bits */ - bits8 bit_dat[1]; /* bit string, most sig. byte first */ + bits8 bit_dat[FLEXIBLE_ARRAY_MEMBER]; /* bit string, most sig. + * byte first */ } VarBit; /* -- 2.3.0
From a30ec54acc9bcc997a3a5f5d5c731cbc3b0b21e2 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Thu, 19 Feb 2015 13:40:59 +0900 Subject: [PATCH 2/4] Add forgotten CATALOG_VARLEN pg_authid The laste two fields of this catalog table, rolvaliduntil and rolpassword can be null, but were not marked as such. --- src/include/catalog/pg_authid.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/include/catalog/pg_authid.h b/src/include/catalog/pg_authid.h index e01e6aa..5b34288 100644 --- a/src/include/catalog/pg_authid.h +++ b/src/include/catalog/pg_authid.h @@ -55,9 +55,10 @@ CATALOG(pg_authid,1260) BKI_SHARED_RELATION BKI_ROWTYPE_OID(2842) BKI_SCHEMA_MAC bool rolbypassrls; /* allowed to bypass row level security? */ int32 rolconnlimit; /* max connections allowed (-1=no limit) */ - /* remaining fields may be null; use heap_getattr to read them! */ +#ifdef CATALOG_VARLEN text rolpassword; /* password, if any */ timestamptz rolvaliduntil; /* password expiration time, if any */ +#endif } FormData_pg_authid; #undef timestamptz -- 2.3.0
From d59e066f252fbe9697bc9ef5b9ded0442e853797 Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Thu, 19 Feb 2015 14:02:14 +0900 Subject: [PATCH 3/4] Switch varlena to use FLEXIBLE_ARRAY_MEMBER As compilers normally complain about a flexible-array element not at the end of a structure (clang does, while gcc sometimes does not), this has needed some modifications in structures using bytea as such. This commit ensures as well that those structures have enough room to work as intended as well. --- src/backend/access/heap/tuptoaster.c | 4 ++-- src/backend/storage/large_object/inv_api.c | 8 ++++---- src/include/c.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index f8c1401..547f21f 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -1365,10 +1365,10 @@ toast_save_datum(Relation rel, Datum value, CommandId mycid = GetCurrentCommandId(true); struct varlena *result; struct varatt_external toast_pointer; - struct + union { struct varlena hdr; - char data[TOAST_MAX_CHUNK_SIZE]; /* make struct big enough */ + char data[TOAST_MAX_CHUNK_SIZE + VARHDRSZ]; /* make struct big enough */ int32 align_it; /* ensure struct is aligned well enough */ } chunk_data; int32 chunk_size; diff --git a/src/backend/storage/large_object/inv_api.c b/src/backend/storage/large_object/inv_api.c index a19c401..2e877bc 100644 --- a/src/backend/storage/large_object/inv_api.c +++ b/src/backend/storage/large_object/inv_api.c @@ -562,10 +562,10 @@ inv_write(LargeObjectDesc *obj_desc, const char *buf, int nbytes) bool neednextpage; bytea *datafield; bool pfreeit; - struct + union { bytea hdr; - char data[LOBLKSIZE]; /* make struct big enough */ + char data[LOBLKSIZE + VARHDRSZ]; /* make struct big enough */ int32 align_it; /* ensure struct is aligned well enough */ } workbuf; char *workb = VARDATA(&workbuf.hdr); @@ -748,10 +748,10 @@ inv_truncate(LargeObjectDesc *obj_desc, int64 len) SysScanDesc sd; HeapTuple oldtuple; Form_pg_largeobject olddata; - struct + union { bytea hdr; - char data[LOBLKSIZE]; /* make struct big enough */ + char data[LOBLKSIZE + VARHDRSZ]; /* make struct big enough */ int32 align_it; /* ensure struct is aligned well enough */ } workbuf; char *workb = VARDATA(&workbuf.hdr); diff --git a/src/include/c.h b/src/include/c.h index bbd0d53..36ec468 100644 --- a/src/include/c.h +++ b/src/include/c.h @@ -391,7 +391,7 @@ typedef struct struct varlena { char vl_len_[4]; /* Do not touch this field directly! */ - char vl_dat[1]; + char vl_dat[FLEXIBLE_ARRAY_MEMBER]; }; #define VARHDRSZ ((int32) sizeof(int32)) -- 2.3.0
From f41eea583f806cd8bbb4f604ee81b49c81f69cbe Mon Sep 17 00:00:00 2001 From: Michael Paquier <michael@otacoo.com> Date: Thu, 19 Feb 2015 14:29:57 +0900 Subject: [PATCH 4/4] Replace some struct declarations with union in heapam.c To ensure that there is enough room for operations, add a size equivalent to the tuple header data. --- src/backend/access/heap/heapam.c | 12 ++++++------ src/include/access/htup_details.h | 2 ++ 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index 46060bc1..c4b825d 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -7351,10 +7351,10 @@ heap_xlog_insert(XLogReaderState *record) xl_heap_insert *xlrec = (xl_heap_insert *) XLogRecGetData(record); Buffer buffer; Page page; - struct + union { HeapTupleHeaderData hdr; - char data[MaxHeapTupleSize]; + char data[SizeOfHeapTupleHeaderData + MaxHeapTupleSize]; } tbuf; HeapTupleHeader htup; xl_heap_header xlhdr; @@ -7469,10 +7469,10 @@ heap_xlog_multi_insert(XLogReaderState *record) BlockNumber blkno; Buffer buffer; Page page; - struct + union { HeapTupleHeaderData hdr; - char data[MaxHeapTupleSize]; + char data[SizeOfHeapTupleHeaderData + MaxHeapTupleSize]; } tbuf; HeapTupleHeader htup; uint32 newlen; @@ -7618,10 +7618,10 @@ heap_xlog_update(XLogReaderState *record, bool hot_update) uint16 prefixlen = 0, suffixlen = 0; char *newp; - struct + union { HeapTupleHeaderData hdr; - char data[MaxHeapTupleSize]; + char data[SizeOfHeapTupleHeaderData + MaxHeapTupleSize]; } tbuf; xl_heap_header xlhdr; uint32 newlen; diff --git a/src/include/access/htup_details.h b/src/include/access/htup_details.h index d2ad910..2da4f42 100644 --- a/src/include/access/htup_details.h +++ b/src/include/access/htup_details.h @@ -155,6 +155,8 @@ struct HeapTupleHeaderData /* MORE DATA FOLLOWS AT END OF STRUCT */ }; +#define SizeOfHeapTupleHeaderData MAXALIGN(sizeof(HeapTupleHeaderData)) + /* typedef appears in tupbasics.h */ /* -- 2.3.0
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers