Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King writes: > You actually don't need errno for that. You can write: > > ret = read_in_full(..., size); > if (ret < 0) > die_errno("oops"); > else if (ret != size) > die("short read"); > > So I think using errno as a sentinel value to tell between the two

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Tue, Sep 26, 2017 at 01:11:59PM +0900, Junio C Hamano wrote: > Jeff King writes: > > >> #ifndef EUNDERFLOW > >> # ifdef ENODATA > >> # define EUNDERFLOW ENODATA > >> # else > >> # define EUNDERFLOW ESPIPE > >> # endif > >> #endif > > > > Right, I think our mails just crossed

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King writes: >> #ifndef EUNDERFLOW >> # ifdef ENODATA >> # define EUNDERFLOW ENODATA >> # else >> # define EUNDERFLOW ESPIPE >> # endif >> #endif > > Right, I think our mails just crossed but I'm leaning in this direction. Hmph, I may be slow (or may be skimming the

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > What do you think of patch 7 in light of this? If the short-read case > gives us a sane errno, should we even bother trying to consistently > handle its error separately? I like the read_exactly_or_die variant because it makes callers more concise, but on the other hand a

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 05:20:20PM -0700, Jonathan Nieder wrote: > > What do you think of ENODATA? The glibc text for it is pretty > > appropriate. If it's not available everywhere, we'd have to fallback to > > something else (EIO? 0?). I don't know how esoteric it is. The likely > > candidate to

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > On Mon, Sep 25, 2017 at 05:17:32PM -0700, Jonathan Nieder wrote: >> #ifndef EUNDERFLOW >> # ifdef ENODATA >> # define EUNDERFLOW ENODATA >> # else >> # define EUNDERFLOW ESPIPE >> # endif >> #endif > > Right, I think our mails just crossed but I'm leaning in this direction. >

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 05:17:32PM -0700, Jonathan Nieder wrote: > Jonathan Nieder wrote: > > On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: > > >> ENODATA is not too bad. On my glibc system it yields "No data available" > >> from strerror(), which is at least comprehensible. > >> >

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > I definitely would prefer to avoid EIO, or anything that an actual > read() might return. > > What do you think of ENODATA? The glibc text for it is pretty > appropriate. If it's not available everywhere, we'd have to fallback to > something else (EIO? 0?). I don't know how

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jonathan Nieder wrote: > On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: >> ENODATA is not too bad. On my glibc system it yields "No data available" >> from strerror(), which is at least comprehensible. >> >> We're still left with the question of whether it is defined everywhere >>

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 05:13:54PM -0700, Jonathan Nieder wrote: > > I actually prefer "0" of the all of the options discussed. At least it > > is immediately clear that it is not a syscall error. > > I have a basic aversion to ": Success" in error messages. Whenever I > see such an error

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 08:09:13PM -0400, Jeff King wrote: > On Mon, Sep 25, 2017 at 05:06:58PM -0700, Stefan Beller wrote: >> After reading this discussion from the sideline, maybe >> >> ENODATA No message is available on the STREAM head read >> queue (POSIX.1) >> >> is not too bad

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > On Mon, Sep 25, 2017 at 04:55:10PM -0700, Jonathan Nieder wrote: >> All that really matters is the strerror() that the user would see. > > Agreed, though I didn't think those were necessarily standardized. The standard allows them to vary for the sake of internationalization,

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 05:06:58PM -0700, Stefan Beller wrote: > >> If you are okay with the too-long/too-short confusion in EOVERFLOW, then > >> there is EMSGSIZE: > >> > >> Message too long > > > > I don't really like that one either. > > > > I actually prefer "0" of the all of the

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 5:01 PM, Jeff King wrote: > >> If you are okay with the too-long/too-short confusion in EOVERFLOW, then >> there is EMSGSIZE: >> >> Message too long > > I don't really like that one either. > > I actually prefer "0" of the all of the options

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 04:55:10PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > [EOVERFLOW] > > The file is a regular file, nbyte is greater than 0, the starting > > position is before the end-of-file, and the starting position is > > greater than or equal to the offset

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > [EOVERFLOW] > The file is a regular file, nbyte is greater than 0, the starting > position is before the end-of-file, and the starting position is > greater than or equal to the offset maximum established in the open > file description associated with fildes.

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > On Mon, Sep 25, 2017 at 04:33:16PM -0700, Jonathan Nieder wrote: >> Jef King wrote: >>> errno = 0; >>> read_in_full(fd, buf, sizeof(buf)); >>> if (errno) >>> die_errno("oops"); [...] >>> in general we frown on it for calls like >>> read(). >>

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 07:37:32PM -0400, Jeff King wrote: > > 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

Re: [PATCH 3/7] read_in_full: reset errno before reading

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

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: >> --- a/wrapper.c >> +++ b/wrapper.c >> @@ -318,8 +318,10 @@ ssize_t read_in_full(int fd, void *buf, size_t count) >> ssize_t loaded = xread(fd, p, count); >> if (loaded < 0) >> return -1; >> -if (loaded == 0) >> +

Re: [PATCH 3/7] read_in_full: reset errno before reading

On Mon, Sep 25, 2017 at 03:09:14PM -0700, Jonathan Nieder wrote: > Jeff King wrote: > > > In an ideal world, callers would always distinguish between > > these cases and give a useful message for each. But as an > > easy way to make our imperfect world better, let's reset > > errno to a known

Re: [PATCH 3/7] read_in_full: reset errno before reading

Jeff King wrote: > In an ideal world, callers would always distinguish between > these cases and give a useful message for each. But as an > easy way to make our imperfect world better, let's reset > errno to a known value. The best we can do is "0", which > will yield something like: > >