On Thu, Feb 19, 2015 at 2:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael.paquier@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
