Horton

Re: Allowing printf("%m") only where it actually works

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

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@enterprisedb.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-05-21T03:54:47Z
Lists: pgsql-hackers 
Thomas Munro <thomas.munro@enterprisedb.com> writes:
> I tried this on macOS and FreeBSD using GCC and Clang: both accept
> printf("%m") without warning and then just print out "m".  It'll be
> interesting to see if the NetBSD patch/idea travels further or some
> other solution can be found.  I've raised this on the freebsd-hackers
> list, let's see... I bet there's other software out there that just
> prints out "m" when things go wrong.  It's arguably something that
> you'd want the complier to understand as a C dialect thing.

Yeah.  ISTM that the netbsd guys didn't get this quite right.  The
gcc docs are perfectly clear about what they think the semantics should
be:

    The parameter archetype determines how the format string is
    interpreted, and should be printf, scanf, strftime, gnu_printf,
    gnu_scanf, gnu_strftime or strfmon. ...  archetype values such as
    printf refer to the formats accepted by the system’s C runtime
    library, while values prefixed with ‘gnu_’ always refer to the formats
    accepted by the GNU C Library.

Therefore, what netbsd should have done was leave the semantics of
"gnu_printf" alone (because glibc undoubtedly does take %m), and just emit
a warning with the "printf" archetype --- which is supposed to describe
the behavior of the platform's stdio, which in their case is known not
to take %m.  If they'd done it that way then they'd have a patch that gcc
upstream certainly ought to accept, because it follows gcc's own stated
semantics for the check.  This would also partially resolve the complaint
Roy Marples had in the thread I alluded to, ie he could use "gnu_printf"
to describe a function that accepts %m.  (There might still need to be
some work to be done to avoid bogus -Wmissing-format-attribute complaints,
not sure.)  I'm not sure whether a separate archetype for syslog is really
needed.  Formally you could say that distinguishing syslog from GNU printf
is wise, but I'm not sure I see a practical need for it.

			regards, tom lane

Commits

  1. In pg_log_generic(), be more paranoid about preserving errno.

  2. Make src/common/exec.c's error logging less ugly.

  3. Select appropriate PG_PRINTF_ATTRIBUTE for recent NetBSD.

  4. Fix detection of the result type of strerror_r().

  5. Try another way to detect the result type of strerror_r().

  6. Clean up *printf macros to avoid conflict with format archetypes.

  7. Fix link failures due to snprintf/strerror changes.

  8. Implement %m in src/port/snprintf.c, and teach elog.c to rely on that.

  9. Always use our own versions of *printf().

  10. Incorporate strerror_r() into src/port/snprintf.c, too.

  11. Convert elog.c's useful_strerror() into a globally-used strerror wrapper.

  12. Revert "Distinguish printf-like functions that support %m from those that don't."

  13. Produce compiler errors if errno is referenced inside elog/ereport calls.

  14. Distinguish printf-like functions that support %m from those that don't.

  15. Fix unportable usage of printf("%m").

  16. Be more robust when strerror() doesn't give a useful result.