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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 a bit, and my "solution" was similar
to yours. I don't like it, because EIO is a real thing that can happen,
too, and it would certainly be surprising to a user to see. But it's
probably better than the alternative, which is getting whatever random
value happened to be in errno.

The only downside is that if the value of errno _was_ valid (because the
last thing you did really was writing to the filehandle, then we'd
overwrite it).

> To be really pedantic, there's also the question of what errno the
> caller would want if *both* ferror() and fclose() fail. Normally I would
> say "the first error that occurred", but in this case we don't know the
> correct errno from the error reported by ferror(), so maybe the fclose()
> errno is more likely to hint at the underlying reason for the failure.

Yes, I think we're better to take what fclose gives us, if we can.

> So I (reluctantly) propose
> 
> 	if (ferror(fp)) {
> 		if (!fclose(fp)) {
> 			/*
> 			 * ferror() doesn't set errno, so we have to
> 			 * set one. (By contrast, when fclose() fails
> 			 * too, we leave *its* errno in place.)
> 			 */
> 			errno = EIO;
> 		}
> 		return -1;
> 	}
> 	return fclose();

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 same thing.

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]