On Thu, Feb 19, 2015 at 3:57 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> 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.
Grr. Completely forgot to use offsetof in dumputils.c as well. Patch
that can be applied on top of 0001 is attached.
--
Michael
