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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robbie Harwood <rharwood(at)redhat(dot)com>, Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>, pgsql-hackers(at)postgresql(dot)org, tgl(at)sss(dot)pgh(dot)pa(dot)us, magnus(at)hagander(dot)net, hlinnaka(at)iki(dot)fi
Subject: Re: Fix some error handling for read() and errno
Date: 2018-06-13 03:52:35
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

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.


Attachment Content-Type Size
read-errno-handling-v4.patch text/x-diff 17.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-06-13 04:15:47 Re: why partition pruning doesn't work?
Previous Message Michael Paquier 2018-06-13 03:11:08 Re: pg_config.h.win32 missing a set of flags from added in v11 development