[PATCH] prevent retries on fclose/fflush after write errors

[Denys Vlasenko]

On Sunday 11 March 2012 14:46, Denys Vlasenko wrote:
> I propose the following patch.
> Only compile tested by me, care to run-test?

A more correct (I hope) version:


diff --git a/include/stdio_ext.h b/include/stdio_ext.h
index 9ad54f6..ff1ff20 100644
--- a/include/stdio_ext.h
+++ b/include/stdio_ext.h
@@ -71,6 +71,7 @@ extern int __flbf (FILE *__fp) __THROW;
 
 /* Discard all pending buffered I/O on the stream FP.  */
 extern void __fpurge (FILE *__fp) __THROW;
+libc_hidden_proto(__fpurge)
 
 /* Return amount of output in bytes pending on a stream FP.  */
 extern size_t __fpending (FILE *__fp) __THROW;
diff --git a/libc/stdio/_stdio.h b/libc/stdio/_stdio.h
index 2c0efc9..78069a5 100644
--- a/libc/stdio/_stdio.h
+++ b/libc/stdio/_stdio.h
@@ -212,7 +212,7 @@ extern int __stdio_seek(FILE *stream, register __offmax_t *pos, int whence) attr
 #define __STDIO_STREAM_SET_EOF(S) \
 	((void)((S)->__modeflags |= __FLAG_EOF))
 #define __STDIO_STREAM_SET_ERROR(S) \
-	((void)((S)->__modeflags |= __FLAG_ERROR))
+	((void)((S)->__errno_value = errno, (S)->__modeflags |= __FLAG_ERROR))
 
 #define __STDIO_STREAM_CLEAR_EOF(S) \
 	((void)((S)->__modeflags &= ~__FLAG_EOF))
diff --git a/libc/stdio/clearerr.c b/libc/stdio/clearerr.c
index a96ecaa..c801ccd 100644
--- a/libc/stdio/clearerr.c
+++ b/libc/stdio/clearerr.c
@@ -5,8 +5,47 @@
  * Dedicated to Toni.  See uClibc/DEDICATION.mjn3 for details.
  */
 
+#include <stdio_ext.h>
 #include "_stdio.h"
 
+static inline void discard_buffered_data(FILE *stream)
+{
+    	/*
+	 * Puty, but fpurge is rather non-portable,
+	 * thus many apps won't use it. Let them be able to clear
+	 * buffered data on write errors using clearerr().
+	 *
+	 * Some errors are non-fatal. Example:
+	 * printf("Hi there!\n"); // let's assume EAGAIN happens here
+	 * clearerr(stdout);      // must NOT drop buffered data
+	 * fflush(stdout);        // must try to write buffered data again
+	 */
+	if (__FERROR_UNLOCKED(stream)  /* was there indeed an error? */
+	 && __STDIO_STREAM_IS_WRITING(stream)
+	 && stream->__errno_value != 0
+	 && stream->__errno_value != EINTR
+	 && stream->__errno_value != EAGAIN
+	 /* add more non-fatal error codes here */
+	) {
+		__fpurge(stream);
+		/*
+		 * Prevent spurious buffer resets on superfluous clearerrs.
+		 * Example:
+		 * close(fileno(stdout));
+		 * printf("Hi there!\n"); // EBADF, data is buffered
+		 * dup2(good_fd, fileno(stdout));
+		 * clearerr(stdout);      // drops buffered data
+		 * printf("Hi there!");   // buffers new data
+		 * clearerr(stdout);      // must NOT drop newly buffered data
+		 *
+		 * We assure this by __FERROR_UNLOCKED check above,
+		 * and by clearing __errno_value below
+		 * (yes, I am a bit paranoid).
+		 */
+		stream->__errno_value = 0;
+	}
+}
+
 #undef clearerr
 #ifdef __DO_UNLOCKED
 
@@ -15,6 +54,8 @@ void clearerr_unlocked(register FILE *stream)
 {
 	__STDIO_STREAM_VALIDATE(stream);
 
+	discard_buffered_data(stream);
+
 	__CLEARERR_UNLOCKED(stream);
 }
 
@@ -32,6 +73,8 @@ void clearerr(register FILE *stream)
 
 	__STDIO_STREAM_VALIDATE(stream);
 
+	discard_buffered_data(stream);
+
 	__CLEARERR_UNLOCKED(stream);
 
 	__STDIO_AUTO_THREADUNLOCK(stream);
diff --git a/libc/stdio/fflush.c b/libc/stdio/fflush.c
index d9104a4..4599b58 100644
--- a/libc/stdio/fflush.c
+++ b/libc/stdio/fflush.c
@@ -134,8 +134,8 @@ int fflush_unlocked(register FILE *stream)
 		 * shouldn't flush a stream you were reading from.  As usual, glibc
 		 * caters to broken programs and simply ignores this. */
 		__UNDEFINED_OR_NONPORTABLE;
-		__STDIO_STREAM_SET_ERROR(stream);
 		__set_errno(EBADF);
+		__STDIO_STREAM_SET_ERROR(stream);
 		retval = EOF;
 	}
 #endif
@@ -166,8 +166,8 @@ int fflush_unlocked(register FILE *stream)
 		 * shouldn't flush a stream you were reading from.  As usual, glibc
 		 * caters to broken programs and simply ignores this. */
 		__UNDEFINED_OR_NONPORTABLE;
-		__STDIO_STREAM_SET_ERROR(stream);
 		__set_errno(EBADF);
+		__STDIO_STREAM_SET_ERROR(stream);
 		return EOF;
 	}
 #endif
diff --git a/libc/stdio/fread.c b/libc/stdio/fread.c
index d2fcc70..5ef358b 100644
--- a/libc/stdio/fread.c
+++ b/libc/stdio/fread.c
@@ -77,8 +77,8 @@ size_t fread_unlocked(void * __restrict ptr, size_t size, size_t nmemb,
 			return (bytes - todo) / size;
 		}
 
-		__STDIO_STREAM_SET_ERROR(stream);
 		__set_errno(EINVAL);
+		__STDIO_STREAM_SET_ERROR(stream);
 	}
 
 	__STDIO_STREAM_VALIDATE(stream);
diff --git a/libc/stdio/fwrite.c b/libc/stdio/fwrite.c
index 71793ff..3eacd7e 100644
--- a/libc/stdio/fwrite.c
+++ b/libc/stdio/fwrite.c
@@ -28,8 +28,8 @@ size_t fwrite_unlocked(const void * __restrict ptr, size_t size,
 								  size*nmemb, stream) / size;
 		}
 
-		__STDIO_STREAM_SET_ERROR(stream);
 		__set_errno(EINVAL);
+		__STDIO_STREAM_SET_ERROR(stream);
 	}
 
 	return 0;
diff --git a/libc/sysdeps/linux/common/bits/uClibc_stdio.h b/libc/sysdeps/linux/common/bits/uClibc_stdio.h
index a8cf4eb..f746f14 100644
--- a/libc/sysdeps/linux/common/bits/uClibc_stdio.h
+++ b/libc/sysdeps/linux/common/bits/uClibc_stdio.h
@@ -250,6 +250,7 @@ struct __STDIO_FILE_STRUCT {
 	unsigned char __ungot[2];
 #endif /* __UCLIBC_HAS_WCHAR__ */
 	int __filedes;
+	int __errno_value;
 #ifdef __STDIO_BUFFERS
 	unsigned char *__bufstart;	/* pointer to buffer */
 	unsigned char *__bufend;	/* pointer to 1 past end of buffer */