Thomas Munro <thomas.munro@gmail.com> writes:
> As noted by Amit Khandhekar yesterday[1], BufFileLoad() silently eats
> pread()'s error and makes them indistinguishable from EOF.
That's definitely bad.
> I think the choices are: (1) switch to ssize_t and return -1, (2) add
> at least one of BufFileEof(), BufFileError(), (3) have BufFileRead()
> raise errors with elog(). I lean towards (2), and I doubt we need
> BufFileClear() because the only thing we ever do in client code on
> error is immediately burn the world down.
I'd vote for (3), I think. Making the callers responsible for error
checks just leaves us with a permanent hazard of errors-of-omission,
and as you say, there's really no use-case where we'd be trying to
recover from the error.
I think that the motivation for making the caller do it might've
been an idea that the caller could provide a more useful error
message, but I'm not real sure that that's true --- the caller
doesn't know the physical file's name, and it doesn't necessarily
have the right errno either.
Possibly we could address any loss of usefulness by requiring callers
to pass some sort of context identification to BufFileCreate?
regards, tom lane