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

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andres Freund <andres(at)2ndquadrant(dot)com>, PostgreSQL mailing lists <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Expanding the use of FLEXIBLE_ARRAY_MEMBER for declarations like foo[1]
Date: 2015-02-19 06:57:31
Message-ID: CAB7nPqSY6B-jePz8Mk9m=9sd-h1jVuupmvJcy4P99Q+qkBgT6A@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Attachment Content-Type Size
0001-First-cut-with-FLEXIBLE_ARRAY_MEMBER.patch application/x-patch 30.5 KB
0002-Add-forgotten-CATALOG_VARLEN-pg_authid.patch application/x-patch 1.1 KB
0003-Switch-varlena-to-use-FLEXIBLE_ARRAY_MEMBER.patch application/x-patch 2.8 KB
0004-Replace-some-struct-declarations-with-union-in-heapa.patch application/x-patch 2.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shigeru Hanada 2015-02-19 07:19:07 Re: Join push-down support for foreign tables
Previous Message Peter Geoghegan 2015-02-19 06:43:25 Re: INSERT ... ON CONFLICT {UPDATE | IGNORE} 2.0