Re: [PATCH] tempfile: avoid "ferror | fclose" trick

Jeff King writes: > I'm also not sure that it's all that useful to distinguish errors from > fwrite() versus fclose(). In practice, errors will come from write() in > either case, and the caller does not have much control over when the > flushing happens. So any error saying

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 03:52:36PM -0800, Junio C Hamano wrote: > > So I think that "easy" thing falls far short of a solution, but it's at > > least easy. I could take it or leave it at this point. > > Well, I already said that earlier in this thread, and ended up > queuing your patch on 'pu'

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 02:40:19PM -0800, Junio C Hamano wrote: > > Right. EIO is almost certainly _not_ the error we saw. But I would > > rather consistently say "I/O error" and have the user scratch their > > head, look up this thread, and say "ah, it was probably a deferred > > error", as

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

Jeff King writes: >> If we are trying to make sure that the caller would not say "failed >> to close tempfile: ERRNO" with an ERRNO that is unrelated to any >> stdio opration, I am not sure if the patch improves things. The >> caller did not fail to close (most likely we

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 01:42:21PM -0800, Junio C Hamano wrote: > Jeff King writes: > > > On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote: > > > >> Stepping back a bit, would this be really needed? Even if the ferror() > >> does not update errno, the original

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

Jeff King writes: > On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote: > >> Stepping back a bit, would this be really needed? Even if the ferror() >> does not update errno, the original stdio operation that failed >> would have, no? > > Sure, but we have no clue what

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 01:17:06PM -0800, Junio C Hamano wrote: > > I guess we are simultaneously assuming that it is OK to munge errno on > > success in our function, but that fclose() will not do so. Which seems a > > bit hypocritical. Maybe the "if" dance is better. > > Yes. When both

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

Jeff King writes: > On Fri, Feb 17, 2017 at 11:42:25AM +0100, Michael Haggerty wrote: > >> On 02/17/2017 09:07 AM, Jeff King wrote: >> > [...] >> > That's similar to what I wrote earlier, but if we don't mind overwriting >> > errno unconditionally, I think just: >> > >> > errno

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 03:54:42PM -0500, Jeff King wrote: > I guess we are simultaneously assuming that it is OK to munge errno on > success in our function, but that fclose() will not do so. Which seems a > bit hypocritical. Maybe the "if" dance is better. So here's that patch with a

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 11:42:25AM +0100, Michael Haggerty wrote: > On 02/17/2017 09:07 AM, Jeff King wrote: > > [...] > > That's similar to what I wrote earlier, but if we don't mind overwriting > > errno unconditionally, I think just: > > > > errno = EIO; /* covers ferror(), overwritten by

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On 02/17/2017 09:07 AM, Jeff King wrote: > [...] > That's similar to what I wrote earlier, but if we don't mind overwriting > errno unconditionally, I think just: > > errno = EIO; /* covers ferror(), overwritten by failing fclose() */ > err |= ferror(fp); > err |= fclose(fp); > > does the

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On Fri, Feb 17, 2017 at 09:00:09AM +0100, Michael Haggerty wrote: > As you pointed out, if ferror() fails, it doesn't set errno properly. At > least one caller tries to strerror(errno), so it would probably be good > to store *something* in there, probably EIO. Yeah, we discussed this up-thread

Re: [PATCH] tempfile: avoid "ferror | fclose" trick

On 02/16/2017 10:31 PM, Jeff King wrote: > On Thu, Feb 16, 2017 at 11:43:59AM -0500, Jeff King wrote: > >> On Thu, Feb 16, 2017 at 11:10:18AM +0100, Andreas Schwab wrote: >> > int xfclose(FILE *fp) > { > return ferror(fp) | fclose(fp); > } Yes, that's