Unportable coding in reorderbuffer.h - Mailing list pgsql-hackers

From Tom Lane
Subject Unportable coding in reorderbuffer.h
Date
Msg-id 31718.1394059256@sss.pgh.pa.us
Whole thread Raw
Responses Re: Unportable coding in reorderbuffer.h  (Andres Freund <andres@2ndquadrant.com>)
List pgsql-hackers
I don't believe that this is legal per C90:

typedef struct ReorderBufferChange
{   XLogRecPtr    lsn;
   /* type of change */   union   {       enum ReorderBufferChangeType action;       /* do not leak internal enum
valuesto the outside */       int            action_internal;   };
 
   ...

That union field needs a name.  Yeah, I realize this makes references
to the contained fields more verbose.  Tough.  Our project standard
is we compile on C90 compilers, and we're not moving that goalpost
just to save you a couple of keystrokes.

Worse, this isn't portable even if you assume a C99 compiler.  There is
nothing at all constraining the compiler to make the enum field the same
size as the int field, and if they're not, reading the int will not yield
the same value you wrote as an enum (or vice versa).  It might've
accidentally failed to fail on the compilers you tested on, but it's
still wrong.

The other nameless union in ReorderBufferChange needs a name too.

By the time you get done fixing the portability issue, I suspect you
won't have a union at all for the first case.  I'm not real sure that
you need a union for the second case either --- is it really important
to shave a few bytes from the size of this struct?  So you don't
necessarily need to do a notation change for the second union.

What drew my attention to it was an older gcc version complaining
thusly:

In file included from ../../../../src/include/replication/decode.h:13,                from decode.c:38:
../../../../src/include/replication/reorderbuffer.h:60: warning: unnamed struct/union that defines no instances
../../../../src/include/replication/reorderbuffer.h:94: warning: unnamed struct/union that defines no instances
decode.c: In function `DecodeInsert':
decode.c:588: structure has no member named `action'
decode.c:589: structure has no member named `tp'
decode.c:595: structure has no member named `tp'
decode.c:599: structure has no member named `tp'
decode.c: In function `DecodeUpdate':
decode.c:628: structure has no member named `action'
decode.c:629: structure has no member named `tp'
decode.c:637: structure has no member named `tp'
decode.c:641: structure has no member named `tp'
decode.c:651: structure has no member named `tp'
decode.c:654: structure has no member named `tp'
... etc etc ...
        regards, tom lane



pgsql-hackers by date:

Previous
From: Stephen Frost
Date:
Subject: Re: jsonb and nested hstore
Next
From: Merlin Moncure
Date:
Subject: Re: jsonb and nested hstore