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 |
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 |