C99 compliance for src/port/snprintf.c

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: C99 compliance for src/port/snprintf.c
Date: 2018-08-14 23:28:49
Message-ID: 17245.1534289329@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-www

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

Attachment Content-Type Size
c99-compliant-snprintf-result-1.patch text/x-diff 9.0 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2018-08-14 23:32:28 Re: Postgres, fsync, and OSs (specifically linux)
Previous Message Chapman Flack 2018-08-14 23:17:20 Re: Facility for detecting insecure object naming

Browse pgsql-www by date

  From Date Subject
Next Message Andreas 'ads' Scherbaum 2018-08-15 07:57:38 Re: Updating docbot URLs
Previous Message Andreas 'ads' Scherbaum 2018-08-14 11:17:39 Re: Updating docbot URLs