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

Denys Vlasenko vda.linux at googlemail.com
Sun Mar 11 13:46:24 UTC 2012


On Saturday 10 March 2012 23:55, Kevin Cernekee wrote:
> On Sat, Feb 12, 2011 at 6:31 PM, Denys Vlasenko
> <vda.linux at googlemail.com> wrote:
> >> >Currently, uclibc retains buffered data on stdio write errors,
> >> >and subsequent fclose and fflush will try to write it out again
> >> >(in most cases, in vain).
> 
> This seems to be causing some strange problems on bash as well.  Is
> there any interest in trying to mimic the glibc behavior?
> 
> uClibc version is 0.9.32.1.
> 
> 
> bash problem:
> 
> # bash --version
> GNU bash, version 3.2.0(1)-release (mipsel-unknown-linux-gnu)
> Copyright (C) 2005 Free Software Foundation, Inc.
> # echo "none of the above" > /sys/power/state
> sh: echo: write error: Invalid argument
> # aaaaa
> sh: aaaaa: command not found
> none of the abov# bbbbb
> sh: bbbbb: command not found
> none of the abov#
> # echo mem > /sys/power/state
> sh: echo: write error: Invalid argument
> # echo mem > /sys/power/state
> sh: echo: write error: Invalid argument
> # busybox echo mem > /sys/power/state
> PM: Syncing filesystems ... done.
> Freezing user space processes ... (elapsed 0.01 seconds) done.
> Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
> Suspending console(s) (use no_console_suspend to debug)
> 
> 
> Simple test case to show that the buffered data persists (albeit in
> truncated form) even after calling fflush() and clearerr():
> 
> -- 8< --
> 
> #include <stdio.h>
> #include <unistd.h>
> #include <fcntl.h>
> 
> int main(int argc, char **argv)
> {
> 	int tmpfd, fd;
> 
> 	tmpfd = fcntl(fileno(stdout), F_DUPFD, 0);
> 	fd = open("/dev/null", O_RDONLY);
> 
> 	dup2(fd, fileno(stdout));
> 
> 	printf("hello world\n");
> 	fflush(stdout);
> 	if (ferror(stdout))
> 		clearerr(stdout);
> 
> 	dup2(tmpfd, fileno(stdout));
> 
> 	printf("goodbye world\n");
> 
> 	return 0;
> }
> 
> -- 8< --
> 
> Output:
> 
> # ./test
> hello worlgoodbye world


I propose the following patch.
Only compile tested by me, care to run-test?

-- 
vda


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..af1d09f 100644
--- a/libc/stdio/clearerr.c
+++ b/libc/stdio/clearerr.c
@@ -7,6 +7,41 @@
 
 #include "_stdio.h"
 
+static inline void discard_buffered_data(FILE *stream)
+{
+#ifdef __STDIO_BUFFERS
+    	/* 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().
+	 */
+	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 */
+	) {
+		stream->__bufpos = stream->__bufstart;
+		/*
+		 * Prevent spurious buffer resets on superfluous clearerrs.
+		 * Example:
+		 *
+		 * close(fileno(stdout));
+		 * printf("Hi there!\n"); // error happens here, 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;
+	}
+#endif
+}
+
 #undef clearerr
 #ifdef __DO_UNLOCKED
 
@@ -15,6 +50,8 @@ void clearerr_unlocked(register FILE *stream)
 {
 	__STDIO_STREAM_VALIDATE(stream);
 
+	discard_buffered_data(stream);
+
 	__CLEARERR_UNLOCKED(stream);
 }
 
@@ -32,6 +69,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 */


More information about the uClibc mailing list