On Thu, Feb 19, 2015 at 3:57 PM, Michael Paquier <michael.paqu...@gmail.com> wrote: > 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.
Grr. Completely forgot to use offsetof in dumputils.c as well. Patch that can be applied on top of 0001 is attached. -- Michael
diff --git a/src/bin/pg_dump/dumputils.c b/src/bin/pg_dump/dumputils.c index 892d3fa..3459adc 100644 --- a/src/bin/pg_dump/dumputils.c +++ b/src/bin/pg_dump/dumputils.c @@ -1217,7 +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)); + cell = (SimpleStringListCell *) pg_malloc(offsetof(SimpleStringListCell, val)); cell->next = NULL; strcpy(cell->val, val);
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
