Horton
Menu

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

Thomas Munro <thomas.munro@enterprisedb.com>

View thread
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, David Steele <david@pgmasters.net>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-08-16T00:24:47Z
Lists: pgsql-hackers 
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

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.