[PATCH]: Remove "is closed check" in fclose.c and fwalk.c

[Antony KING <antony dot king at st dot com>]

Hi,

Please find attached a patch to fclose.c and fwalk.c you may want to
consider to prevent a memory leak problem I encountered a while ago. The
problem I was having is a result of our implementation of FILE locks
being created on demand. This was resulting in a FILE lock being created
for a closed FILE which was being lost (i.e. overwritten) when the FILE
was re-used.

The solution I adopted was to remove the checks for a closed FILE in
fclose() entirely since fclose() should only be called on an open FILE
and therefore the check is not strictly necessary. A side effect of this
will be to break an application where multiple threads close the same
FILE but this is not defined (i.e. closing a FILE which is already closed).

A similar solution was adopted for fwalk() (although I suspect that the
taking of the FILE lock is unnecessary anyway since fwalk() has the
additional protection of taking the global "sfp" lock before testing if
the FILE is open).

The ChangeLog comment is as follows:

2006-11-03  Antony King  <antony.king@st.com>

	* libc/stdio/fclose.c (_fclose_r): Remove unnecessary check if
	FILE is closed and fix consequential potential memory leak when
	FILE locks are created on demand.
	* libc/stdio/fwalk.c: (__fwalk, __fwalk_reent): Ditto.

Cheers,

Antony.


--- newlib/libc/stdio/fwalk.c.orig	2004-09-16 22:26:51.000000000 +0100
+++ newlib/libc/stdio/fwalk.c	2006-03-08 15:15:29.000000000 +0000
@@ -38,13 +38,8 @@
 
   for (g = &ptr->__sglue; g != NULL; g = g->_next)
     for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
-      if (fp->_flags != 0)
-        {
-          _flockfile (fp);
-          if (fp->_flags != 0 && fp->_file != -1)
-            ret |= (*function) (fp);
-          _funlockfile (fp);
-        }
+      if (fp->_flags != 0 && fp->_file != -1)
+        ret |= (*function) (fp);
 
   return ret;
 }
@@ -62,13 +57,8 @@
 
   for (g = &ptr->__sglue; g != NULL; g = g->_next)
     for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
-      if (fp->_flags != 0)
-        {
-          _flockfile (fp);
-          if (fp->_flags != 0 && fp->_file != -1)
-            ret |= (*reent_function) (ptr, fp);
-          _funlockfile (fp);
-        }
+      if (fp->_flags != 0 && fp->_file != -1)
+        ret |= (*reent_function) (ptr, fp);
 
   return ret;
 }
--- newlib/libc/stdio/fclose.c.orig	2006-10-06 13:54:21.000000000 +0100
+++ newlib/libc/stdio/fclose.c	2006-10-27 12:39:01.655739000 +0100
@@ -79,13 +79,6 @@
   CHECK_INIT (rptr, fp);
 
   _flockfile (fp);
-  
-  if (fp->_flags == 0)		/* not open! */
-    {
-      _funlockfile (fp);
-      __sfp_lock_release ();
-      return (0);
-    }
   r = fp->_flags & __SWR ? fflush (fp) : 0;
   if (fp->_close != NULL && (*fp->_close) (fp->_cookie) < 0)
     r = EOF;