Horton
Menu

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

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

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Robert Haas <robertmhaas@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-08-15T18:05:29Z
Lists: pgsql-hackers 
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.

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.