Re: Fix some error handling for read() and errno

On Tue, Jun 12, 2018 at 01:19:54PM +0900, Michael Paquier wrote:
> Agreed.  I also quite like the message mentioning directly 2PC files as
> well.  I think that we could gain by making all end messages more
> consistent, as the markers used and the style of each message is
> slightly different, so I would suggest something like that instead to
> gain in consistency:
> if (readBytes < 0)
>     ereport(elevel, "could not blah: %m");
> else
>     ereport(elevel, "could not blah: %d read, expected %zu");
>
> My point is that if we use the same markers and the same end messages,
> then those are easier to grep for, and callers are still free to provide
> the head of error messages the way they want depending on the situation.

I have dug again into this stuff, and I have finished with the attached
which uses mainly "could not read file %s: read %d bytes, expected
%zu".  The markers are harder to make consistent without being more
invasive so I stopped on that.

There is also this bit in slru.c which I'd like to discuss:
+       /*
+        * Note that this would report success if the number of bytes read is
+        * positive, but lacking data so that errno is not set, which would be
+        * confusing, so set errno to EIO in this case.
+        */
+       if (errno == 0)
+           errno = EIO;
Please note that I don't necessarily propose to add this in the final
patch, and I think that at least an XXX comment should be added here to
mention that errno may not be set.

Thoughts?
--
Michael