Horton
Menu

Re: C99 compliance for src/port/snprintf.c

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
Cc: Andrew Dunstan <andrew@dunslane.net>, Dave Page <dpage@pgadmin.org>, David Steele <david@pgmasters.net>, pgsql-hackers@lists.postgresql.org
Date: 2018-08-25T17:08:18Z
Lists: pgsql-hackers 
Andres Freund <andres@anarazel.de> writes:
> On 2018-08-16 11:41:30 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> While I'd personally have no problem kicking gcc 3.4 to the curb, I'm
>>> still confused what causes this error mode.  Kinda looks like
>>> out-of-sync headers with gcc or something.

>> Yeah, this is *absolutely* unsurprising for a non-native gcc installation
>> on an old platform.

> Sure, but that still requires the headers to behave differently between
> C89 and C99 mode, as this worked before. But it turns out there's two
> different math.h implementation headers, depending on c99 being enabled
> (math_c99.h being the troublesome).  If I understand correctly the
> problem is more that the system library headers are *newer* (and assume
> a sun studio emulating/copying quite a bit of gcc) than the gcc that's
> being used, and therefore gcc fails.

I have some more info on this issue, based on having successfully
updated "gaur" using gcc 3.4.6 (which I picked because it was the last
of the 3.x release series).  It seems very unlikely that there's much
difference between 3.4.3 and 3.4.6 as far as external features go.
What I find in the 3.4.6 documentation is

 -- Built-in Function: double __builtin_inf (void)
     Similar to `__builtin_huge_val', except a warning is generated if
     the target floating-point format does not support infinities.
     This function is suitable for implementing the ISO C99 macro
     `INFINITY'.

Note that the function is called "__builtin_inf", whereas what we see
protosciurus choking on is "__builtin_infinity".  So I don't think this
is a version skew issue at all.  I think that the system headers are
written for the Solaris cc, and its name for the equivalent function is
__builtin_infinity, whereas what gcc wants is __builtin_inf.  Likewise,
the failures we see for __builtin_isinf and __builtin_isnan are because
Solaris cc provides those but gcc does not.

If we wanted to keep protosciurus going without a compiler update, my
thought would be to modify gcc's copy of math_c99.h to correct the
function name underlying INFINITY, and change the definitions of isinf()
and isnan() back to whatever was being used pre-C99.

It's possible that newer gcc releases have been tweaked so that they
make appropriate corrections in this header file automatically, but
that's not a sure thing.

			regards, tom lane

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Remove test for VA_ARGS, implied by C99.

  2. Introduce minimal C99 usage to verify compiler support.

  3. Require C99 (and thus MSCV 2013 upwards).

  4. Require a C99-compliant snprintf(), and remove related workarounds.

  5. Try to enable C99 in configure, but do not rely on it (yet).

  6. Make snprintf.c follow the C99 standard for snprintf's result value.

  7. Clean up assorted misuses of snprintf()'s result value.