docs/82508: misleading man page for basename/dirname

[Vlad Skvortsov]

The following reply was made to PR docs/82508; it has been noted by GNATS.

From: Vlad Skvortsov <vss at 73rus.com>
To: Giorgos Keramidas <keramida at freebsd.org>
Cc: Vlad Skvortsov <vss at high.net.ru>, bug-followup at freebsd.org
Subject: Re: docs/82508: misleading man page for basename/dirname
Date: Fri, 01 Jul 2005 22:45:48 -0700

 Hey, I understand everything about that - and I don't ask to rework the 
 implementation of the functions. What I want is just to make the 
 documentaion more accurate on that point. If the man page would state 
 that those functions allocate storage and are not thread-safe, I 
 wouldn't have to look into the source code for the explanation of their 
 behaviour. That's my point. :-)
 
 
 Giorgos Keramidas wrote:
 > On 2005-06-30 23:24, Vlad Skvortsov <vss at 73rus.com> wrote:
 > 
 >>Giorgos Keramidas wrote:
 >>
 >>>On 2005-06-22 02:51, Vlad Skvortsov <vss at high.net.ru> wrote:
 >>>
 >>>>The man pages for both basename(3) and dirname(3) state that the
 >>>>functions return pointers to the internal _static_ storage.
 >>>>However, those functions actually perform malloc() call to
 >>>>allocate storage on the first invocation. Thus, the memory pointer
 >>>>returned is actually a pointer to internal but dynamically
 >>>>allocated storage.
 >>>>
 >>>>I don't know whether this violates standard or not, but the
 >>>>documentation is misleading.
 >>>
 >>>The term 'static' here is a warning that these functions are not
 >>>thread-safe.
 >>>
 >>>It does NOT mean that the ``bname'' object that is internal to
 >>>basename() is actually an array declared as:
 >>>
 >>>	char bname[MAXPATHLEN];
 >>>
 >>>It merely means that multiple invocations of the function from
 >>>concurrent threads may clobber each other's data, so some form of
 >>>locking should be used around calls to basename() from threaded
 >>>applications or the function should be avoided altogether.
 >>
 >>Yes, I do understand what it supposed to mean. But, anyway, 'static'
 >>means 'static', not (not only) 'thread-safe'. ;-)
 > 
 > 
 > The storage *IS* accessed through a static pointer:
 > 
 >   42: char *
 >   43: dirname(path)
 >   44:        const char *path;
 >   45: {
 >   46:        static char *bname = NULL;
 >   47:        const char *endp;
 >   48:
 >   49:        if (bname == NULL) {
 >   50:                bname = (char *)malloc(MAXPATHLEN);
 >   51:                if (bname == NULL)
 >   52:                        return(NULL);
 >   53:        }
 > 
 > 
 >>I've ran into this issue while running a testsuite checking for memory leaks.
 >>I expected those values to be static, not thread-safe.
 > 
 > 
 > Oh, I see I think.  So, basically, you're saying that dirname() and
 > basename() trigger false positives in the memory leak detection tools
 > you used.
 > 
 > I think the reason `bname' is not declared as:
 > 
 > 	static char bname[MAXPATHLEN];
 > 
 > is to avoid actually allocating this array in the BSS area of programs
 > that link with libc (practically every dynamic/shared executable on
 > FreeBSD).
 > 
 > This is exactly what the rationale for the change was, according to the
 > CVS log of src/lib/libc/gen/dirname.c (rev. 1.6).
 > 
 > AFAICT, there isn't an easy/clean way to have this automagically
 > deallocated on program exit, which would let us keep both the malloc()
 > call *and* make your memory leak detection tools happy :-/
 > 
 
 
 -- 
 Vlad Skvortsov, vss at 73rus.com, vss at high.net.ru