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

2015-02-22 Thread Michael Paquier
On Sun, Feb 22, 2015 at 6:57 AM, Tom Lane wrote: > After some more hacking, the only remaining uses of foo[1] in struct > declarations are: > > 1. A couple of places where the array is actually the only struct member; > for some unexplainable reason gcc won't let you use flexible array syntax > in

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

2015-02-21 Thread Tom Lane
After some more hacking, the only remaining uses of foo[1] in struct declarations are: 1. A couple of places where the array is actually the only struct member; for some unexplainable reason gcc won't let you use flexible array syntax in that case. 2. struct sqlda_struct in ecpg's sqlda-native.h.

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

2015-02-21 Thread Andres Freund
On 2015-02-21 15:16:55 -0500, Tom Lane wrote: > Andres, would you double-check the changes in reorderbuffer.c? > There were some weird calculations with > offsetof(ReorderBufferTupleBuf, data) - offsetof(HeapTupleHeaderData, t_bits) > which Michael simplified in a way that's not 100% equivalent.

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

2015-02-21 Thread Tom Lane
Michael Paquier writes: > And after all those commits attached is a patch changing > HeapTupleHeaderData, using the following macro to track the size of > the structure: > #define SizeofHeapTupleHeader offsetof(HeapTupleHeaderData, t_bits) I've pushed this with a few minor fixes (mostly, using MA

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

2015-02-21 Thread Michael Paquier
On Fri, Feb 20, 2015 at 8:57 PM, Michael Paquier wrote: > Attached is a new series. 0001 and 0002 are the same, 0003 and 0004 > the backend structures listed previously. I noticed as well that > indexed_tlist in setrefs.c meritates some attention. And after all those commits attached is a patch c

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

2015-02-20 Thread Michael Paquier
On Fri, Feb 20, 2015 at 4:59 PM, Michael Paquier wrote: > > Attached are 3 more patches to improve the coverage (being careful > this time with calls of offsetof and sizeof...): > - 0001 covers varlena in c.h > - 0002 covers HeapTupleHeaderData and MinimalTupleData, with things > changed in code pa

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

2015-02-20 Thread Michael Paquier
On Fri, Feb 20, 2015 at 2:21 PM, Michael Paquier wrote: > On Fri, Feb 20, 2015 at 2:14 PM, Tom Lane wrote: >> Michael Paquier writes: >>> Thanks for the clarifications and the review. Attached is a new set. >> >> I've reviewed and pushed the 0001 patch (you missed a few things :-(). > > My apologi

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

2015-02-19 Thread Michael Paquier
On Fri, Feb 20, 2015 at 2:14 PM, Tom Lane wrote: > Michael Paquier writes: >> Thanks for the clarifications and the review. Attached is a new set. > > I've reviewed and pushed the 0001 patch (you missed a few things :-(). My apologies. I completely forgot to check for any calls of offsetof with th

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

2015-02-19 Thread Tom Lane
Michael Paquier writes: > Thanks for the clarifications and the review. Attached is a new set. I've reviewed and pushed the 0001 patch (you missed a few things :-(). Let's see how unhappy the buildfarm is with this before we start on the rest of them. regards, tom lane

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

2015-02-19 Thread Andres Freund
On 2015-02-18 21:00:43 -0500, Tom Lane wrote: > Michael Paquier writes: > > 3) heapam.c in three places with HeapTupleHeaderData: > > struct > > { > > HeapTupleHeaderData hdr; > > chardata[MaxHeapTupleSize]; > > }

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

2015-02-19 Thread Andres Freund
On 2015-02-18 17:29:27 -0500, Tom Lane wrote: > Michael Paquier writes: > > On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund > > wrote: > >> The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the > >> middle of a struct but not when when you embed a struct that uses it > >> into the

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

2015-02-18 Thread Michael Paquier
On Thu, Feb 19, 2015 at 3:57 PM, Michael Paquier wrote: > On Thu, Feb 19, 2015 at 2:58 PM, Tom Lane wrote: >> Michael Paquier writes: >>> 1-2) sizeof(ParamListInfoData) is present in a couple of places, >>> assuming that sizeof(ParamListInfoData) has the equivalent of 1 >>> parameter, like prepa

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

2015-02-18 Thread Michael Paquier
On Thu, Feb 19, 2015 at 2:58 PM, Tom Lane wrote: > Michael Paquier 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(P

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

2015-02-18 Thread Tom Lane
Michael Paquier 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 */ >

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

2015-02-18 Thread Michael Paquier
On Thu, Feb 19, 2015 at 11:00 AM, Tom Lane wrote: > Michael Paquier writes: >> On Thu, Feb 19, 2015 at 7:29 AM, Tom Lane wrote: >>> Moreover, if we have any code that is assuming such cases are okay, it >>> probably needs a second look. Isn't this situation effectively assuming >>> that a varia

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

2015-02-18 Thread Gavin Flower
On 19/02/15 15:00, Tom Lane wrote: Michael Paquier writes: On Thu, Feb 19, 2015 at 7:29 AM, Tom Lane wrote: Moreover, if we have any code that is assuming such cases are okay, it probably needs a second look. Isn't this situation effectively assuming that a variable-length array is fixed-len

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

2015-02-18 Thread Tom Lane
Michael Paquier writes: > On Thu, Feb 19, 2015 at 7:29 AM, Tom Lane wrote: >> Moreover, if we have any code that is assuming such cases are okay, it >> probably needs a second look. Isn't this situation effectively assuming >> that a variable-length array is fixed-length? > AFAIK, switching a b

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

2015-02-18 Thread Michael Paquier
On Thu, Feb 19, 2015 at 7:29 AM, Tom Lane wrote: > Michael Paquier writes: >> On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund >> wrote: >>> The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the >>> middle of a struct but not when when you embed a struct that uses it >>> into the

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

2015-02-18 Thread Tom Lane
Michael Paquier writes: > On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund > wrote: >> The compiler will complain if you use a FLEXIBLE_ARRAY_MEMBER in the >> middle of a struct but not when when you embed a struct that uses it >> into the middle another struct. At least gcc doesn't and I think i

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

2015-02-18 Thread Michael Paquier
On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund wrote: > On 2015-02-18 17:15:18 +0900, Michael Paquier wrote: >> >> - I don't think that the t_bits fields in htup_details.h should be >> >> updated either. >> > >> > Why not? Any not broken code should already use MinHeapTupleSize and >> > similar m

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

2015-02-18 Thread Andres Freund
On 2015-02-19 07:10:19 +0900, Michael Paquier wrote: > On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund > wrote: > > On 2015-02-18 17:15:18 +0900, Michael Paquier wrote: > >> >> - I don't think that the t_bits fields in htup_details.h should be > >> >> updated either. > >> > > >> > Why not? Any no

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

2015-02-18 Thread Tom Lane
Andres Freund writes: > On 2015-02-16 21:34:57 -0500, Tom Lane wrote: >> Also, if we want to insist that these fields be accessed >> through heap_getattr, I'd be inclined to put them inside the "#ifdef >> CATALOG_VARLEN" to enforce that. > That we definitely should do. It's imo just a small bug t

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

2015-02-18 Thread Andres Freund
On 2015-02-16 21:34:57 -0500, Tom Lane wrote: > Andres Freund writes: > > On 2015-02-17 05:51:22 +0900, Michael Paquier wrote: > >> diff --git a/src/include/catalog/pg_authid.h > >> b/src/include/catalog/pg_authid.h > >> index e01e6aa..d8789a5 100644 > >> --- a/src/include/catalog/pg_authid.h > >

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

2015-02-18 Thread Andres Freund
On 2015-02-18 17:15:18 +0900, Michael Paquier wrote: > >> - I don't think that the t_bits fields in htup_details.h should be > >> updated either. > > > > Why not? Any not broken code should already use MinHeapTupleSize and > > similar macros. > > Changing t_bits impacts HeapTupleHeaderData, Reorde

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

2015-02-18 Thread Michael Paquier
On Tue, Feb 17, 2015 at 9:02 AM, Andres Freund wrote: > On 2015-02-17 05:51:22 +0900, Michael Paquier wrote: >> -- inv_api.c uses bytea in an internal structure without putting it at >> the end of the structure. For clarity I think that we should just use >> a bytea pointer and do a sufficient allo

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

2015-02-16 Thread Tom Lane
Andres Freund writes: > On 2015-02-17 05:51:22 +0900, Michael Paquier wrote: >> diff --git a/src/include/catalog/pg_authid.h >> b/src/include/catalog/pg_authid.h >> index e01e6aa..d8789a5 100644 >> --- a/src/include/catalog/pg_authid.h >> +++ b/src/include/catalog/pg_authid.h >> @@ -56,8 +56,10 @

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

2015-02-16 Thread Andres Freund
On 2015-02-17 05:51:22 +0900, Michael Paquier wrote: > - 0002 does the same for catalog tables > - 0003 changes varlena in c.h. This is done as a separate patch > because this needs some other modifications as variable-length things > need to be placed at the end of structures, because of: > -- rol

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

2015-02-16 Thread Michael Paquier
Hi, In a lot of places in the code we have many structures doing declarations of the type foo[1] to emulate variable length arrays. Attached are a set of patches aimed at replacing that with FLEXIBLE_ARRAY_MEMBER to prevent potential failures that could be caused by compiler optimizations and impr