Re: Making psql error out on output failures - Mailing list pgsql-hackers

From Daniel Verite
Subject Re: Making psql error out on output failures
Date
Msg-id 080f5790-35d7-435c-bda8-76052adb4dc7@manitou-mail.org
Whole thread Raw
In response to Re: Making psql error out on output failures  (David Zhang <david.zhang@highgo.ca>)
Responses Re: Making psql error out on output failures  (David Zhang <david.zhang@highgo.ca>)
List 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



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: remove some STATUS_* symbols
Next
From: Fabien COELHO
Date:
Subject: Re: Patch to document base64 encoding