On Mon, Sep 25, 2017 at 04:33:16PM -0700, Jonathan Nieder wrote:
> > If I do this:
> >
> > errno = 0;
> > read_in_full(fd, buf, sizeof(buf));
> > if (errno)
> > die_errno("oops");
> >
> > then we'll claim an error, even though there was none (remember that
> > it's only an error for _some_ callers to not read the whole length).
> >
> > This may be sufficiently odd that we don't need to care about it. There
> > are some calls (like strtoul) which require this kind of clear-and-check
> > strategy with errno, but in general we frown on it for calls like
> > read().
>
> Correct. Actually more than "frown on": except for with the few calls
> like strtoul that are advertised to work this way, POSIX does not make
> the guarantee the above code would rely on, at all.
>
> So it's not just frowned upon: it's so unportable that the standard
> calls it out as something that won't work.
Is it unportable? Certainly read() is free reset errno to zero on
success. But is it allowed to set it to another random value?
I think we're getting pretty academic here, but I'm curious if you have
a good reference.
> > We could also introduce a better helper like this:
> >
> > int read_exactly(int fd, void *buf, size_t count)
> > {
> > ssize_t ret = read_in_full(fd, buf, count);
> > if (ret < 0)
> > return -1;
> > if (ret != count) {
> > errno = EILSEQ;
> > return -1;
> > }
> > return 0;
> > }
> >
> > Then we know that touching errno always coincides with an error return.
> > And it's shorter to check "< 0" compared to "!= count" in the caller.
> > But of course a caller which wants to distinguish the two cases for its
> > error messages then has to look at errno:
>
> That sounds nice, but it doesn't solve the original problem of callers
> using read_in_full that way.
Right. None of the options discussed in this patch can do so. They can
only take a caller which doesn't distinguish between the cases, and give
it a deterministic value in errno for the short-read case.
IMHO as long as it _is_ deterministic and recognize as not an error from
read(), that's the best we can do. Which is why I went with "0" in the
first place. Seeing "read error: success" is a common-ish idiom (to me
anyway) for "read didn't fail, but some user-space logic did", if only
because it often happens accidentally.
-Peff
