On 2020-01-16 5:20 a.m., Daniel Verite wrote:
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.
Yes, I agree with you. For case 2 "select repeat('111', 1000000) \g
/mnt/ramdisk/file", it can be easily fixed with more accurate error
message similar to pg_dump, one example could be something like below.
But for case 1 "psql -d postgres -At -c "select repeat('111', 1000000)"
> /mnt/ramdisk/file" , it may require a lot of refactoring work.
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
index 8fd997553e..e6c239fd9f 100644
--- a/src/port/snprintf.c
+++ b/src/port/snprintf.c
@@ -309,8 +309,10 @@ flushbuffer(PrintfTarget *target)
written = fwrite(target->bufstart, 1, nc, target->stream);
target->nchars += written;
- if (written != nc)
+ if (written != nc) {
target->failed = true;
+ fprintf(stderr, "could not write to output file:
%s\n", strerror(errno));
+ }
Besides, I'm not even
sure that errno is necessarily set on non-POSIX platforms when fputc
or fputs fails.
Verified, fputs does set the errno at least in Ubuntu Linux.
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,
--
David
Software Engineer
Highgo Software Inc. (Canada)
www.highgo.ca
