Horton
Menu

Thread

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.

  1. C99 compliance for src/port/snprintf.c

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-14T23:28:49Z

    I noticed that src/port/snprintf.c still follows the old (pre-C99)
semantics for what to return from snprintf() after buffer overrun.
This seems bad for a couple of reasons:

* It results in potential performance loss in psnprintf and
appendPQExpBufferVA, since those have to fly blind about how much
to enlarge the buffer before retrying.

* Given that we override the system-supplied *printf if it doesn't
support the 'z' width modifier, it seems highly unlikely that we are
still using any snprintf's with pre-C99 semantics, except when this
code is used.  So there's not much point in keeping this behavior
as a test case to ensure compatibility with such libraries.

* When we install snprintf.c, port.h forces it to be used everywhere
that includes c.h, including extensions.  It seems quite possible that
there is extension code out there that assumes C99 snprintf behavior.
Such code would work today everywhere except Windows, since that's the
only platform made in this century that requires snprintf.c.  Between
the existing Windows porting hazard and our nearby discussion about
using snprintf.c on more platforms, I'd say this is a gotcha waiting
to bite somebody.

Hence, PFA a patch that changes snprintf.c to follow C99 by counting
dropped bytes in the result of snprintf(), including in the case where
the passed count is zero.

(I also dropped the tests for str == NULL when count isn't zero; that's
not permitted by either C99 or SUSv2, so I see no reason for this code
to support it.  Also, avoid wasting one byte in the local buffer for
*fprintf.)

I'm almost tempted to think that the reasons above make this a
back-patchable bug fix.  Comments?

			regards, tom lane

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

    Robert Haas <robertmhaas@gmail.com> — 2018-08-15T14:58:54Z

    On Tue, Aug 14, 2018 at 7:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I'm almost tempted to think that the reasons above make this a
> back-patchable bug fix.  Comments?

No objections to changing the behavior.  Have you checked whether
there are any noticeable performance consequences?

Back-patching seems a bit aggressive to me considering that the danger
is hypothetical.  I'd want to have some tangible evidence that
back-patching was going help somebody. For all we know somebody's got
an extension which they only use on Windows that happens to be relying
on the current behavior, although more likely still (IMHO) is that it
there is little or no code relying on either behavior.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T15:41:46Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Aug 14, 2018 at 7:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I'm almost tempted to think that the reasons above make this a
>> back-patchable bug fix.  Comments?

> No objections to changing the behavior.  Have you checked whether
> there are any noticeable performance consequences?

Hard to believe there could be any performance loss.  The normal (non
buffer overflowing) code path gets two additional instructions, storing
zero into .nchars and then adding it to the result.  There would be more
work added to buffer-overflow cases, but we'd surely more than win that
back by avoiding repeated repalloc-and-try-again cycles.

> Back-patching seems a bit aggressive to me considering that the danger
> is hypothetical.  I'd want to have some tangible evidence that
> back-patching was going help somebody. For all we know somebody's got
> an extension which they only use on Windows that happens to be relying
> on the current behavior, although more likely still (IMHO) is that it
> there is little or no code relying on either behavior.

Yeah, it's theoretically possible that there's somebody out there who's
depending on the pre-C99 behavior and never tested their code anywhere
but Windows.  But come on, how likely is that really, compared to the
much more plausible risks I already cited?  It's not even that easy
to imagine how someone would construct code that had such a dependency
while not being outright buggy in the face of buffer overrun.

BTW, independently of whether to back-patch, it strikes me that what
we ought to do in HEAD (after applying this) is to just assume we have
C99-compliant behavior, and rip out the baroque logic in psnprintf
and appendPQExpBufferVA that tries to deal with pre-C99 snprintf.
I don't expect that'd save any really meaningful number of cycles,
but at least it'd buy back the two added instructions mentioned above.
I suppose we could put in a configure check that verifies whether
the system snprintf returns the right value for overrun cases, though
it's hard to believe there are any platforms that pass the 'z' check
and would fail this one.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T15:52:37Z

    Hi,

On 2018-08-15 11:41:46 -0400, Tom Lane wrote:
> BTW, independently of whether to back-patch, it strikes me that what
> we ought to do in HEAD (after applying this) is to just assume we have
> C99-compliant behavior, and rip out the baroque logic in psnprintf
> and appendPQExpBufferVA that tries to deal with pre-C99 snprintf.
> I don't expect that'd save any really meaningful number of cycles,
> but at least it'd buy back the two added instructions mentioned above.
> I suppose we could put in a configure check that verifies whether
> the system snprintf returns the right value for overrun cases, though
> it's hard to believe there are any platforms that pass the 'z' check
> and would fail this one.

We could just mandate C99, more generally.

/me goes and hides in a bush.

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

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2018-08-15T15:56:43Z

    On 2018-Aug-15, Robert Haas wrote:

> On Tue, Aug 14, 2018 at 7:28 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I'm almost tempted to think that the reasons above make this a
> > back-patchable bug fix.  Comments?
> 
> No objections to changing the behavior.  Have you checked whether
> there are any noticeable performance consequences?
> 
> Back-patching seems a bit aggressive to me considering that the danger
> is hypothetical.

That was my first thought too, and my preferred path would be to make
this master-only and only consider a backpatch later if we find some
practical reason to do so.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Robert Haas <robertmhaas@gmail.com> — 2018-08-15T16:01:28Z

    On Wed, Aug 15, 2018 at 11:52 AM, Andres Freund <andres@anarazel.de> wrote:
> We could just mandate C99, more generally.
>
> /me goes and hides in a bush.

It's hard to believe that would cost much.  Personally, I'd prefer to
continue avoiding // comments and intermingled declarations of
variables and code on grounds of style and readability.  But it's kind
of difficult to believe that we really need to worry about people
still running 20-year old compilers very much.  My first exposure to
Tom arguing that we ought to continue supporting C89 was about a
decade ago, but the argument that people might still have older
systems in service was a lot more plausible then than it is now.

BTW, I think a bush is probably not a nearly sufficient place to hide.
The wrath of Tom will find you wherever you may go... :-)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

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

    Stephen Frost <sfrost@snowman.net> — 2018-08-15T16:07:40Z

    Greetings,

* Andres Freund (andres@anarazel.de) wrote:
> On 2018-08-15 11:41:46 -0400, Tom Lane wrote:
> > BTW, independently of whether to back-patch, it strikes me that what
> > we ought to do in HEAD (after applying this) is to just assume we have
> > C99-compliant behavior, and rip out the baroque logic in psnprintf
> > and appendPQExpBufferVA that tries to deal with pre-C99 snprintf.
> > I don't expect that'd save any really meaningful number of cycles,
> > but at least it'd buy back the two added instructions mentioned above.
> > I suppose we could put in a configure check that verifies whether
> > the system snprintf returns the right value for overrun cases, though
> > it's hard to believe there are any platforms that pass the 'z' check
> > and would fail this one.
> 
> We could just mandate C99, more generally.

*cough* +1 *cough*

> /me goes and hides in a bush.

/me runs

Thanks!

Stephen

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T16:11:00Z

    Hi,

On 2018-08-15 12:01:28 -0400, Robert Haas wrote:
> On Wed, Aug 15, 2018 at 11:52 AM, Andres Freund <andres@anarazel.de> wrote:
> > We could just mandate C99, more generally.
> >
> > /me goes and hides in a bush.
> 
> It's hard to believe that would cost much.

Yea.


> Personally, I'd prefer to continue avoiding // comments and
> intermingled declarations of variables and code on grounds of style
> and readability.

I don't really care much about either. The calculus for intermingled
declarations would personally change the minute that we decided to allow
some C++ - allowing for scoped locks etc via RAII - but not earlier.

The thing I'd really want is designated initializers for structs. Makes
code for statically allocated structs *so* much more readable. And guess
who's working on code that adds statically allocated structs with lots
of members...


> BTW, I think a bush is probably not a nearly sufficient place to hide.
> The wrath of Tom will find you wherever you may go... :-)

That's why I keep moving. At 200 mph on a train :P. A continent away.

Greetings,

Andres Freund

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T16:17:18Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Wed, Aug 15, 2018 at 11:52 AM, Andres Freund <andres@anarazel.de> wrote:
>> We could just mandate C99, more generally.
>> 
>> /me goes and hides in a bush.

> It's hard to believe that would cost much.

I think we have done that, piece by piece, where it was actually buying us
something.  In particular we've gradually moved the goalposts for *printf
compliance, and I'm proposing here to move them a bit further.  I'm not
sure what "we're going to insist on C99" even means concretely, given
this position ...

> Personally, I'd prefer to
> continue avoiding // comments and intermingled declarations of
> variables and code on grounds of style and readability.

... which I agree with.

> But it's kind
> of difficult to believe that we really need to worry about people
> still running 20-year old compilers very much.

Sure.  It's been a long time since anybody worried about those as
optimization targets, for instance.  Still, I'm not in favor of
actively breaking compatibility unless it buys us something.

			regards, tom lane

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T17:05:39Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2018-Aug-15, Robert Haas wrote:
>> Back-patching seems a bit aggressive to me considering that the danger
>> is hypothetical.

> That was my first thought too, and my preferred path would be to make
> this master-only and only consider a backpatch later if we find some
> practical reason to do so.

Meh --- the hazards of back-patching seem to me to be more hypothetical
than the benefits.  Still, I seem to be in the minority, so I withdraw
the proposal to back-patch.

			regards, tom lane

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

    David Steele <david@pgmasters.net> — 2018-08-15T18:03:16Z

    On 8/15/18 12:17 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
> 
>> Personally, I'd prefer to
>> continue avoiding // comments and intermingled declarations of
>> variables and code on grounds of style and readability.
> 
> ... which I agree with.

We already have -Wdeclaration-after-statement to prevent mixed
declarations.  Not sure what to do about comments except manual enforcement.

>> But it's kind
>> of difficult to believe that we really need to worry about people
>> still running 20-year old compilers very much.
> 
> Sure.  It's been a long time since anybody worried about those as
> optimization targets, for instance.  Still, I'm not in favor of
> actively breaking compatibility unless it buys us something.

We use C99 for the pgBackRest project and we've found designated
initializers, compound declarations, and especially variadic macros to
be very helpful.  Only the latter really provides new functionality but
simplifying and clarifying code is always a bonus.

So, +1 from me!

Regards,
-- 
-David
david@pgmasters.net

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T18:05:29Z

    I wrote:
> Meh --- the hazards of back-patching seem to me to be more hypothetical
> than the benefits.  Still, I seem to be in the minority, so I withdraw
> the proposal to back-patch.

Actually, after digging around a bit, I'm excited about this again.
There are only a couple dozen places in our tree that pay any attention
to the result of (v)snprintf, but with the exception of psnprintf,
appendPQExpBufferVA, and one or two other places, *they're all assuming
C99 semantics*, and will fail to detect buffer overflow with the pre-C99
behavior.

Probably a lot of these are not live bugs because buffer overrun is
not ever going to occur in practice.  But at least pg_upgrade and
pg_regress are constructing command strings including externally
supplied paths, so overrun doesn't seem impossible.  If it happened,
they'd merrily proceed to execute a truncated command.

If we don't backpatch the snprintf change, we're morally obliged to
back-patch some other fix for these places.  At least one of them,
in plperl's pport.h, is not our code and so changing it seems like
a bad idea.

Still want to argue for no backpatch?

			regards, tom lane

PS: I also found a couple of places that are just wrong regardless
of semantics: they're checking overflow by "result > bufsize", not
"result >= bufsize".  Will fix those in any case.

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T18:09:08Z

    David Steele <david@pgmasters.net> writes:
> On 8/15/18 12:17 PM, Tom Lane wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> Personally, I'd prefer to
>>> continue avoiding // comments and intermingled declarations of
>>> variables and code on grounds of style and readability.

>> ... which I agree with.

> We already have -Wdeclaration-after-statement to prevent mixed
> declarations.  Not sure what to do about comments except manual enforcement.

pgindent will change // comments to /* style, so at least on the timescale
of a release cycle, we have enforcement for that.

			regards, tom lane

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

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-15T18:59:46Z

    
On 08/15/2018 12:17 PM, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>
>> Personally, I'd prefer to
>> continue avoiding // comments and intermingled declarations of
>> variables and code on grounds of style and readability.
> ... which I agree with.
>
>


A decade or so ago I would have strongly agreed with you. But the 
language trend seems to be in the other direction. And there is 
something to be said for declaration near use without having to use an 
inner block. I'm not advocating that we change policy, however.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T19:18:57Z

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> On 08/15/2018 12:17 PM, Tom Lane wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> Personally, I'd prefer to
>>> continue avoiding // comments and intermingled declarations of
>>> variables and code on grounds of style and readability.

>> ... which I agree with.

> A decade or so ago I would have strongly agreed with you. But the 
> language trend seems to be in the other direction. And there is 
> something to be said for declaration near use without having to use an 
> inner block. I'm not advocating that we change policy, however.

FWIW, the issue I've got with what C99 did is that you can narrow the
*start* of the scope of a local variable easily, but not the *end* of
its scope, which seems to me to be solving at most half of the problem.
To solve the whole problem, you end up needing a nested block anyway.

I do dearly miss the ability to easily limit the scope of a loop's
control variable to just the loop, eg

	for (int i = 0; ...) { ... }

But AFAIK that's C++ not C99.

			regards, tom lane

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

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-15T19:21:30Z

    
On 08/15/2018 03:18 PM, Tom Lane wrote:
>
> FWIW, the issue I've got with what C99 did is that you can narrow the
> *start* of the scope of a local variable easily, but not the *end* of
> its scope, which seems to me to be solving at most half of the problem.
> To solve the whole problem, you end up needing a nested block anyway.
>
> I do dearly miss the ability to easily limit the scope of a loop's
> control variable to just the loop, eg
>
> 	for (int i = 0; ...) { ... }
>
> But AFAIK that's C++ not C99.
>
> 			



Agree completely.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    David Steele <david@pgmasters.net> — 2018-08-15T19:37:51Z

    On 8/15/18 3:18 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> On 08/15/2018 12:17 PM, Tom Lane wrote:
>>> Robert Haas <robertmhaas@gmail.com> writes:
>>>> Personally, I'd prefer to
>>>> continue avoiding // comments and intermingled declarations of
>>>> variables and code on grounds of style and readability.
> 
>>> ... which I agree with.
> 
>> A decade or so ago I would have strongly agreed with you. But the 
>> language trend seems to be in the other direction. And there is 
>> something to be said for declaration near use without having to use an 
>> inner block. I'm not advocating that we change policy, however.
> 
> FWIW, the issue I've got with what C99 did is that you can narrow the
> *start* of the scope of a local variable easily, but not the *end* of
> its scope, which seems to me to be solving at most half of the problem.
> To solve the whole problem, you end up needing a nested block anyway.
> 
> I do dearly miss the ability to easily limit the scope of a loop's
> control variable to just the loop, eg
> 
> 	for (int i = 0; ...) { ... }
> 
> But AFAIK that's C++ not C99.

This works in C99 -- and I'm a really big fan.

-- 
-David
david@pgmasters.net

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T19:57:43Z

    David Steele <david@pgmasters.net> writes:
> On 8/15/18 3:18 PM, Tom Lane wrote:
>> I do dearly miss the ability to easily limit the scope of a loop's
>> control variable to just the loop, eg
>> 	for (int i = 0; ...) { ... }
>> But AFAIK that's C++ not C99.

> This works in C99 -- and I'm a really big fan.

It does?  [ checks standard... ]  Oh wow:

       6.8.5  Iteration statements

       Syntax

               iteration-statement:
                       while ( expression ) statement
                       do statement while ( expression ) ;
                       for ( expr-opt ; expr-opt ; expr-opt ) statement
                       for ( declaration ; expr-opt ; expr-opt ) statement

I'd always thought this was only in C++.  This alone might be a sufficient
reason to drop C89 compiler support ...

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T21:49:31Z

    Hi,

On 2018-08-15 15:57:43 -0400, Tom Lane wrote:
> I'd always thought this was only in C++.  This alone might be a sufficient
> reason to drop C89 compiler support ...

It's also IIRC reasonably widely supported from before C99. So, for the
sake of designated initializers, for loop scoping, snprintf, let's do
this in master?

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T21:52:17Z

    On 2018-08-15 14:05:29 -0400, Tom Lane wrote:
> I wrote:
> > Meh --- the hazards of back-patching seem to me to be more hypothetical
> > than the benefits.  Still, I seem to be in the minority, so I withdraw
> > the proposal to back-patch.
> 
> Actually, after digging around a bit, I'm excited about this again.
> There are only a couple dozen places in our tree that pay any attention
> to the result of (v)snprintf, but with the exception of psnprintf,
> appendPQExpBufferVA, and one or two other places, *they're all assuming
> C99 semantics*, and will fail to detect buffer overflow with the pre-C99
> behavior.
> 
> Probably a lot of these are not live bugs because buffer overrun is
> not ever going to occur in practice.  But at least pg_upgrade and
> pg_regress are constructing command strings including externally
> supplied paths, so overrun doesn't seem impossible.  If it happened,
> they'd merrily proceed to execute a truncated command.
> 
> If we don't backpatch the snprintf change, we're morally obliged to
> back-patch some other fix for these places.  At least one of them,
> in plperl's pport.h, is not our code and so changing it seems like
> a bad idea.
> 
> Still want to argue for no backpatch?
> 
> 			regards, tom lane
> 
> PS: I also found a couple of places that are just wrong regardless
> of semantics: they're checking overflow by "result > bufsize", not
> "result >= bufsize".  Will fix those in any case.

I'm a bit confused. Why did you just backpatch this ~two hours after
people objected to the idea?  Even if it were during my current work
hours, I don't even read mail that often if I'm hacking on something
complicated.

Greetings,

Andres Freund

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T22:13:59Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-08-15 15:57:43 -0400, Tom Lane wrote:
>> I'd always thought this was only in C++.  This alone might be a sufficient
>> reason to drop C89 compiler support ...

> It's also IIRC reasonably widely supported from before C99. So, for the
> sake of designated initializers, for loop scoping, snprintf, let's do
> this in master?

Nitpick: snprintf is an independent concern: that's from the C library,
not from the compiler.  To drive the point home, I could still test
master on "gaur" if I were to install a just-slightly-newer gcc on that
machine (its existing gcc installation isn't native either...); but
replacing its libc is a nonstarter.

Experimenting here says that even reasonably modern gcc's won't take
declarations-inside-for without "--std=c99" or such.  No idea about
other compilers.  So we'd have a little bit of work to do on
configuration before we could open the floodgates on this.

			regards, tom lane

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T22:18:21Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-08-15 14:05:29 -0400, Tom Lane wrote:
>> Still want to argue for no backpatch?

> I'm a bit confused. Why did you just backpatch this ~two hours after
> people objected to the idea?  Even if it were during my current work
> hours, I don't even read mail that often if I'm hacking on something
> complicated.

If a consensus emerges to deal with this some other way, reverting
isn't hard.  But I think it's pretty clear at this point that we're
dealing with real bugs versus entirely hypothetical bugs, and that's
not a decision that's hard to make IMO.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T22:24:01Z

    Hi,

On 2018-08-15 18:13:59 -0400, Tom Lane wrote:
> Experimenting here says that even reasonably modern gcc's won't take
> declarations-inside-for without "--std=c99" or such.  No idea about
> other compilers.  So we'd have a little bit of work to do on
> configuration before we could open the floodgates on this.

I think autoconf's magic knows about most of that:

 — Macro: AC_PROG_CC_C99

    If the C compiler is not in C99 mode by default, try to add an
    option to output variable CC to make it so. This macro tries various
    options that select C99 on some system or another. It considers the
    compiler to be in C99 mode if it handles _Bool, flexible arrays,
    inline, long long int, mixed code and declarations, named
    initialization of structs, restrict, varargs macros, variable
    declarations in for loops and variable length arrays.

    After calling this macro you can check whether the C compiler has
    been set to accept C99; if not, the shell variable ac_cv_prog_cc_c99
    is set to ‘no’. ~

I think we could get a start by adding that test to configure, without
relying on it for now (i.e. keeping mylodon with -Wc99-extensions
-Werror=c99-extensions alive). That'd tell us about which machines,
besides presumably gaur, we'd need to either kick to the curb or change.

Greetings,

Andres Freund

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T22:31:10Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-08-15 18:13:59 -0400, Tom Lane wrote:
>> Experimenting here says that even reasonably modern gcc's won't take
>> declarations-inside-for without "--std=c99" or such.

> I think autoconf's magic knows about most of that:
>  — Macro: AC_PROG_CC_C99

Ah, of course.  What about the MSVC build?

> I think we could get a start by adding that test to configure, without
> relying on it for now (i.e. keeping mylodon with -Wc99-extensions
> -Werror=c99-extensions alive). That'd tell us about which machines,
> besides presumably gaur, we'd need to either kick to the curb or change.

Sure, no objection to putting that in just to see how much of the
buildfarm can handle it.  If the answer turns out to be "a lot",
we might have to reconsider, but gathering data seems like the
first thing to do.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T22:40:26Z

    Hi,

On 2018-08-15 18:31:10 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-08-15 18:13:59 -0400, Tom Lane wrote:
> >> Experimenting here says that even reasonably modern gcc's won't take
> >> declarations-inside-for without "--std=c99" or such.
> 
> > I think autoconf's magic knows about most of that:
> >  — Macro: AC_PROG_CC_C99
> 
> Ah, of course.  What about the MSVC build?

It looks like it mostly just enables that by default. But I only looked
cursorily.  It's a bit annoying because that makes it harder to be sure
which animals support what.  Looks like e.g. hammerkop (supposedly msvc
2005) might not support the subset we want; not that I'd loose sleep
over raising the minimum msvc in master a bit.


> > I think we could get a start by adding that test to configure, without
> > relying on it for now (i.e. keeping mylodon with -Wc99-extensions
> > -Werror=c99-extensions alive). That'd tell us about which machines,
> > besides presumably gaur, we'd need to either kick to the curb or change.
> 
> Sure, no objection to putting that in just to see how much of the
> buildfarm can handle it.  If the answer turns out to be "a lot",
> we might have to reconsider, but gathering data seems like the
> first thing to do.

Cool. Too late today (in Europe for a few more days), but I'll try to
come up with something tomorrow.

Greetings,

Andres Freund

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

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-08-15T22:54:01Z

    On Thu, Aug 16, 2018 at 10:40 AM, Andres Freund <andres@anarazel.de> wrote:
> Hi,
>
> On 2018-08-15 18:31:10 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>> > On 2018-08-15 18:13:59 -0400, Tom Lane wrote:
>> >> Experimenting here says that even reasonably modern gcc's won't take
>> >> declarations-inside-for without "--std=c99" or such.
>>
>> > I think autoconf's magic knows about most of that:
>> >  — Macro: AC_PROG_CC_C99
>>
>> Ah, of course.  What about the MSVC build?
>
> It looks like it mostly just enables that by default. But I only looked
> cursorily.  It's a bit annoying because that makes it harder to be sure
> which animals support what.  Looks like e.g. hammerkop (supposedly msvc
> 2005) might not support the subset we want; not that I'd loose sleep
> over raising the minimum msvc in master a bit.

Really?  I am not an MSVC user but I had the impression that their C
mode (/TC or files named .c) was stuck on C89/C90 as a matter of
policy, as Herb Sutter explained here (though maybe the situation has
changed since then):

https://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/

That's presumably why cfbot's appveyor build always complains about
people declaring variables after statements.  To allow that particular
language feature, it looks like you have to tell it that your .c file
is really a C++ program with /TP.  But that opens a separate can of
worms, doesn't it?

-- 
Thomas Munro
http://www.enterprisedb.com

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-15T23:00:50Z

    Thomas Munro <thomas.munro@enterprisedb.com> writes:
> Really?  I am not an MSVC user but I had the impression that their C
> mode (/TC or files named .c) was stuck on C89/C90 as a matter of
> policy, as Herb Sutter explained here (though maybe the situation has
> changed since then):

> https://herbsutter.com/2012/05/03/reader-qa-what-about-vc-and-c99/

Hm, I read that and what it says is that they don't plan to support
C99 features that aren't also in C++.  But this one surely is.

How you turn it on (without enabling all of C++) is not very clear
though.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-15T23:06:13Z

    Hi,

On 2018-08-16 10:54:01 +1200, Thomas Munro wrote:
> Really?  I am not an MSVC user but I had the impression that their C
> mode (/TC or files named .c) was stuck on C89/C90 as a matter of
> policy, as Herb Sutter explained here (though maybe the situation has
> changed since then):

They revised their position gradually, starting soon after. They claim
full C99 "language" (vs library, which is also pretty complete)
compliance now.  I've not yet bothered to fully figure out which version
supports what however.  Nor am I really sure about the whole flag thing,
it appears there's a gui element to choose, which we might need to mirror on
the xml level.

A bit of googling shows
https://docs.microsoft.com/en-us/previous-versions/visualstudio/visual-studio-2013/hh409293(v=vs.120)
"
Supports these ISO C99 language features:

    _Bool

    Compound literals.

    Designated initializers.

    Mixing declarations with code.
"

which I think is what we roughly would want.  So it looks like msvc 2013
might be the relevant requirement.


> That's presumably why cfbot's appveyor build always complains about
> people declaring variables after statements.

IIRC even in older versions there's a flag to allow that.


> To allow that particular language feature, it looks like you have to
> tell it that your .c file is really a C++ program with /TP.  But that
> opens a separate can of worms, doesn't it?

Yea, I don't think we want to go there by default soon.

Greetings,

Andres Freund

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-16T00:15:58Z

    I wrote:
> BTW, independently of whether to back-patch, it strikes me that what
> we ought to do in HEAD (after applying this) is to just assume we have
> C99-compliant behavior, and rip out the baroque logic in psnprintf
> and appendPQExpBufferVA that tries to deal with pre-C99 snprintf.

Here's a proposed patch for that.  It also gets rid of some ancient
code that tried to deal with snprintfs that were outright broken,
such as writing past the end of the specified buffer.  Even if anyone
is still using platforms where that's a problem, I'd expect that we'd
have rejected the system snprintf thanks to configure's feature checks.

			regards, tom lane

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

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-08-16T00:24:47Z

    On Thu, Aug 16, 2018 at 11:06 AM, Andres Freund <andres@anarazel.de> wrote:
> On 2018-08-16 10:54:01 +1200, Thomas Munro wrote:
>> Really?  I am not an MSVC user but I had the impression that their C
>> mode (/TC or files named .c) was stuck on C89/C90 as a matter of
>> policy, as Herb Sutter explained here (though maybe the situation has
>> changed since then):
>
> They revised their position gradually, starting soon after. They claim
> full C99 "language" (vs library, which is also pretty complete)
> compliance now.  I've not yet bothered to fully figure out which version
> supports what however.  Nor am I really sure about the whole flag thing,
> it appears there's a gui element to choose, which we might need to mirror on
> the xml level.

Hah, I see.  Thanks apparently due to FFmpeg for helping them change
their minds.  That seems like a bit of a catch-22 for projects that
care about portability.  (Maybe if we start writing C11 they'll change
the compiler to keep up?  Actually I already did that once, with an
anonymous union that turned the build farm slightly red...)

https://stackoverflow.com/questions/27826409/what-is-the-official-status-of-c99-support-in-vs2013

> ...
> which I think is what we roughly would want.  So it looks like msvc 2013
> might be the relevant requirement.

FWIW cfbot is using Visual Studio 2010 right now.  Appveyor provides
2008, 2010, 2012, 2013 (AKA 12.0), 2015, 2017, and to test with a
different toolchain you can take the example patch from
https://wiki.postgresql.org/wiki/Continuous_Integration and add a line
like this to the end of the install section (worked for me; for 2015+
you probably also need to request a different image):

  - 'call "C:\Program Files (x86)\Microsoft Visual Studio
12.0\VC\vcvarsall.bat" x86_amd64'

I'll make that change to cfbot if we decree that it is the new
baseline for PostgreSQL on Windows.  Or I could do it sooner if anyone
wants to be able to post test C99 patches in the Commitfest before we
decide.

-- 
Thomas Munro
http://www.enterprisedb.com

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

    Nico Williams <nico@cryptonector.com> — 2018-08-16T02:57:46Z

    There's also clang on Windows, which VS apparently supports.  With clang
on Windows PG could even make use of GCC/Clang C extensions :^)

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T08:41:34Z

    Hi,

On 2018-08-15 18:31:10 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I think we could get a start by adding that test to configure, without
> > relying on it for now (i.e. keeping mylodon with -Wc99-extensions
> > -Werror=c99-extensions alive). That'd tell us about which machines,
> > besides presumably gaur, we'd need to either kick to the curb or change.
> 
> Sure, no objection to putting that in just to see how much of the
> buildfarm can handle it.  If the answer turns out to be "a lot",
> we might have to reconsider, but gathering data seems like the
> first thing to do.

I've pushed a minimal version adding the C99 test. If we were to
actually go for this permanently, we'd likely want to clean up a bunch
of other tests (say removing PGAC_C_VA_ARGS), but I don't see much point
in doing that while just gathering evidence (to the contrary, it seems
like it'd just muddy the water a bit).

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T11:18:59Z

    On 2018-08-16 01:41:34 -0700, Andres Freund wrote:
> I've pushed a minimal version adding the C99 test.

So, we get:

* lotsa animals, unsurprisingly, showing C99 work without any flags.

  checking for ccache gcc option to accept ISO C99... none needed


* rhinoceros, nudibranch, grouse, ...:

  https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=rhinoceros&dt=2018-08-16%2008%3A45%3A01&stg=configure
  https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=nudibranch&dt=2018-08-16%2009%3A16%3A46&stg=configure
  https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=grouse&dt=2018-08-16%2009%3A17%3A25&stg=configure
  checking for ccache gcc option to accept ISO C99... -std=gnu99 (or variations thereof)

  So, the autoconf magic is doing it's thing here.


* dunlin (icc):

  https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dunlin&dt=2018-08-16%2009%3A35%3A19&stg=configure
  checking for icc option to accept ISO C99... -std=gnu99

  (later fails, but not newly so, and just because of ENOSPC)

* anole (HP C compiler)

  https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=anole&dt=2018-08-16 09%3A32%3A19
  checking for cc option to accept ISO C99... none needed


* dromedary:

  https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=dromedary&dt=2018-08-16%2008%3A37%3A28&stg=configure
  checking for ccache gcc option to accept ISO C99... unsupported

  I suspect that's because of the '-ansi' flag in CFLAGS, not because
  the compiler is incapable of actually supporting C99.


Besides gaur, I'm also awaiting casteroides' results. The latter
definitely does support C99, but I'm not sure autconf pushes hard
enough.  I think every other relevant animal has reported back.

Greetings,

Andres Freund

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

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-16T12:26:07Z

    On 16/08/2018 13:18, Andres Freund wrote:
>   checking for ccache gcc option to accept ISO C99... unsupported
> 
>   I suspect that's because of the '-ansi' flag in CFLAGS, not because
>   the compiler is incapable of actually supporting C99.

-ansi is equivalent to -std=c90.  If we make the switch, the build farm
configuration should just probably replace -ansi with -std=c99.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-16T12:28:25Z

    On 16/08/2018 01:06, Andres Freund wrote:
> So it looks like msvc 2013 might be the relevant requirement.

According to my research (completely untested in practice), you need
2010 for mixed code and declarations and 2013 for named initialization
of structs.

I wonder what raising the msvc requirement would imply for supporting
older Windows versions.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T12:30:50Z

    Hi,

On 2018-08-16 14:26:07 +0200, Peter Eisentraut wrote:
> On 16/08/2018 13:18, Andres Freund wrote:
> >   checking for ccache gcc option to accept ISO C99... unsupported
> > 
> >   I suspect that's because of the '-ansi' flag in CFLAGS, not because
> >   the compiler is incapable of actually supporting C99.
> 
> -ansi is equivalent to -std=c90.  If we make the switch, the build farm
> configuration should just probably replace -ansi with -std=c99.

Right. I just hadn't checked the addition of -std=c99 doesn't override
the -ansi. Presumably just an ordering thing.

Greetings,

Andres Freund

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

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-16T12:54:17Z

    On 16/08/2018 14:30, Andres Freund wrote:
> Hi,
> 
> On 2018-08-16 14:26:07 +0200, Peter Eisentraut wrote:
>> On 16/08/2018 13:18, Andres Freund wrote:
>>>   checking for ccache gcc option to accept ISO C99... unsupported
>>>
>>>   I suspect that's because of the '-ansi' flag in CFLAGS, not because
>>>   the compiler is incapable of actually supporting C99.
>>
>> -ansi is equivalent to -std=c90.  If we make the switch, the build farm
>> configuration should just probably replace -ansi with -std=c99.
> 
> Right. I just hadn't checked the addition of -std=c99 doesn't override
> the -ansi. Presumably just an ordering thing.

The mistake is putting -ansi or similar options into CFLAGS.  You need
to put them into CC.

The Autoconf test for C99 correctly puts the candidate options into CC,
but then if the test compilation invocation is something like "$CC
$CFLAGS -c conftest.c", the -ansi cancels out the earlier -std=c99 or
whatever.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T13:00:30Z

    Hi,

On 2018-08-16 14:28:25 +0200, Peter Eisentraut wrote:
> On 16/08/2018 01:06, Andres Freund wrote:
> > So it looks like msvc 2013 might be the relevant requirement.
> 
> According to my research (completely untested in practice), you need
> 2010 for mixed code and declarations and 2013 for named initialization
> of structs.
> 
> I wonder what raising the msvc requirement would imply for supporting
> older Windows versions.

One relevant tidbit is that afaict 2013 still allows *targeting* older
versions of windows, down to XP and 2003, while requiring a newer
platforms to run. See:
https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2013-compatibility-vs
I don't know if that's hard to do, but I strongly suspect that the
existing installers already do that (otherwise supporting newer versions
would likely require separate builds).

2013 still runs on Windows 7, should you want that:
https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2013-sysrequirements-vs

According to https://www.postgresql.org/download/windows/
the available binaries already effectively restrict windows support:

EDB installers, for 10, restrict to:
64 Bit Windows: 2016, 2012 R2 & R1, 2008 R2, 7, 8, 10
32 Bit Windows: 2008 R1, 7, 8, 10

BIGSQL to: Windows 10 and Windows Server 2012.

Of those 2013 only doesn't run on 2008 R1 anymore. Which still can be
targeted from the newer windows versions.


It'd be good to get confirmation that the windows binaries / installers
are indeed built on newer platforms than the oldest supported version.


Random observation: http://www.openscg.com/bigsql/postgresql/installers/
seems to indicate that packages aren't updated anymore. While it says
"(09-Aug-18)" besides the major versions, it does not actually in fact
have the last set of minor releases.  I suspect that's related to
openscg's acquisition by amazon?  Either they need to catch up, or we
need to take down the page and probably alert people about that fact.

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T14:22:20Z

    Hi,

On 2018-08-16 04:18:59 -0700, Andres Freund wrote:
> Besides gaur, I'm also awaiting casteroides' results. The latter
> definitely does support C99, but I'm not sure autconf pushes hard
> enough.  I think every other relevant animal has reported back.

Casteroides wasn't a problem, -D_STDC_C99= does the trick.

But enabling C99 support triggered a somewhat weird failure on
protosciurus (also solaris, but gcc)
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=protosciurus&dt=2018-08-16%2013%3A37%3A46

It detects C99 support successfully via -std=gnu99 but then fails
somewhere during build with:

gcc -std=gnu99 -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security -fno-strict-aliasing -fwrapv -Wno-unused-command-line-argument -g -I/usr/local/include -m64 -I../../../../src/include    -c -o gistutil.o gistutil.c
In file included from gistutil.c:24:
../../../../src/include/utils/float.h: In function `get_float4_infinity':
../../../../src/include/utils/float.h:71: error: `__builtin_infinity' undeclared (first use in this function)
../../../../src/include/utils/float.h:71: error: (Each undeclared identifier is reported only once
../../../../src/include/utils/float.h:71: error: for each function it appears in.)
../../../../src/include/utils/float.h: In function `get_float8_infinity':
../../../../src/include/utils/float.h:91: error: `__builtin_infinity' undeclared (first use in this function)
../../../../src/include/utils/float.h: In function `get_float4_nan':
../../../../src/include/utils/float.h:108: error: pointer value used where a floating point value was expected
../../../../src/include/utils/float.h: In function `get_float8_nan':
../../../../src/include/utils/float.h:121: error: pointer value used where a floating point value was expected
../../../../src/include/utils/float.h: In function `check_float4_val':
../../../../src/include/utils/float.h:136: warning: implicit declaration of function `__builtin_isinf'
../../../../src/include/utils/float.h: In function `float4_eq':
../../../../src/include/utils/float.h:283: warning: implicit declaration of function `__builtin_isnan'

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.

Greetings,

Andres Freund

  40. Problem with OpenSCG downloads

    Bruce Momjian <bruce@momjian.us> — 2018-08-16T14:35:01Z

    On Thu, Aug 16, 2018 at 06:00:30AM -0700, Andres Freund wrote:
> Random observation: http://www.openscg.com/bigsql/postgresql/installers/
> seems to indicate that packages aren't updated anymore. While it says
> "(09-Aug-18)" besides the major versions, it does not actually in fact
> have the last set of minor releases.  I suspect that's related to
> openscg's acquisition by amazon?  Either they need to catch up, or we
> need to take down the page and probably alert people about that fact.

Forwarding Andres's email above to www for research.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  41. Re: Problem with OpenSCG downloads

    Dave Page <dpage@pgadmin.org> — 2018-08-16T15:00:15Z

    On Thu, Aug 16, 2018 at 3:35 PM, Bruce Momjian <bruce@momjian.us> wrote:

> On Thu, Aug 16, 2018 at 06:00:30AM -0700, Andres Freund wrote:
> > Random observation: http://www.openscg.com/bigsql/postgresql/installers/
> > seems to indicate that packages aren't updated anymore. While it says
> > "(09-Aug-18)" besides the major versions, it does not actually in fact
> > have the last set of minor releases.  I suspect that's related to
> > openscg's acquisition by amazon?  Either they need to catch up, or we
> > need to take down the page and probably alert people about that fact.
>
> Forwarding Andres's email above to www for research.
>

Jimbo assured me at PGCon that Amazon were going to ensure those packages
were kept up to date in the normal schedule.

Jim, do you know what's happening?

-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  42. Re: Problem with OpenSCG downloads

    Jim Mlodgenski <jimm@postgresconf.org> — 2018-08-16T15:19:49Z

    On Thu, Aug 16, 2018 at 11:00 AM, Dave Page <dpage@pgadmin.org> wrote:

>
>
> On Thu, Aug 16, 2018 at 3:35 PM, Bruce Momjian <bruce@momjian.us> wrote:
>
>> On Thu, Aug 16, 2018 at 06:00:30AM -0700, Andres Freund wrote:
>> > Random observation: http://www.openscg.com/bigsql/
>> postgresql/installers/
>> > seems to indicate that packages aren't updated anymore. While it says
>> > "(09-Aug-18)" besides the major versions, it does not actually in fact
>> > have the last set of minor releases.  I suspect that's related to
>> > openscg's acquisition by amazon?  Either they need to catch up, or we
>> > need to take down the page and probably alert people about that fact.
>>
>> Forwarding Andres's email above to www for research.
>>
>
> Jimbo assured me at PGCon that Amazon were going to ensure those packages
> were kept up to date in the normal schedule.
>
> Jim, do you know what's happening?
>
>

Yea, we are working on getting them out ASAP. Because of the acquisition,
our build servers are now sitting in physical locations where people don't
regularly work. In this particular case, they are sitting in our NJ office
which had a power outage long enough the the UPS drained requiring someone
to physically hit the button to power up the servers so we can do the
builds. We're working on moving the builds to this newfangled thing called
the cloud so we don't have the problem in the future. :-)

 I'll ask the team to give me an ETA and report back.

  43. Re: Problem with OpenSCG downloads

    Dave Page <dpage@pgadmin.org> — 2018-08-16T15:21:52Z

    On Thu, Aug 16, 2018 at 4:19 PM, Jim Mlodgenski <jimm@postgresconf.org>
wrote:

>
>
> On Thu, Aug 16, 2018 at 11:00 AM, Dave Page <dpage@pgadmin.org> wrote:
>
>>
>>
>> On Thu, Aug 16, 2018 at 3:35 PM, Bruce Momjian <bruce@momjian.us> wrote:
>>
>>> On Thu, Aug 16, 2018 at 06:00:30AM -0700, Andres Freund wrote:
>>> > Random observation: http://www.openscg.com/bigsql/
>>> postgresql/installers/
>>> > seems to indicate that packages aren't updated anymore. While it says
>>> > "(09-Aug-18)" besides the major versions, it does not actually in fact
>>> > have the last set of minor releases.  I suspect that's related to
>>> > openscg's acquisition by amazon?  Either they need to catch up, or we
>>> > need to take down the page and probably alert people about that fact.
>>>
>>> Forwarding Andres's email above to www for research.
>>>
>>
>> Jimbo assured me at PGCon that Amazon were going to ensure those packages
>> were kept up to date in the normal schedule.
>>
>> Jim, do you know what's happening?
>>
>>
>
> Yea, we are working on getting them out ASAP. Because of the acquisition,
> our build servers are now sitting in physical locations where people don't
> regularly work. In this particular case, they are sitting in our NJ office
> which had a power outage long enough the the UPS drained requiring someone
> to physically hit the button to power up the servers so we can do the
> builds. We're working on moving the builds to this newfangled thing called
> the cloud so we don't have the problem in the future. :-)
>
>  I'll ask the team to give me an ETA and report back.
>
> Thanks Jim.


-- 
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  44. Re: Problem with OpenSCG downloads

    Andres Freund <andres@anarazel.de> — 2018-08-16T15:25:53Z

    Hi,

On 2018-08-16 11:19:49 -0400, Jim Mlodgenski wrote:
> On Thu, Aug 16, 2018 at 11:00 AM, Dave Page <dpage@pgadmin.org> wrote:
> > Jimbo assured me at PGCon that Amazon were going to ensure those packages
> > were kept up to date in the normal schedule.
> >
> > Jim, do you know what's happening?

> Yea, we are working on getting them out ASAP. Because of the acquisition,
> our build servers are now sitting in physical locations where people don't
> regularly work. In this particular case, they are sitting in our NJ office
> which had a power outage long enough the the UPS drained requiring someone
> to physically hit the button to power up the servers so we can do the
> builds. We're working on moving the builds to this newfangled thing called
> the cloud so we don't have the problem in the future. :-)
> 
>  I'll ask the team to give me an ETA and report back.

FWIW, I find this pretty damning given that there's been new security
release for a week: You've added no notes about it to the bigsql
download page. Pinged nobody, to get the downloadlinks temporarily
adorned with a warning on the pg site. And then there's the issue that
the dates besides the releases on the download page are referencing the
date of the newest set of minor releases, but aren't actually new.

This is ridiculously intransparent.

Greetings,

Andres Freund

  45. Re: Problem with OpenSCG downloads

    Justin Clift <justin@postgresql.org> — 2018-08-16T15:32:00Z

    On 2018-08-16 16:25, Andres Freund wrote:
> Hi,
> 
> On 2018-08-16 11:19:49 -0400, Jim Mlodgenski wrote:
>> On Thu, Aug 16, 2018 at 11:00 AM, Dave Page <dpage@pgadmin.org> wrote:
>> > Jimbo assured me at PGCon that Amazon were going to ensure those packages
>> > were kept up to date in the normal schedule.
>> >
>> > Jim, do you know what's happening?
> 
>> Yea, we are working on getting them out ASAP. Because of the 
>> acquisition,
>> our build servers are now sitting in physical locations where people 
>> don't
>> regularly work. In this particular case, they are sitting in our NJ 
>> office
>> which had a power outage long enough the the UPS drained requiring 
>> someone
>> to physically hit the button to power up the servers so we can do the
>> builds. We're working on moving the builds to this newfangled thing 
>> called
>> the cloud so we don't have the problem in the future. :-)
>> 
>>  I'll ask the team to give me an ETA and report back.
> 
> FWIW, I find this pretty damning given that there's been new security
> release for a week: You've added no notes about it to the bigsql
> download page. Pinged nobody, to get the downloadlinks temporarily
> adorned with a warning on the pg site. And then there's the issue that
> the dates besides the releases on the download page are referencing the
> date of the newest set of minor releases, but aren't actually new.
> 
> This is ridiculously intransparent.

Is it fairly simple for us to just comment out/remove the links for now?

We don't want to be pointing people to software with known security 
issues.

We can put the links back in when the updated downloads are in place. :)

+ Justin

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-16T15:41:30Z

    Andres Freund <andres@anarazel.de> writes:
> But enabling C99 support triggered a somewhat weird failure on
> protosciurus (also solaris, but gcc)
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=protosciurus&dt=2018-08-16%2013%3A37%3A46

> It detects C99 support successfully via -std=gnu99 but then fails
> somewhere during build with:
> ../../../../src/include/utils/float.h:71: error: `__builtin_infinity' undeclared (first use in this function)

> 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.  It only works to the extent that the platform's
library headers are up to snuff.  The gcc installation process actually
knows enough to do a certain amount of editing of the platform's headers,
and install "fixed" copies of those headers where gcc will find them
before the ones in /usr/include.  But it looks like the fixing didn't
account for __builtin_infinity on this platform.  Maybe a newer gcc
would get it right.

I just launched gaur/pademelon builds for you, though I'm quite certain
they'll both report "unsupported".  If we go through with this, my plan
would be to retire pademelon (vendor's compiler) and see if I can install
gcc 4.something to replace the 2.95.3 version that gaur is using.  The
-ansi option that dromedary is using is certainly losable, too (I'd
probably replace it with either --std=c99 or --std=gnu99, whichever
autoconf *doesn't* pick by default, just to be contrary).

Andrew is going to need to give us a policy decision on whether to
keep using the existing animal names or take out new ones for these
rather-significantly-modified configurations.

			regards, tom lane

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

    Andres Freund <andres@anarazel.de> — 2018-08-16T16:05:08Z

    Hi,

On 2018-08-16 11:41:30 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > But enabling C99 support triggered a somewhat weird failure on
> > protosciurus (also solaris, but gcc)
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=protosciurus&dt=2018-08-16%2013%3A37%3A46
> 
> > It detects C99 support successfully via -std=gnu99 but then fails
> > somewhere during build with:
> > ../../../../src/include/utils/float.h:71: error: `__builtin_infinity' undeclared (first use in this function)
> 
> > 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.  It only works to the extent that the platform's
> library headers are up to snuff.  The gcc installation process actually
> knows enough to do a certain amount of editing of the platform's headers,
> and install "fixed" copies of those headers where gcc will find them
> before the ones in /usr/include.  But it looks like the fixing didn't
> account for __builtin_infinity on this platform.  Maybe a newer gcc
> would get it right.

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.

Gcc 3.4.3 has been released November 4, 2004, whereas solaris 10 is from
January 31, 2005. The sun studio used for castoroides running on, I
assume, the same hardware, is quite a bit newer in turn.


> I just launched gaur/pademelon builds for you, though I'm quite certain
> they'll both report "unsupported".

Yea, that seems fairly likely.


> If we go through with this, my plan would be to retire pademelon
> (vendor's compiler) and see if I can install gcc 4.something to
> replace the 2.95.3 version that gaur is using.

K.


> The -ansi option that dromedary is using is certainly losable, too
> (I'd probably replace it with either --std=c99 or --std=gnu99,
> whichever autoconf *doesn't* pick by default, just to be contrary).

Makes sense.


> Andrew is going to need to give us a policy decision on whether to
> keep using the existing animal names or take out new ones for these
> rather-significantly-modified configurations.

CCed.

Greetings,

Andres Freund

  48. Re: Problem with OpenSCG downloads

    Andres Freund <andres@anarazel.de> — 2018-08-16T16:25:36Z

    On 2018-08-16 16:32:00 +0100, Justin Clift wrote:
> On 2018-08-16 16:25, Andres Freund wrote:
> > FWIW, I find this pretty damning given that there's been new security
> > release for a week: You've added no notes about it to the bigsql
> > download page. Pinged nobody, to get the downloadlinks temporarily
> > adorned with a warning on the pg site. And then there's the issue that
> > the dates besides the releases on the download page are referencing the
> > date of the newest set of minor releases, but aren't actually new.
> > 
> > This is ridiculously intransparent.
> 
> Is it fairly simple for us to just comment out/remove the links for now?
> 
> We don't want to be pointing people to software with known security issues.
> 
> We can put the links back in when the updated downloads are in place. :)

Probably don't want to remove them entirely, it might prevent people
from upgrading from an even older release with more serious issues. But
a red warning seems appropriate.

Greetings,

Andres Freund

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

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-16T20:36:39Z

    
On 08/16/2018 12:05 PM, Andres Freund wrote:
> Hi,
>
> On 2018-08-16 11:41:30 -0400, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> But enabling C99 support triggered a somewhat weird failure on
>>> protosciurus (also solaris, but gcc)
>>> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=protosciurus&dt=2018-08-16%2013%3A37%3A46
>>> It detects C99 support successfully via -std=gnu99 but then fails
>>> somewhere during build with:
>>> ../../../../src/include/utils/float.h:71: error: `__builtin_infinity' undeclared (first use in this function)
>>> 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.  It only works to the extent that the platform's
>> library headers are up to snuff.  The gcc installation process actually
>> knows enough to do a certain amount of editing of the platform's headers,
>> and install "fixed" copies of those headers where gcc will find them
>> before the ones in /usr/include.  But it looks like the fixing didn't
>> account for __builtin_infinity on this platform.  Maybe a newer gcc
>> would get it right.
> 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.
>
> Gcc 3.4.3 has been released November 4, 2004, whereas solaris 10 is from
> January 31, 2005. The sun studio used for castoroides running on, I
> assume, the same hardware, is quite a bit newer in turn.
>
>
>> I just launched gaur/pademelon builds for you, though I'm quite certain
>> they'll both report "unsupported".
> Yea, that seems fairly likely.
>
>
>> If we go through with this, my plan would be to retire pademelon
>> (vendor's compiler) and see if I can install gcc 4.something to
>> replace the 2.95.3 version that gaur is using.
> K.
>
>
>> The -ansi option that dromedary is using is certainly losable, too
>> (I'd probably replace it with either --std=c99 or --std=gnu99,
>> whichever autoconf *doesn't* pick by default, just to be contrary).
> Makes sense.
>
>
>> Andrew is going to need to give us a policy decision on whether to
>> keep using the existing animal names or take out new ones for these
>> rather-significantly-modified configurations.
> CCed.
>


The usual policy is that animal names need to change if the OS or 
compiler names change, but not if just the versions of those change. 
There is a script in the suite to update those settings, and the admins 
can also help if necessary.

cheers

andrew


-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  50. Re: Problem with OpenSCG downloads

    Bruce Momjian <bruce@momjian.us> — 2018-08-17T02:39:11Z

    On Thu, Aug 16, 2018 at 09:25:36AM -0700, Andres Freund wrote:
> On 2018-08-16 16:32:00 +0100, Justin Clift wrote:
> > On 2018-08-16 16:25, Andres Freund wrote:
> > > FWIW, I find this pretty damning given that there's been new security
> > > release for a week: You've added no notes about it to the bigsql
> > > download page. Pinged nobody, to get the downloadlinks temporarily
> > > adorned with a warning on the pg site. And then there's the issue that
> > > the dates besides the releases on the download page are referencing the
> > > date of the newest set of minor releases, but aren't actually new.
> > > 
> > > This is ridiculously intransparent.
> > 
> > Is it fairly simple for us to just comment out/remove the links for now?
> > 
> > We don't want to be pointing people to software with known security issues.
> > 
> > We can put the links back in when the updated downloads are in place. :)
> 
> Probably don't want to remove them entirely, it might prevent people
> from upgrading from an even older release with more serious issues. But
> a red warning seems appropriate.

Agreed.  We need to do something _now_, and the fact that we are having
to discover this instead of OpenSCG telling us is a good reason to
suspect the use of this download site in the future.

Looking at their website now, does it show they now have the proper
binaries?

	https://www.openscg.com/bigsql/postgresql/installers/
	
	PostgreSQL 10.5 - Stable  (09-Aug-18)
	
	    postgresql-10.5-win64.exe
	    postgresql-10.5-osx64.dmg

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  51. Re: Problem with OpenSCG downloads

    Magnus Hagander <magnus@hagander.net> — 2018-08-17T07:48:26Z

    On Fri, Aug 17, 2018 at 4:39 AM, Bruce Momjian <bruce@momjian.us> wrote:

> On Thu, Aug 16, 2018 at 09:25:36AM -0700, Andres Freund wrote:
> > On 2018-08-16 16:32:00 +0100, Justin Clift wrote:
> > > On 2018-08-16 16:25, Andres Freund wrote:
> > > > FWIW, I find this pretty damning given that there's been new security
> > > > release for a week: You've added no notes about it to the bigsql
> > > > download page. Pinged nobody, to get the downloadlinks temporarily
> > > > adorned with a warning on the pg site. And then there's the issue
> that
> > > > the dates besides the releases on the download page are referencing
> the
> > > > date of the newest set of minor releases, but aren't actually new.
> > > >
> > > > This is ridiculously intransparent.
> > >
> > > Is it fairly simple for us to just comment out/remove the links for
> now?
> > >
> > > We don't want to be pointing people to software with known security
> issues.
> > >
> > > We can put the links back in when the updated downloads are in place.
> :)
> >
> > Probably don't want to remove them entirely, it might prevent people
> > from upgrading from an even older release with more serious issues. But
> > a red warning seems appropriate.
>
> Agreed.  We need to do something _now_, and the fact that we are having
> to discover this instead of OpenSCG telling us is a good reason to
> suspect the use of this download site in the future.
>
> Looking at their website now, does it show they now have the proper
> binaries?
>
>         https://www.openscg.com/bigsql/postgresql/installers/
>
>         PostgreSQL 10.5 - Stable  (09-Aug-18)
>
>             postgresql-10.5-win64.exe
>             postgresql-10.5-osx64.dmg
>
>
Per the filenames it looks like they do. But the dates are still backdated
on them?

Jim, any confirmation on the status?


-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  52. Re: Problem with OpenSCG downloads

    Bruce Momjian <bruce@momjian.us> — 2018-08-17T12:34:28Z

    On Fri, Aug 17, 2018 at 09:48:26AM +0200, Magnus Hagander wrote:
>     Looking at their website now, does it show they now have the proper
>     binaries?
> 
>             https://www.openscg.com/bigsql/postgresql/installers/
> 
>             PostgreSQL 10.5 - Stable  (09-Aug-18)
> 
>                 postgresql-10.5-win64.exe
>                 postgresql-10.5-osx64.dmg
> 
> Per the filenames it looks like they do. But the dates are still backdated on
> them?

Uh, what's the matter with the dates now?  2018-08-09 is the release
date of 10.5.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

  53. Re: Problem with OpenSCG downloads

    Jim Mlodgenski <jimmy76@gmail.com> — 2018-08-17T12:35:28Z

    On Fri, Aug 17, 2018 at 3:48 AM, Magnus Hagander <magnus@hagander.net>
wrote:

>
>
> On Fri, Aug 17, 2018 at 4:39 AM, Bruce Momjian <bruce@momjian.us> wrote:
>
>> On Thu, Aug 16, 2018 at 09:25:36AM -0700, Andres Freund wrote:
>> > On 2018-08-16 16:32:00 +0100, Justin Clift wrote:
>> > > On 2018-08-16 16:25, Andres Freund wrote:
>> > > > FWIW, I find this pretty damning given that there's been new
>> security
>> > > > release for a week: You've added no notes about it to the bigsql
>> > > > download page. Pinged nobody, to get the downloadlinks temporarily
>> > > > adorned with a warning on the pg site. And then there's the issue
>> that
>> > > > the dates besides the releases on the download page are referencing
>> the
>> > > > date of the newest set of minor releases, but aren't actually new.
>> > > >
>> > > > This is ridiculously intransparent.
>> > >
>> > > Is it fairly simple for us to just comment out/remove the links for
>> now?
>> > >
>> > > We don't want to be pointing people to software with known security
>> issues.
>> > >
>> > > We can put the links back in when the updated downloads are in place.
>> :)
>> >
>> > Probably don't want to remove them entirely, it might prevent people
>> > from upgrading from an even older release with more serious issues. But
>> > a red warning seems appropriate.
>>
>> Agreed.  We need to do something _now_, and the fact that we are having
>> to discover this instead of OpenSCG telling us is a good reason to
>> suspect the use of this download site in the future.
>>
>> Looking at their website now, does it show they now have the proper
>> binaries?
>>
>>         https://www.openscg.com/bigsql/postgresql/installers/
>>
>>         PostgreSQL 10.5 - Stable  (09-Aug-18)
>>
>>             postgresql-10.5-win64.exe
>>             postgresql-10.5-osx64.dmg
>>
>>
> Per the filenames it looks like they do. But the dates are still backdated
> on them?
>
> Jim, any confirmation on the status?
>
>
Yes, we pushed the latest installers last night.

The reason for the back date is because we did post new binaries on Aug-9,
but didn't post the new installers until last night. That meant that
existing users of the installers would get the latest updates posted on
Aug-9 if they checked for updates through the mechanism of their existing
install. Also, if new users installed the older version, at the end they
would see there are updates available if they checked. The server we used
to wrap the installers was down which caused the delay.

Sorry for the trouble and we'll be much more proactive of letting everyone
know if we have any difficulty in the future which I don't anticipate
happening.

  54. Re: Problem with OpenSCG downloads

    Magnus Hagander <magnus@hagander.net> — 2018-08-17T12:48:00Z

    On Fri, Aug 17, 2018 at 2:35 PM, Jim Mlodgenski <jimmy76@gmail.com> wrote:

>
>
> On Fri, Aug 17, 2018 at 3:48 AM, Magnus Hagander <magnus@hagander.net>
> wrote:
>
>>
>>
>> On Fri, Aug 17, 2018 at 4:39 AM, Bruce Momjian <bruce@momjian.us> wrote:
>>
>>> On Thu, Aug 16, 2018 at 09:25:36AM -0700, Andres Freund wrote:
>>> > On 2018-08-16 16:32:00 +0100, Justin Clift wrote:
>>> > > On 2018-08-16 16:25, Andres Freund wrote:
>>> > > > FWIW, I find this pretty damning given that there's been new
>>> security
>>> > > > release for a week: You've added no notes about it to the bigsql
>>> > > > download page. Pinged nobody, to get the downloadlinks temporarily
>>> > > > adorned with a warning on the pg site. And then there's the issue
>>> that
>>> > > > the dates besides the releases on the download page are
>>> referencing the
>>> > > > date of the newest set of minor releases, but aren't actually new.
>>> > > >
>>> > > > This is ridiculously intransparent.
>>> > >
>>> > > Is it fairly simple for us to just comment out/remove the links for
>>> now?
>>> > >
>>> > > We don't want to be pointing people to software with known security
>>> issues.
>>> > >
>>> > > We can put the links back in when the updated downloads are in
>>> place. :)
>>> >
>>> > Probably don't want to remove them entirely, it might prevent people
>>> > from upgrading from an even older release with more serious issues. But
>>> > a red warning seems appropriate.
>>>
>>> Agreed.  We need to do something _now_, and the fact that we are having
>>> to discover this instead of OpenSCG telling us is a good reason to
>>> suspect the use of this download site in the future.
>>>
>>> Looking at their website now, does it show they now have the proper
>>> binaries?
>>>
>>>         https://www.openscg.com/bigsql/postgresql/installers/
>>>
>>>         PostgreSQL 10.5 - Stable  (09-Aug-18)
>>>
>>>             postgresql-10.5-win64.exe
>>>             postgresql-10.5-osx64.dmg
>>>
>>>
>> Per the filenames it looks like they do. But the dates are still
>> backdated on them?
>>
>> Jim, any confirmation on the status?
>>
>>
> Yes, we pushed the latest installers last night.
>

Great, thanks for confirming!


The reason for the back date is because we did post new binaries on Aug-9,
> but didn't post the new installers until last night. That meant that
> existing users of the installers would get the latest updates posted on
> Aug-9 if they checked for updates through the mechanism of their existing
> install. Also, if new users installed the older version, at the end they
> would see there are updates available if they checked. The server we used
> to wrap the installers was down which caused the delay.
>

Ah, gotcha. That explains it.


Sorry for the trouble and we'll be much more proactive of letting everyone
> know if we have any difficulty in the future which I don't anticipate
> happening.
>
>
Thanks!


-- 
 Magnus Hagander
 Me: https://www.hagander.net/ <http://www.hagander.net/>
 Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

  55. Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-21T15:56:16Z

    On 16/08/2018 15:00, Andres Freund wrote:
>> According to my research (completely untested in practice), you need
>> 2010 for mixed code and declarations and 2013 for named initialization
>> of structs.
>>
>> I wonder what raising the msvc requirement would imply for supporting
>> older Windows versions.
> One relevant tidbit is that afaict 2013 still allows *targeting* older
> versions of windows, down to XP and 2003, while requiring a newer
> platforms to run. See:
> https://docs.microsoft.com/en-us/visualstudio/productinfo/vs2013-compatibility-vs
> I don't know if that's hard to do, but I strongly suspect that the
> existing installers already do that (otherwise supporting newer versions
> would likely require separate builds).

So, does anyone with Windows build experience want to comment on this?

The proposal is to desupport anything older than (probably) MSVC 2013,
or alternatively anything that cannot compile the attached test file.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  56. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-21T17:29:20Z

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> So, does anyone with Windows build experience want to comment on this?
> The proposal is to desupport anything older than (probably) MSVC 2013,
> or alternatively anything that cannot compile the attached test file.

We've got a buildfarm handy that could answer the question.
Let's just stick a test function in there for a day and see
which animals fail.

			regards, tom lane

  57. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-21T17:31:23Z

    Hi,

On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > So, does anyone with Windows build experience want to comment on this?
> > The proposal is to desupport anything older than (probably) MSVC 2013,
> > or alternatively anything that cannot compile the attached test file.
> 
> We've got a buildfarm handy that could answer the question.
> Let's just stick a test function in there for a day and see
> which animals fail.

I think we pretty much know the answer already, anything before 2013
will fail. The question is more whether that's problematic for the
people building on windows.  My theory, quoted by Peter upthread, is
that it shouldn't be problematic because 2013 can build binaries that
run on XP etc.

Greetings,

Andres Freund

  58. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-21T17:46:10Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
>> We've got a buildfarm handy that could answer the question.
>> Let's just stick a test function in there for a day and see
>> which animals fail.

> I think we pretty much know the answer already, anything before 2013
> will fail.

Do we know that for sure?  I thought it was theoretical.

			regards, tom lane

  59. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-21T17:54:29Z

    Hi,

On 2018-08-21 13:46:10 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
> >> We've got a buildfarm handy that could answer the question.
> >> Let's just stick a test function in there for a day and see
> >> which animals fail.
> 
> > I think we pretty much know the answer already, anything before 2013
> > will fail.
> 
> Do we know that for sure?  I thought it was theoretical.

Pretty much. I'm on mobile data so I don't want to search too much, but
I've previously looked it up, and designated initializer support was
introduced in 2013.  See e.g. the graph in
https://blogs.msdn.microsoft.com/somasegar/2013/06/28/c-conformance-roadmap/

Greetings,

Andres Freund

  60. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Chapman Flack <chap@anastigmatix.net> — 2018-08-21T18:03:26Z

    On 08/21/2018 01:46 PM, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
>>> We've got a buildfarm handy that could answer the question.
>>> Let's just stick a test function in there for a day and see
>>> which animals fail.
> 
>> I think we pretty much know the answer already, anything before 2013
>> will fail.
> 
> Do we know that for sure?  I thought it was theoretical.

I thought I remembered a message where it had been looked up in docs,
but I think the one I was remembering was Peter's "According to my
research (completely untested in practice), you need 2010 for
mixed code and declarations and 2013 for named initialization
of structs." [1] which didn't quite actually say it was documented.

-Chap

[1]
https://www.postgresql.org/message-id/ef986aa7-c7ca-ec34-19d9-fef38716b109%402ndquadrant.com

  61. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-21T18:06:18Z

    
On 08/21/2018 01:31 PM, Andres Freund wrote:
> Hi,
>
> On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
>> Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
>>> So, does anyone with Windows build experience want to comment on this?
>>> The proposal is to desupport anything older than (probably) MSVC 2013,
>>> or alternatively anything that cannot compile the attached test file.
>> We've got a buildfarm handy that could answer the question.
>> Let's just stick a test function in there for a day and see
>> which animals fail.
> I think we pretty much know the answer already, anything before 2013
> will fail. The question is more whether that's problematic for the
> people building on windows.  My theory, quoted by Peter upthread, is
> that it shouldn't be problematic because 2013 can build binaries that
> run on XP etc.
>



XP at least is essentially a dead platform for us. My animals are not 
able to build anything after release 10.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  62. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Joshua D. Drake <jd@commandprompt.com> — 2018-08-21T18:09:15Z

    On 08/21/2018 11:06 AM, Andrew Dunstan wrote:
>
>
>
> XP at least is essentially a dead platform for us. My animals are not 
> able to build anything after release 10.

I wouldn't think XP should even be on our list anymore. Microsoft hasn't 
supported it in 4 years.

JD

-- 
Command Prompt, Inc. || http://the.postgres.company/ || @cmdpromptinc
***  A fault and talent of mine is to tell it exactly how it is.  ***
PostgreSQL centered full stack support, consulting and development.
Advocate: @amplifypostgres || Learn: https://postgresconf.org
*****     Unless otherwise stated, opinions are my own.   *****

  63. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-21T18:10:05Z

    On 2018-08-21 14:06:18 -0400, Andrew Dunstan wrote:
> 
> 
> On 08/21/2018 01:31 PM, Andres Freund wrote:
> > Hi,
> > 
> > On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
> > > Peter Eisentraut <peter.eisentraut@2ndquadrant.com> writes:
> > > > So, does anyone with Windows build experience want to comment on this?
> > > > The proposal is to desupport anything older than (probably) MSVC 2013,
> > > > or alternatively anything that cannot compile the attached test file.
> > > We've got a buildfarm handy that could answer the question.
> > > Let's just stick a test function in there for a day and see
> > > which animals fail.
> > I think we pretty much know the answer already, anything before 2013
> > will fail. The question is more whether that's problematic for the
> > people building on windows.  My theory, quoted by Peter upthread, is
> > that it shouldn't be problematic because 2013 can build binaries that
> > run on XP etc.

> XP at least is essentially a dead platform for us. My animals are not able
> to build anything after release 10.

I'm perfectly fine with that, FWIW. It's out of extended support for
several years now.  But my point was that you can newer versions of MSVC
to build things that run on XP (and more relevantly 2008, vista, 7 etc).
No idea if we'd need to change anything in our build infra for that.

Greetings,

Andres Freund

  64. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-21T20:49:36Z

    On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote:
> On 08/21/2018 11:06 AM, Andrew Dunstan wrote:
> > 
> > 
> > 
> > XP at least is essentially a dead platform for us. My animals are not
> > able to build anything after release 10.
> 
> I wouldn't think XP should even be on our list anymore. Microsoft hasn't
> supported it in 4 years.

XP isn't the only thing relevant here, vista and 2008 R1 are in the same
class.

Greetings,

Andres Freund

  65. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-21T21:58:00Z

    
On 08/21/2018 04:49 PM, Andres Freund wrote:
> On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote:
>> On 08/21/2018 11:06 AM, Andrew Dunstan wrote:
>>>
>>>
>>> XP at least is essentially a dead platform for us. My animals are not
>>> able to build anything after release 10.
>> I wouldn't think XP should even be on our list anymore. Microsoft hasn't
>> supported it in 4 years.
> XP isn't the only thing relevant here, vista and 2008 R1 are in the same
> class.
>


I do have a machine in my laptop graveyard with Vista. The only WS2008 
instace I have available is R2 and AWS doesn't seem to have any AMIs for R1.

Honestly, I don't think these matter terribly much. Anyone building now 
is not likely to be targeting them.

Of the buildfarm animals, dory looks OK, hamerkop, bowerbird and thrips 
look not ok, and whelk and woodlouse look borderline.

I'm perfectly prepared to upgrade bowerbird if necessary.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  66. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-21T23:29:52Z

    On 2018-08-21 17:58:00 -0400, Andrew Dunstan wrote:
> 
> 
> On 08/21/2018 04:49 PM, Andres Freund wrote:
> > On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote:
> > > On 08/21/2018 11:06 AM, Andrew Dunstan wrote:
> > > > 
> > > > 
> > > > XP at least is essentially a dead platform for us. My animals are not
> > > > able to build anything after release 10.
> > > I wouldn't think XP should even be on our list anymore. Microsoft hasn't
> > > supported it in 4 years.
> > XP isn't the only thing relevant here, vista and 2008 R1 are in the same
> > class.
> > 
> 
> 
> I do have a machine in my laptop graveyard with Vista. The only WS2008
> instace I have available is R2 and AWS doesn't seem to have any AMIs for R1.
> 
> Honestly, I don't think these matter terribly much. Anyone building now is
> not likely to be targeting them.

I agree, I think we should just decree that the minimum is MSVC 2013 and
that people building 12 need to deal with that.  I would personally
*additionally* would say that we officially don't support *running* (not
compiling) on XP, 2003, 2008R1 and Vista (all unsupported by MS) - but
that's a somewhat orthogonal decision.

Greetings,

Andres Freund

  67. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Amit Kapila <amit.kapila16@gmail.com> — 2018-08-22T10:28:45Z

    On Tue, Aug 21, 2018 at 11:16 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2018-08-21 13:29:20 -0400, Tom Lane wrote:
>>> We've got a buildfarm handy that could answer the question.
>>> Let's just stick a test function in there for a day and see
>>> which animals fail.
>
>> I think we pretty much know the answer already, anything before 2013
>> will fail.
>
> Do we know that for sure?  I thought it was theoretical.
>

I have MSVC 2010 on my Windows 7 VM where I have tried the C99
constructs provided by Peter E. and below are my findings:

Tried compiling below function in one of the .c files in the src/backend
-----------------------------------------------------------------------------------------------
int
bar()
{
int x;
x = 1;
int y;
y = 2;

for (int i = 0; i < 5; i++)
;

return 0;
}

error C2143: syntax error : missing ';' before 'type'
error C2065: 'y' : undeclared identifier
error C2143: syntax error : missing ';' before 'type'
error C2143: syntax error : missing ';' before 'type'
error C2143: syntax error : missing ')' before 'type'
error C2143: syntax error : missing ';' before 'type'
error C2065: 'i' : undeclared identifier
warning C4552: '<' : operator has no effect; expected operator with side-effect
error C2065: 'i' : undeclared identifier
error C2059: syntax error : ')'

Tried compiling below struct in one of the .c files in the src/backend
-----------------------------------------------------------------------------------------------
struct foo
{
int a;
int b;
};

struct foo f = {
.a = 1,
.b = 2
};

error C2059: syntax error : '.'

I have also tried compiling above in standalone console application
project in MSVC 2010 and able to compile the function successfully,
but struct is giving the same error as above.  So, it seems to me that
it won't work on <= MSVC 2010.  I am personally okay with upgrading my
Windows VM.

I have found a couple of links which suggest that MSVC 2015 has a good
amount of conformance with C99 [1].  It seems MSVC 2013 also has
decent support for C99 [2], but I am not able to verify if the
constructs shared by Peter E can be compiled on it.

[1] https://social.msdn.microsoft.com/Forums/en-US/fa17bcdd-7165-4645-a676-ef3797b95918/details-on-c99-support-in-msvc?forum=vcgeneral
[2] https://blogs.msdn.microsoft.com/vcblog/2013/07/19/c99-library-support-in-visual-studio-2013/

-- 
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

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

    Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> — 2018-08-22T11:44:51Z

    Hi

On Thu, Aug 16, 2018 at 6:30 PM, Andres Freund <andres@anarazel.de> wrote:

> Hi,
>
> On 2018-08-16 14:28:25 +0200, Peter Eisentraut wrote:
> > On 16/08/2018 01:06, Andres Freund wrote:
> > > So it looks like msvc 2013 might be the relevant requirement.
> >
> > According to my research (completely untested in practice), you need
> > 2010 for mixed code and declarations and 2013 for named initialization
> > of structs.
> >
> > I wonder what raising the msvc requirement would imply for supporting
> > older Windows versions.
>
> One relevant tidbit is that afaict 2013 still allows *targeting* older
> versions of windows, down to XP and 2003, while requiring a newer
> platforms to run. See:
> https://docs.microsoft.com/en-us/visualstudio/productinfo/
> vs2013-compatibility-vs
> I don't know if that's hard to do, but I strongly suspect that the
> existing installers already do that (otherwise supporting newer versions
> would likely require separate builds).
>
> 2013 still runs on Windows 7, should you want that:
> https://docs.microsoft.com/en-us/visualstudio/productinfo/
> vs2013-sysrequirements-vs
>
> According to https://www.postgresql.org/download/windows/
> the available binaries already effectively restrict windows support:
>
> EDB installers, for 10, restrict to:
> 64 Bit Windows: 2016, 2012 R2 & R1, 2008 R2, 7, 8, 10
> 32 Bit Windows: 2008 R1, 7, 8, 10
>
> BIGSQL to: Windows 10 and Windows Server 2012.
>
> Of those 2013 only doesn't run on 2008 R1 anymore. Which still can be
> targeted from the newer windows versions.
>
>
> It'd be good to get confirmation that the windows binaries / installers
> are indeed built on newer platforms than the oldest supported version.
>
> We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 R2.
For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use
2013. For v11, we use 2017.


> Random observation: http://www.openscg.com/bigsql/postgresql/installers/
> seems to indicate that packages aren't updated anymore. While it says
> "(09-Aug-18)" besides the major versions, it does not actually in fact
> have the last set of minor releases.  I suspect that's related to
> openscg's acquisition by amazon?  Either they need to catch up, or we
> need to take down the page and probably alert people about that fact.
>
> Greetings,
>
> Andres Freund
>



-- 
Sandeep Thakkar

  69. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> — 2018-08-22T11:47:27Z

    On Wed, Aug 22, 2018 at 4:59 AM, Andres Freund <andres@anarazel.de> wrote:

> On 2018-08-21 17:58:00 -0400, Andrew Dunstan wrote:
> >
> >
> > On 08/21/2018 04:49 PM, Andres Freund wrote:
> > > On 2018-08-21 11:09:15 -0700, Joshua D. Drake wrote:
> > > > On 08/21/2018 11:06 AM, Andrew Dunstan wrote:
> > > > >
> > > > >
> > > > > XP at least is essentially a dead platform for us. My animals are
> not
> > > > > able to build anything after release 10.
> > > > I wouldn't think XP should even be on our list anymore. Microsoft
> hasn't
> > > > supported it in 4 years.
> > > XP isn't the only thing relevant here, vista and 2008 R1 are in the
> same
> > > class.
> > >
> >
> >
> > I do have a machine in my laptop graveyard with Vista. The only WS2008
> > instace I have available is R2 and AWS doesn't seem to have any AMIs for
> R1.
> >
> > Honestly, I don't think these matter terribly much. Anyone building now
> is
> > not likely to be targeting them.
>
> I agree, I think we should just decree that the minimum is MSVC 2013 and
> that people building 12 need to deal with that.  I would personally
> *additionally* would say that we officially don't support *running* (not
> compiling) on XP, 2003, 2008R1 and Vista (all unsupported by MS) - but
> that's a somewhat orthogonal decision.
>
> We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 R2.
For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use
2013. For v11, we use 2017.


> Greetings,
>
> Andres Freund
>



-- 
Sandeep Thakkar

  70. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-22T12:02:11Z

    Hi,

On 2018-08-22 17:17:27 +0530, Sandeep Thakkar wrote:
> > We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012 R2.
> For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use
> 2013. For v11, we use 2017.

Sndeep: Thanks for the information.  Did you ever encounter problems (at
build or during runtime) with using those binaries on older platforms?

Everyone: Given the fact that all the people building windows packages
currently use a new enough stack by a fair margin, I think we should
conclude that there's no obstacle on the windows side of things.


If we agree on that, I'm going to propose a patch that includes:
- relevant cleanups to configure
- adapts sources.sgml to refer to C99 instead of C89
- add some trivial conversions to for(int i;;) and struct initializers,
  so the relevant old animals fail
- adds a configure check to enable errors with vla usage (-Werror=vla)

Questions:

- do we want to make declarations at arbitrary points errors? It's
  already a warning currently.
- other new restrictions that we want to introduce at the same time?

Greetings,

Andres Freund

  71. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Sandeep Thakkar <sandeep.thakkar@enterprisedb.com> — 2018-08-22T12:22:11Z

    On Wed, Aug 22, 2018 at 5:32 PM, Andres Freund <andres@anarazel.de> wrote:

> Hi,
>
> On 2018-08-22 17:17:27 +0530, Sandeep Thakkar wrote:
> > > We build windows binaries (>=9.3) on Windows 7 and Windows Server 2012
> R2.
> > For 9.3, the Visual Studio version is 2010 and for 9.4 and v10, we use
> > 2013. For v11, we use 2017.
>
> Sndeep: Thanks for the information.  Did you ever encounter problems (at
> build or during runtime) with using those binaries on older platforms?
>
> IIRC when the binaries were built with VC++ 2013 on 9.4, we had problems
running them on XP and hence we had used "/p:PlatformToolset=v120_xp"
option to msbuild during build time. From v10, we stopped using that
toolset and instead used the default one i.e v120


> Everyone: Given the fact that all the people building windows packages
> currently use a new enough stack by a fair margin, I think we should
> conclude that there's no obstacle on the windows side of things.
>
>
> If we agree on that, I'm going to propose a patch that includes:
> - relevant cleanups to configure
> - adapts sources.sgml to refer to C99 instead of C89
> - add some trivial conversions to for(int i;;) and struct initializers,
>   so the relevant old animals fail
> - adds a configure check to enable errors with vla usage (-Werror=vla)
>
> Questions:
>
> - do we want to make declarations at arbitrary points errors? It's
>   already a warning currently.
> - other new restrictions that we want to introduce at the same time?
>
> Greetings,
>
> Andres Freund
>



-- 
Sandeep Thakkar

  72. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-22T14:56:15Z

    On 22/08/2018 14:02, Andres Freund wrote:
> If we agree on that, I'm going to propose a patch that includes:
> - relevant cleanups to configure
> - adapts sources.sgml to refer to C99 instead of C89
> - add some trivial conversions to for(int i;;) and struct initializers,
>   so the relevant old animals fail
> - adds a configure check to enable errors with vla usage (-Werror=vla)

sounds good

> - do we want to make declarations at arbitrary points errors? It's
>   already a warning currently.

While there are legitimate criticisms, it's a standard feature in C,
C++, and many other languages, so I don't see what we'd gain by fighting it.

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  73. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-22T15:01:47Z

    Hi,

On 2018-08-22 16:56:15 +0200, Peter Eisentraut wrote:
> On 22/08/2018 14:02, Andres Freund wrote:
> > - do we want to make declarations at arbitrary points errors? It's
> >   already a warning currently.
> 
> While there are legitimate criticisms, it's a standard feature in C,
> C++, and many other languages, so I don't see what we'd gain by fighting it.

I personally don't really care - for C there's really not much of a
difference (contrast to C++ with RAII). But Robert and Tom both said
this would be an issue with moving to C99 for them. I don't want to hold
up making progress here by fighting over this issue.

Greetings,

Andres Freund

  74. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    David Steele <david@pgmasters.net> — 2018-08-22T16:56:01Z

    On 8/22/18 10:56 AM, Peter Eisentraut wrote:
> On 22/08/2018 14:02, Andres Freund wrote:
>> If we agree on that, I'm going to propose a patch that includes:
>> - relevant cleanups to configure
>> - adapts sources.sgml to refer to C99 instead of C89
>> - add some trivial conversions to for(int i;;) and struct initializers,
>>   so the relevant old animals fail
>> - adds a configure check to enable errors with vla usage (-Werror=vla)
> 
> sounds good

Sounds good to me.

> 
>> - do we want to make declarations at arbitrary points errors? It's
>>   already a warning currently.
> 
> While there are legitimate criticisms, it's a standard feature in C,
> C++, and many other languages, so I don't see what we'd gain by fighting it.

+1.=

-- 
-David
david@pgmasters.net

  75. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-23T00:09:05Z

    Hi,

On 2018-08-22 05:02:11 -0700, Andres Freund wrote:
> If we agree on that, I'm going to propose a patch that includes:
> - relevant cleanups to configure
> - adapts sources.sgml to refer to C99 instead of C89
> - add some trivial conversions to for(int i;;) and struct initializers,
>   so the relevant old animals fail
> - adds a configure check to enable errors with vla usage (-Werror=vla)
>
> Questions:
>
> - do we want to make declarations at arbitrary points errors? It's
>   already a warning currently.
> - other new restrictions that we want to introduce at the same time?

Attached is a version doing so. Turns out ripping < msvc 2010 support
allows us to get rid of a fair bit of code.  Using appveyor I tested
that I didn't bungle things too badly.  But I don't really know what I'm
doing in that area, Andrew or Craig, your input would be appreciated.

I tried to update sources.sgml to reflect my understanding of the subset
of C99 we're going to use.  I'd rather start more restrictive and then
argue about relaxing things separately.


There's a few further potential cleanups due to relying on c99:
- Use __func__ unconditionally, rather than having configure test for it
- Use inline unconditionally, rather than having configure test for it
- Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T,
  AC_TYPE_LONG_LONG_INT, we can rely on them being present.
- probably more in that vein


I'd rather do these separately lateron, in case one of them causes
trouble on some animals.


There's some potential ones I think we should *not* do:
- remove AC_C_FLEXIBLE_ARRAY_MEMBER, and rely on it unconditionally.
  I'm disinclined to do this, because C++ IIRC doesn't support it in any
  version, and I don't want to make Peter's life unnecessarily hard.
- remove AC_C_RESTRICT check, rely on it unconditionally: MSVC still
  spells this differently.


Greetings,

Andres Freund

  76. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-23T00:16:24Z

    Andres Freund <andres@anarazel.de> writes:
> There's a few further potential cleanups due to relying on c99:
> - Use __func__ unconditionally, rather than having configure test for it
> - Use inline unconditionally, rather than having configure test for it
> - Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T,
>   AC_TYPE_LONG_LONG_INT, we can rely on them being present.
> - probably more in that vein

I wouldn't be in too much of a hurry to do that, particularly not the
third item.  You are confusing "compiler is c99" with "system headers
are c99".  Moreover, I don't see that we're buying much with such
changes.

			regards, tom lane

  77. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-23T00:19:32Z

    Hi,

On 2018-08-22 20:16:24 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > There's a few further potential cleanups due to relying on c99:
> > - Use __func__ unconditionally, rather than having configure test for it
> > - Use inline unconditionally, rather than having configure test for it
> > - Remove tests for AC_TYPE_INTPTR_T, AC_TYPE_UINTPTR_T,
> >   AC_TYPE_LONG_LONG_INT, we can rely on them being present.
> > - probably more in that vein
> 
> I wouldn't be in too much of a hurry to do that, particularly not the
> third item.  You are confusing "compiler is c99" with "system headers
> are c99".  Moreover, I don't see that we're buying much with such
> changes.

Yea, I am not in much of a hurry on any of them.  I think the only
argument for them is that it'd buy us a littlebit of a reduction in
configure runtime...

Greetings,

Andres Freund

  78. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-23T01:31:34Z

    On 2018-08-22 17:09:05 -0700, Andres Freund wrote:
> Attached is a version doing so.

Mildly updated version attached. This adds an explanatory commit
message, removes one stray docs C89 reference, and fixes a mis-squashing
of a patch.

Greetings,

Andres Freund

  79. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Peter Eisentraut <peter.eisentraut@2ndquadrant.com> — 2018-08-23T20:02:26Z

    On 23/08/2018 03:31, Andres Freund wrote:
> On 2018-08-22 17:09:05 -0700, Andres Freund wrote:
>> Attached is a version doing so.
> 
> Mildly updated version attached. This adds an explanatory commit
> message, removes one stray docs C89 reference, and fixes a mis-squashing
> of a patch.

Let's do it!

-- 
Peter Eisentraut              http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  80. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-24T01:44:34Z

    On 2018-08-23 22:02:26 +0200, Peter Eisentraut wrote:
> On 23/08/2018 03:31, Andres Freund wrote:
> > On 2018-08-22 17:09:05 -0700, Andres Freund wrote:
> >> Attached is a version doing so.
> > 
> > Mildly updated version attached. This adds an explanatory commit
> > message, removes one stray docs C89 reference, and fixes a mis-squashing
> > of a patch.
> 
> Let's do it!

Pushed the first two.  I'll send the presumably affected buildfarm
owners an email, asking them whether they want to update.


Greetings,

Andres Freund

  81. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-08-24T02:25:27Z

    On Fri, Aug 24, 2018 at 1:44 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2018-08-23 22:02:26 +0200, Peter Eisentraut wrote:
>> On 23/08/2018 03:31, Andres Freund wrote:
>> > On 2018-08-22 17:09:05 -0700, Andres Freund wrote:
>> >> Attached is a version doing so.
>> >
>> > Mildly updated version attached. This adds an explanatory commit
>> > message, removes one stray docs C89 reference, and fixes a mis-squashing
>> > of a patch.
>>
>> Let's do it!
>
> Pushed the first two.  I'll send the presumably affected buildfarm
> owners an email, asking them whether they want to update.

Note that designated initialisers are not in C++ yet (though IIUC they
have been accepted in P0329R4[1] for the draft that will become C++20
whatever it turns out to be).

[1] http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/n4691.html

-- 
Thomas Munro
http://www.enterprisedb.com

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

    Thomas Munro <thomas.munro@enterprisedb.com> — 2018-08-24T02:36:51Z

    On Thu, Aug 16, 2018 at 12:24 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> FWIW cfbot is using Visual Studio 2010 right now.  Appveyor provides
> 2008, 2010, 2012, 2013 (AKA 12.0), 2015, 2017, and to test with a
> different toolchain you can take the example patch from
> https://wiki.postgresql.org/wiki/Continuous_Integration and add a line
> like this to the end of the install section (worked for me; for 2015+
> you probably also need to request a different image):
>
>   - 'call "C:\Program Files (x86)\Microsoft Visual Studio
> 12.0\VC\vcvarsall.bat" x86_amd64'
>
> I'll make that change to cfbot if we decree that it is the new
> baseline for PostgreSQL on Windows.

Done.

-- 
Thomas Munro
http://www.enterprisedb.com

  83. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-24T15:46:23Z

    Hi,

On 2018-08-23 18:44:34 -0700, Andres Freund wrote:
> Pushed the first two.

Seems to have worked like expected.

> I'll send the presumably affected buildfarm owners an email, asking
> them whether they want to update.

Did that.


Andrew, as expected my buildfarm animal mylodon, which uses compiler
flags to enforce C89 compliance, failed due to this commit:
https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=mylodon&br=HEAD

I'd like to change it so it doesn't enforce C89 compliance across the
board, but instead enforces the relevant standard. For that I'd need to
change CFLAGS per-branch in the buildfarm. Is that possible already? Do
I need two different config files?

Greetings,

Andres Freund

  84. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-24T16:10:34Z

    Andres Freund <andres@anarazel.de> writes:
> I'd like to change it so it doesn't enforce C89 compliance across the
> board, but instead enforces the relevant standard. For that I'd need to
> change CFLAGS per-branch in the buildfarm. Is that possible already? Do
> I need two different config files?

I just did that on dromedary, with a stanza like this at the bottom:

if ($branch eq 'HEAD' or $branch ge 'REL_12')
{
        $conf{config_env}->{CC} = 'ccache gcc -std=c99';
}
else
{
        $conf{config_env}->{CC} = 'ccache gcc -ansi';
}

			regards, tom lane

  85. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-24T17:02:29Z

    On 2018-08-24 12:10:34 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > I'd like to change it so it doesn't enforce C89 compliance across the
> > board, but instead enforces the relevant standard. For that I'd need to
> > change CFLAGS per-branch in the buildfarm. Is that possible already? Do
> > I need two different config files?
> 
> I just did that on dromedary, with a stanza like this at the bottom:
> 
> if ($branch eq 'HEAD' or $branch ge 'REL_12')
> {
>         $conf{config_env}->{CC} = 'ccache gcc -std=c99';
> }
> else
> {
>         $conf{config_env}->{CC} = 'ccache gcc -ansi';
> }

Thanks, did something similar, mylodon should become green soon. I kinda
was hoping that CFLAGS would directly accept version specific like some
other vars directly...

Greetings,

Andres Freund

  86. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-24T18:09:09Z

    
On 08/24/2018 11:46 AM, Andres Freund wrote:
> Hi,
>
> On 2018-08-23 18:44:34 -0700, Andres Freund wrote:
>> Pushed the first two.
> Seems to have worked like expected.
>
>> I'll send the presumably affected buildfarm owners an email, asking
>> them whether they want to update.
> Did that.



I have installed VS2017 on bowerbird and a test is currently running. 
It's got past the make phase so I assume everything is kosher.

However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. 
Perhaps we should consider backpatching support for those down to 9.3.

If not, I will just restrict bowerbird to building 9.6 and up. That's 
fine by me, we'll still have coverage from, say, currawong, but it's a 
pity we'll only be able to support the older compilers on the old branches.



>
>
> Andrew, as expected my buildfarm animal mylodon, which uses compiler
> flags to enforce C89 compliance, failed due to this commit:
> https://buildfarm.postgresql.org/cgi-bin/show_history.pl?nm=mylodon&br=HEAD
>
> I'd like to change it so it doesn't enforce C89 compliance across the
> board, but instead enforces the relevant standard. For that I'd need to
> change CFLAGS per-branch in the buildfarm. Is that possible already? Do
> I need two different config files?
>
>


I saw Tom's answer, and it will work as far as it goes. But maybe we 
should look at doing that in configure instead of putting the onus on 
all buildfarm owners? It already knows if it's using a GNU compiler, not 
sure how ubiquitous the -ansi and -std=c99 flags are.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  87. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-08-24T18:12:55Z

    Hi,

On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote:
> I have installed VS2017 on bowerbird and a test is currently running. It's
> got past the make phase so I assume everything is kosher.

Cool, thanks.

> However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps
> we should consider backpatching support for those down to 9.3.

Hm, I have no strong objections to that.   I don't think it's strictly
necessary, given 2013 is supported across the board, but for the non MSVC
world, we do fix compiler issues in older branches.  There's not that
much code for the newer versions afaict?

Greetings,

Andres Freund

  88. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-24T18:36:59Z

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
> I saw Tom's answer, and it will work as far as it goes. But maybe we 
> should look at doing that in configure instead of putting the onus on 
> all buildfarm owners? It already knows if it's using a GNU compiler, not 
> sure how ubiquitous the -ansi and -std=c99 flags are.

No, the only reason either of us are doing that is to force use of a
flag that's different from what configure would select by default
(which evidently is -std=gnu99 for gcc).  Most buildfarm owners have
no need to do anything.

			regards, tom lane

  89. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-24T18:38:28Z

    Andres Freund <andres@anarazel.de> writes:
> On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote:
>> However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps
>> we should consider backpatching support for those down to 9.3.

> Hm, I have no strong objections to that.   I don't think it's strictly
> necessary, given 2013 is supported across the board, but for the non MSVC
> world, we do fix compiler issues in older branches.  There's not that
> much code for the newer versions afaict?

+1 for taking a look at how big a patch it would be.  But I kind of
thought we'd intentionally rejected back-patching some of those changes
to begin with, so I'm not sure the end decision will change.

			regards, tom lane

  90. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-24T19:10:02Z

    
On 08/24/2018 02:36 PM, Tom Lane wrote:
> Andrew Dunstan <andrew.dunstan@2ndquadrant.com> writes:
>> I saw Tom's answer, and it will work as far as it goes. But maybe we
>> should look at doing that in configure instead of putting the onus on
>> all buildfarm owners? It already knows if it's using a GNU compiler, not
>> sure how ubiquitous the -ansi and -std=c99 flags are.
> No, the only reason either of us are doing that is to force use of a
> flag that's different from what configure would select by default
> (which evidently is -std=gnu99 for gcc).  Most buildfarm owners have
> no need to do anything.
>
> 			

Ah. Ok. Then your answer is good.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  91. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-08-24T19:42:33Z

    
On 08/24/2018 02:38 PM, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
>> On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote:
>>> However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. Perhaps
>>> we should consider backpatching support for those down to 9.3.
>> Hm, I have no strong objections to that.   I don't think it's strictly
>> necessary, given 2013 is supported across the board, but for the non MSVC
>> world, we do fix compiler issues in older branches.  There's not that
>> much code for the newer versions afaict?
> +1 for taking a look at how big a patch it would be.  But I kind of
> thought we'd intentionally rejected back-patching some of those changes
> to begin with, so I'm not sure the end decision will change.

The VS2017 patch applies cleanly to 9.5, so that seems easy. The VS2015 
patch from 9.5 needs a very small amount of adjustment by the look of it 
for 9.3 and 9.4, after which I hope the VS2017 patch would again apply 
cleanly.

I'll try to put this together.

The trouble with not back patching support to all live branches as new 
versions come in is that it acts as a significant discouragement to 
buildfarm owners to use the latest Visual Studio versions. I've never 
argued stringly on this point before, but I think i'm goiung to ber 
inclined to in future.

Meanwhile, I will turn bowerbird back on but just for >= 9.6 for now.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

    Tom Lane <tgl@sss.pgh.pa.us> — 2018-08-25T17:08:18Z

    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

  93. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andrew Dunstan <andrew.dunstan@2ndquadrant.com> — 2018-09-11T20:13:15Z

    
On 08/24/2018 03:42 PM, Andrew Dunstan wrote:
>
>
> On 08/24/2018 02:38 PM, Tom Lane wrote:
>> Andres Freund <andres@anarazel.de> writes:
>>> On 2018-08-24 14:09:09 -0400, Andrew Dunstan wrote:
>>>> However, we only support VS2017 down to 9.6 and Vs2015 down to 9.5. 
>>>> Perhaps
>>>> we should consider backpatching support for those down to 9.3.
>>> Hm, I have no strong objections to that.   I don't think it's strictly
>>> necessary, given 2013 is supported across the board, but for the non 
>>> MSVC
>>> world, we do fix compiler issues in older branches.  There's not that
>>> much code for the newer versions afaict?
>> +1 for taking a look at how big a patch it would be.  But I kind of
>> thought we'd intentionally rejected back-patching some of those changes
>> to begin with, so I'm not sure the end decision will change.
>
> The VS2017 patch applies cleanly to 9.5, so that seems easy. The 
> VS2015 patch from 9.5 needs a very small amount of adjustment by the 
> look of it for 9.3 and 9.4, after which I hope the VS2017 patch would 
> again apply cleanly.
>
> I'll try to put this together.
>
> The trouble with not back patching support to all live branches as new 
> versions come in is that it acts as a significant discouragement to 
> buildfarm owners to use the latest Visual Studio versions. I've never 
> argued stringly on this point before, but I think i'm goiung to ber 
> inclined to in future.
>
> Meanwhile, I will turn bowerbird back on but just for >= 9.6 for now.
>


I've pushed support for the latest MSVC compilers back to all live branches.

cheers

andrew

-- 
Andrew Dunstan                https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  94. Re: Windows vs C99 (was Re: C99 compliance for src/port/snprintf.c)

    Andres Freund <andres@anarazel.de> — 2018-09-11T20:27:24Z

    Hi,

On 2018-09-11 16:13:15 -0400, Andrew Dunstan wrote:
> I've pushed support for the latest MSVC compilers back to all live branches.

Thanks!

Greetings,

Andres Freund

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

    Andres Freund <andres@anarazel.de> — 2018-09-28T15:04:34Z

    Hi,

On 2018-08-25 13:08:18 -0400, Tom Lane wrote:
> 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.

I've pinged Dave about this, and he said:

On 2018-09-26 17:04:29 -0400, Dave Page wrote:
> Unfortunately, i think that whole machine is basically EOL now. It's
> already skipping OpenSSL for some builds, as being stuck on a very old
> version of the buildfarm client, as some of the modules used in newer
> versions just won't compile or work. I don't have any support contract, or
> access to newer versions of SunStudio, and the guys that used to package
> GCC for Solaris now charge to download their packages.
> 
> I could potentially build my own version of GCC, but I question whether
> it's really worth it, given the other problems.

He's now disabled building master on protosciurus and casteroides.  We
still have damselfly and rover_firefly so I don't feel too bad about
that.  I've pinged their owners to ask whether they could set up a sun
studio (or however that's called in their solaris descendants) version.

Greetings,

Andres Freund