Re: Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]

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