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

On Thu, Feb 19, 2015 at 7:29 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
>> On Wed, Feb 18, 2015 at 10:09 PM, Andres Freund <andres(at)2ndquadrant(dot)com> 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 it'd be
>>> utterly broken if another compiler did that. If there's a compiler that
>>> does so, we need to make it define FLEXIBLE_ARRAY_MEMBER to 1.
>
>> clang does complain on my OSX laptop regarding that ;)
>
> I'm a bit astonished that gcc doesn't consider this an error. Sure seems
> like it should. (Has anyone tried it on recent gcc?)

Just tried with gcc 4.9.2 on an ArchLinux bix and it does not complain
after switching the declaration of varlena declaration from [1] to
FLEXIBLE_ARRAY_MEMBER in c.h on HEAD. But it does with clang 3.5.1 on
the same box.

> I am entirely
> opposed to Andreas' claim that we ought to consider compilers that do warn
> to be broken; if anything it's the other way around.

I'm on board with that.

> 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 bunch of things to use FLEXIBLE_ARRAY_MEMBER has
put a couple of things in light that could be revisited:
1) tuptoaster.c, with this declaration of varlena:
struct
{
struct varlena hdr;
char data[TOAST_MAX_CHUNK_SIZE]; /* make
struct big enough */
int32 align_it; /* ensure struct is
aligned well enough */
} chunk_data;
2) inv_api.c with this thing:
struct
{
bytea hdr;
char data[LOBLKSIZE]; /* make struct
big enough */
int32 align_it; /* ensure struct is
aligned well enough */
} workbuf;
3) heapam.c in three places with HeapTupleHeaderData:
struct
{
HeapTupleHeaderData hdr;
char data[MaxHeapTupleSize];
} tbuf;
4) pg_authid.h with its use of relpasswd.
5) reorderbuffer.h with its use of HeapTupleHeaderData:
typedef struct ReorderBufferTupleBuf
{
/* position in preallocated list */
slist_node node;

/* tuple, stored sequentially */
HeapTupleData tuple;
HeapTupleHeaderData header;
char data[MaxHeapTupleSize];
} ReorderBufferTupleBuf;

Those issues can be grouped depending on where foo[1] is switched to
FLEXIBLE_ARRAY_MEMBER, so I will try to get a set of patches depending
on that.

Regards,
--
Michael