rayan@cs.toronto.edu (Rayan Zachariassen) (10/24/89)
[ I post this here because we have no official way of notifying AT&T (nor,
I suspect, enough incentive to do so or their attention if we did), and
several machines running 3.1-derived code has this bug so this also
notifies all the vendors of such machines. A bug report has been
called in to SGI. ]
OS version: System V 3.1
Description: closedir() is supposed to deallocate the DIR * returned by
opendir(), and close the filedescriptor whose value is stored
in the DIR * structure. This is done by first freeing the DIR *
structure and then trying to get the value of the file descriptor
stored within it. The semantics of free doesn't guarantee data
integrity of freed memory.
Symptom: an application repeatedly scanning a directory (i.e. opendir()
readdir() closedir() in a loop) will run out of file descriptors
when linked with a version of free that scribbles on the memory
it frees (in order to catch these kinds of problems).
Workaround: instead of a normal closedir(), use the sequence:
close(dirp->dd_fd);
(void) closedir(dirp); /* will return failure indication */
Severity: Embarrassing...
Should be fixed in future releases (if someone can check on
V.4 and let us and/or AT&T know...).
Fix: in lib/libc/port/gen/closedir.c
RCS file: RCS/closedir.c,v
retrieving revision 1.1
diff -b -c -r1.1 closedir.c
*** /tmp/,RCSt1a09053 Mon Oct 23 22:01:05 1989
--- closedir.c Mon Oct 23 22:01:06 1989
***************
*** 35,41 ****
closedir( dirp )
register DIR *dirp; /* stream from opendir() */
{
free( dirp->dd_buf );
free( (char *)dirp );
! return(close( dirp->dd_fd ));
}
--- 35,43 ----
closedir( dirp )
register DIR *dirp; /* stream from opendir() */
{
+ int fd = dirp->dd_fd;
+
free( dirp->dd_buf );
free( (char *)dirp );
! return(close( fd ));
}vandys@hpcupt1.HP.COM (Andrew Valencia(Seattle)) (10/24/89)
/ hpcupt1:comp.bugs.sys5 / rayan@cs.toronto.edu (Rayan Zachariassen) / 7:34 pm Oct 23, 1989 / >stored within it. The semantics of free doesn't guarantee data >integrity of freed memory. From some fairly System-Vish man pages for malloc/free: The argument to free is a pointer to a block previously allocated by malloc; after free is performed this space is made available for further allocation, but its contents are left undisturbed. ===================================== You can claim that this is WRONG, but for compatibility I'd recommend not going against the grain with alternative malloc/free implementations. Andy
gwyn@smoke.BRL.MIL (Doug Gwyn) (10/25/89)
In article <89Oct23.223414edt.2151@neat.cs.toronto.edu> rayan@cs.toronto.edu (Rayan Zachariassen) writes: >Description: closedir() is supposed to deallocate the DIR * ... Yes, this bug is also present in SVR3.0. It was inherited from an old version of my PD implementation, long since fixed along the lines that you indicated.
rayan@cs.toronto.edu (Rayan Zachariassen) (10/26/89)
You are confusing documentation of a particular implementation with a spec. From some equally SystemV-ish man-pages (for libmalloc): The argument to free is a pointer to a block previously allocated by malloc; after free is performed this space is made available for further allocation, and its contents have been destroyed (but see mallopt below for a way to change this behavior). In fact I was using neither of these implementations when I ran into the problem, but that doesn't matter because the closedir() code should have been written with the spec in mind, not the (documented) behaviour of a particular implementation. Lets not turn this into comp.lang.c.
gwyn@smoke.BRL.MIL (Doug Gwyn) (10/27/89)
In article <5890003@hpcupt1.HP.COM> vandys@hpcupt1.HP.COM (Andrew Valencia(Seattle)) writes:
- ... after free is performed this space is made available
- for further allocation, but its contents are left undisturbed.
- =====================================
-You can claim that this is WRONG, but for compatibility I'd recommend not
-going against the grain with alternative malloc/free implementations.
On the other hand, no code should be written to depend on this "guarantee".
Thus closedir() really did have a bug (I should know; I wrote it).guy@auspex.auspex.com (Guy Harris) (10/29/89)
>>stored within it. The semantics of free doesn't guarantee data >>integrity of freed memory. > >From some fairly System-Vish man pages for malloc/free: > > The argument to free is a pointer to a block previously allocated > by malloc; after free is performed this space is made available > for further allocation, but its contents are left undisturbed. > ===================================== > >You can claim that this is WRONG, but for compatibility I'd recommend not >going against the grain with alternative malloc/free implementations. And for safety I'd recommend not depending on those semantics, since: 1) the pANS doesn't require this behavior; 2) POSIX doesn't impose a requirement for it atop the ANSI C specs; 3) the SVID only says that said behavior is required if you explicitly use "mallopt" to set the M_KEEP flag - and, in the section describing it, says "This option is provided only for compatibility with the older version of the function 'malloc' and is not recommended."; 4) the S5 "-lmalloc" routines are the ones described by the SVID, so if you link with that library you'd better call "mallopt" if you want to guarantee that behavior (and better be prepared to have your code run only on systems that have "mallopt"). 4) indicates that if you don't want "closedir()" to barf, you'd better call "mallopt" to set M_KEEP, or run your code only on systems that guarantee it without M_KEEP (SunOS does that, although the comment in the man page indicates that: BUGS Since realloc() accepts a pointer to a block freed since the last call to malloc, calloc, realloc, valloc, or memalign, a degradation of performance results. The semantics of free() should be changed so that the contents of a previously freed block are undefined. so this may very well change in the future to require M_KEEP), or get your vendor to fix their code not to depend on those semantics. The first two options, of course, limit the portability of your code; your application may well be POSIX-conforming except for that call to "mallopt", for example.