|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|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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");
> 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.
|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 pg_config.h.in added in v11 development|