Re: Unportable coding in reorderbuffer.h

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Unportable coding in reorderbuffer.h
Date: 2014-03-06 00:12:15
Message-ID: 22856.1394064735@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> On 2014-03-05 17:40:56 -0500, Tom Lane wrote:
>> By the time you get done fixing the portability issue, I suspect you
>> won't have a union at all for the first case.

> You might be right. I'd rather not leak the internal enum values to the
> public though, so there are two choices:
> * define as enum, but store values in the that aren't actually valid
> members. IIRC that's legal, even if not pretty. Anything outside
> reorderbuffer.c doesn't ever see the values that aren't enum values.
> * define it as an int, but suggest casting to the enum inside switch()
> in examples/docs.

Meh. I think you're being *way* too cute here. I'd just put all the
values in one enum declaration, and use comments and/or naming convention
to mark some of them as internal and not meant to be used outside the
reorderbuffer stuff. That will for one thing allow gdb to print all
the values properly. And somebody who's bound and determined to violate
the abstraction could do it anyway, no?

>> What drew my attention to it was an older gcc version complaining
>> thusly: [errors]

> And there I was, suprised it hadn't turned the buildfarm red.

I'm surprised too; I had thought we still had some critters running
hoary compilers. We need to do something about that if we actually
believe in C90-compiler support.

I experimented a little bit and found that modern gcc with -ansi
(aka -std=c89) will complain about this particular issue, and it also
complains about // comments which are a perpetual hazard; but it is still
happy about a lot of other not-C90 things like flexible array members.
We could try spinning up a buildfarm critter with -ansi -pedantic
but I'm not sure if that's much better. The GCC man page warns that
these options are not meant to be a strict check for C90 standard
compliance, and anyway the important question is not whether gcc with
options that nobody uses will take our code, its whether old compilers
that are still in real use will take it.

For a long time I was using gcc 2.95.3 on my HPPA dinosaur as a
reference for hoary-compiler behavior. That machine is now flaky
enough that I don't want to keep it turned on 24/7, but maybe I could
compile up 2.95.3 on some other box and start a buildfarm critter.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2014-03-06 00:18:54 Re: missing RelationCloseSmgr in FreeFakeRelcacheEntry?
Previous Message Andres Freund 2014-03-05 23:50:15 Re: Unportable coding in reorderbuffer.h