Browse Source

__fsetlocking() and FILE field user_locking were completely broken. :-(
I think they're fixed now (I've run a few tests).
Note: __fsetlocking() is not threadsafe... but glibc's doesn't appear to
be either.

Manuel Novoa III 22 years ago
parent
commit
a70b1b9b0e
3 changed files with 26 additions and 9 deletions
  1. 4 0
      libc/stdio/printf.c
  2. 2 0
      libc/stdio/scanf.c
  3. 20 9
      libc/stdio/stdio.c

+ 4 - 0
libc/stdio/printf.c

@@ -61,6 +61,7 @@
 #include <printf.h>
 
 #ifdef __STDIO_THREADSAFE
+#include <stdio_ext.h>
 #include <pthread.h>
 #endif /* __STDIO_THREADSAFE */
 
@@ -1313,6 +1314,7 @@ int vsnprintf(char *__restrict buf, size_t size,
 #endif /* __STDIO_MBSTATE */
 
 #ifdef __STDIO_THREADSAFE
+	f.user_locking = 0;
 	__stdio_init_mutex(&f.lock);
 #endif
 
@@ -1392,6 +1394,7 @@ int vsnprintf(char *__restrict buf, size_t size,
 #endif /* __STDIO_MBSTATE */
 
 #ifdef __STDIO_THREADSAFE
+	f.user_locking = 0;
 	__stdio_init_mutex(&f.lock);
 #endif
 
@@ -1439,6 +1442,7 @@ int vdprintf(int filedes, const char * __restrict format, va_list arg)
 #endif /* __STDIO_MBSTATE */
 
 #ifdef __STDIO_THREADSAFE
+	f.user_locking = 0;
 	__stdio_init_mutex(&f.lock);
 #endif
 

+ 2 - 0
libc/stdio/scanf.c

@@ -42,6 +42,7 @@
 #include <stdarg.h>
 
 #ifdef __STDIO_THREADSAFE
+#include <stdio_ext.h>
 #include <pthread.h>
 #endif /* __STDIO_THREADSAFE */
 
@@ -128,6 +129,7 @@ int vsscanf(__const char *sp, __const char *fmt, va_list ap)
 #endif /* __STDIO_MBSTATE */
 
 #ifdef __STDIO_THREADSAFE
+	string->user_locking = 0;
 	__stdio_init_mutex(&string->lock);
 #endif
 

+ 20 - 9
libc/stdio/stdio.c

@@ -1023,31 +1023,41 @@ void _flushlbf(void)
 /**********************************************************************/
 #ifdef L___fsetlocking
 
-/* No (serious) reentrancy issues -- return value could be incorrect. */
-/* TODO -- fix race */
+/* NOT threadsafe!!!  (I don't think glibc's is either)
+ *
+ * This interacts badly with internal locking/unlocking.  If you use this routine,
+ * make sure the file isn't being accessed by any other threads.  Typical use would
+ * be to change to user locking immediately after opening the stream.
+ */
+
+link_warning(__fsetlocking, "Oddly enough, __fsetlocking() is NOT threadsafe.")
 
 int __fsetlocking(FILE *stream, int locking_mode)
 {
+#ifdef __STDIO_THREADSAFE
 	int old_mode;
+#endif
 
 	assert((FSETLOCKING_QUERY == 0) && (FSETLOCKING_INTERNAL == 1)
 		   && (FSETLOCKING_BYCALLER == 2));
 
 	assert(((unsigned int) locking_mode) <= 2);
 
-	/* Note: don't even bother locking here... */
+#ifdef __STDIO_THREADSAFE
+	old_mode = stream->user_locking;
 
-	old_mode  = stream->user_locking;
+	assert(((unsigned int) old_mode) <= 1);	/* Must be 0 (internal) or 1 (user). */
 
 	if (locking_mode != FSETLOCKING_QUERY) {
 		/* In case we're not debugging, treat any unknown as a request to
 		 * set internal locking, in order to match glibc behavior. */
-		stream->user_locking = ((locking_mode == FSETLOCKING_BYCALLER)
-								? FSETLOCKING_BYCALLER
-								: FSETLOCKING_INTERNAL);
+		stream->user_locking = (locking_mode == FSETLOCKING_BYCALLER);
 	}
 
-	return old_mode;
+	return 2 - old_mode;
+#else
+	return FSETLOCKING_BYCALLER; /* Well, without thread support... */
+#endif
 }
 
 #endif
@@ -1851,7 +1861,7 @@ void _stdio_term(void)
 	 * Note: Set locking mode to "by caller" to save some overhead later. */
 	__stdio_init_mutex(&_stdio_openlist_lock);
 	for (ptr = _stdio_openlist ; ptr ; ptr = ptr->nextopen ) {
-		ptr->user_locking = FSETLOCKING_BYCALLER;
+		ptr->user_locking = 1;
 		__stdio_init_mutex(&ptr->lock);
 	}
 #endif /* __STDIO_THREADSAFE */
@@ -2358,6 +2368,7 @@ FILE *_stdio_fopen(const char * __restrict filename,
 #endif /* __STDIO_MBSTATE */
 
 #ifdef __STDIO_THREADSAFE
+	stream->user_locking = 0;
 	__stdio_init_mutex(&stream->lock);
 #endif /* __STDIO_THREADSAFE */