Re: Making psql error out on output failures

From: "Daniel Verite" <daniel(at)manitou-mail(dot)org>
To: "David Zhang" <david(dot)zhang(at)highgo(dot)ca>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Making psql error out on output failures
Date: 2020-01-16 13:20:32
Message-ID: 080f5790-35d7-435c-bda8-76052adb4dc7@manitou-mail.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

David Zhang wrote:

> If I change the error log message like below, where "%m" is used to pass the
> value of strerror(errno), "could not write to output file:" is copied from
> function "WRITE_ERROR_EXIT".
> - pg_log_error("Error printing tuples");
> + pg_log_error("could not write to output file: %m");
> then the output message is something like below, which, I believe, is more
> consistent with pg_dump.

The problem is that errno may not be reliable to tell us what was
the problem that leads to ferror(fout) being nonzero, since it isn't
saved at the point of the error and the output code may have called
many libc functions between the first occurrence of the output error
and when pg_log_error() is called.

The linux manpage on errno warns specifically about this:
<quote from "man errno">
NOTES
A common mistake is to do

if (somecall() == -1) {
printf("somecall() failed\n");
if (errno == ...) { ... }
}

where errno no longer needs to have the value it had upon return
from somecall()
(i.e., it may have been changed by the printf(3)). If the value of
errno should be
preserved across a library call, it must be saved:
</quote>

This other bit from the POSIX spec [1] is relevant:

"The value of errno shall be defined only after a call to a function
for which it is explicitly stated to be set and until it is changed
by the next function call or if the application assigns it a value."

To use errno in a way that complies with the above, the psql code
should be refactored. I don't know if having a more precise error
message justifies that refactoring. I've elaborated a bit about that
upthread with the initial submission. Besides, I'm not even
sure that errno is necessarily set on non-POSIX platforms when fputc
or fputs fails.
That's why this patch uses the safer approach to emit a generic
error message.

[1] https://pubs.opengroup.org/onlinepubs/9699919799/functions/errno.html

Best regards,
--
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2020-01-16 13:41:33 Re: Patch to document base64 encoding
Previous Message Robert Haas 2020-01-16 12:56:38 Re: remove some STATUS_* symbols