Horton

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

Thomas Munro <thomas.munro@enterprisedb.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@enterprisedb.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2018-05-27T00:05:38Z
Lists: pgsql-hackers 
On Sun, May 27, 2018 at 4:21 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ...  So that seems like a rather high price to
> pay to deal with what, at present, is a purely hypothetical hazard.
> (Basically what this would protect against is elog_start changing errno,
> which it doesn't.)

Hmm.  It looks like errstart() preserves errno to protect %m not from
itself, but from the caller's other arguments to the elog facility.
That seems reasonable, but do we really need to prohibit direct use of
errno in expressions?  The only rogue actor likely to trash errno is
you, the caller.  I mean, if you call elog(LOG, "foo %d %d", errno,
fsync(bar)) it's obviously UB and your own fault, but who would do
anything like that?  Or maybe I misunderstood the motivation.

> Another approach we could consider is keeping exec.c's one-off approach
> to error handling and letting it redefine pg_prevent_errno_in_scope() as
> empty.  But that's ugly.
>
> Or we could make the affected call sites work like this:
>
>         int save_errno = errno;
>
>         log_error(_("could not identify current directory: %s"),
>                   strerror(save_errno));
>
> which on the whole might be the most expedient thing.

That was what I was going to propose, until I started wondering why we
need to do anything here.

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

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.