Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 24 May 2012 19:29:10 GMT
From:      Garrett Cooper <yanegomi@gmail.com>
To:        freebsd-gnats-submit@FreeBSD.org
Subject:   misc/168315: [patch] add dirname_r(3); document ENOMEM conditions with basename(3) and dirname(3) functions
Message-ID:  <201205241929.q4OJTAx5062815@red.freebsd.org>
Resent-Message-ID: <201205241930.q4OJU6pZ029033@freefall.freebsd.org>

next in thread | raw e-mail | index | archive | help

>Number:         168315
>Category:       misc
>Synopsis:       [patch] add dirname_r(3); document ENOMEM conditions with basename(3) and dirname(3) functions
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Thu May 24 19:30:06 UTC 2012
>Closed-Date:
>Last-Modified:
>Originator:     Garrett Cooper
>Release:        9-STABLE
>Organization:
EMC Isilon
>Environment:
FreeBSD forza.west.isilon.com 9.0-STABLE FreeBSD 9.0-STABLE #4 r235133: Mon May  7 10:31:22 PDT 2012     root@forza.isilon.com:/usr/obj/usr/src/sys/FORZA  amd64
>Description:
Looking through the manpages today and inspecting the libc/gen/{base,dir}name.c, I noticed that there wasn't a dirname_r -- so I whipped one up quickly this morning and tested it out.

I also documented the undocumented errors that could occur if malloc(3) failed in basename(3) or dirname(3).
>How-To-Repeat:
$ cat ~/test_dirname.c
#include <sys/param.h>
#include <assert.h>
#include <errno.h>
#include <libgen.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

struct testcase {
        const char *input;
        const char *exp_res;
        int exp_errno;
};

struct testcase testcases[] = {
        { ".", ".", 0 },
        { "/", "/", 0 },
        { "", ".", 0 },
        { "a.b.c.d", ".", 0 },
        { "a/b/c/d", "a/b/c", 0 },
        { "a/b/c/d/", "a/b/c", 0 },
        { (const char*)NULL, ".", 0 },
        /* python -c 'print "/" + "a" * 1025 + "/b/"' */
        { "/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/b/", NULL, ENAMETOOLONG },
};

int
main(int argc, char *argv[])
{
        char *buf, *res1, *res2;
        int failed, i;

        failed = 0;

        /* Slop */
        buf = malloc(5*MAXPATHLEN);
        assert(buf != NULL);

        for (i = 0; i < sizeof(testcases)/sizeof(testcases[0]); i++) {
                res1 = dirname(testcases[i].input);
                res2 = dirname_r(testcases[i].input, buf);
                if (testcases[i].exp_res == NULL) {
                        assert(res1 == res2);
                        if (res1 != NULL || errno != testcases[i].exp_errno) {
                                printf("FAIL: dirname(\"%s\") != NULL (was %s)\n", testcases[i].input, res1);
                                failed++;
                        } else if (errno != testcases[i].exp_errno) {
                                printf("FAIL: dirname(\"%s\") didn't set errno == %d (was %d)\n", testcases[i].input, testcases[i].exp_errno, errno);
                                failed++;
                        } else
                                printf("OK: dirname(\"%s\") == NULL and set errno == %d\n", testcases[i].input, testcases[i].exp_errno);
                } else {
                        assert(strcmp(res1, res2) == 0);
                        if (strcmp(res1, testcases[i].exp_res) != 0) {
                                printf("FAIL: dirname(\"%s\") != \"%s\" (was %s)\n", testcases[i].input, testcases[i].exp_res, res1);
                                failed++;
                        } else
                                printf("OK: dirname(\"%s\") == '%s'\n", testcases[i].input, testcases[i].exp_res);
                }
        }
        return (failed);
}
$ gcc -Wall -o ~/test_dirname ~/test_dirname.c
$ ~/test_dirname 
OK: dirname(".") == '.'
OK: dirname("/") == '/'
OK: dirname("") == '.'
OK: dirname("a.b.c.d") == '.'
OK: dirname("a/b/c/d") == 'a/b/c'
OK: dirname("a/b/c/d/") == 'a/b/c'
OK: dirname("(null)") == '.'
OK: dirname("/aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa/b/") == NULL and set errno == 63
$
>Fix:


Patch attached with submission follows:

Index: include/libgen.h
===================================================================
--- include/libgen.h	(revision 235484)
+++ include/libgen.h	(working copy)
@@ -38,6 +38,7 @@
 char	*basename(const char *);
 char	*basename_r(const char *, char *);
 char	*dirname(const char *);
+char	*dirname_r(const char *, char *);
 #if 0
 char	*regcmp(const char *, ...);
 char	*regex(const char *, const char *, ...);
Index: lib/libc/gen/Makefile.inc
===================================================================
--- lib/libc/gen/Makefile.inc	(revision 235484)
+++ lib/libc/gen/Makefile.inc	(working copy)
@@ -93,6 +93,7 @@
 	directory.3 fdopendir.3 \
 	directory.3 readdir.3 directory.3 readdir_r.3 directory.3 rewinddir.3 \
 	directory.3 seekdir.3 directory.3 telldir.3
+MLINKS+=dirname.3 dirname_r.3
 MLINKS+=dlopen.3 fdlopen.3 dlopen.3 dlclose.3 dlopen.3 dlerror.3 \
 	dlopen.3 dlfunc.3 dlopen.3 dlsym.3
 MLINKS+=err.3 err_set_exit.3 err.3 err_set_file.3 err.3 errc.3 err.3 errx.3 \
Index: lib/libc/gen/basename.3
===================================================================
--- lib/libc/gen/basename.3	(revision 235484)
+++ lib/libc/gen/basename.3	(working copy)
@@ -81,6 +81,10 @@
 .It Bq Er ENAMETOOLONG
 The path component to be returned was larger than
 .Dv MAXPATHLEN .
+.It Bq Er ENOMEM
+The static buffer used for storing the path in
+.Fn basename
+could not be allocated.
 .El
 .Sh SEE ALSO
 .Xr basename 1 ,
Index: lib/libc/gen/dirname.3
===================================================================
--- lib/libc/gen/dirname.3	(revision 235484)
+++ lib/libc/gen/dirname.3	(working copy)
@@ -26,6 +26,8 @@
 .In libgen.h
 .Ft char *
 .Fn dirname "const char *path"
+.Ft char *
+.Fn dirname_r "const char *path" "char *dname"
 .Sh DESCRIPTION
 The
 .Fn dirname
@@ -53,6 +55,8 @@
 returns a pointer to internal storage space allocated on the first call
 that will be overwritten
 by subsequent calls.
+.Fn dirname_r
+is therefore preferred for threaded applications.
 .Pp
 Other vendor implementations of
 .Fn dirname
@@ -78,6 +82,10 @@
 .It Bq Er ENAMETOOLONG
 The path component to be returned was larger than
 .Dv MAXPATHLEN .
+.It Bq Er ENOMEM
+The static buffer used for storing the path in
+.Fn dirname
+could not be allocated.
 .El
 .Sh SEE ALSO
 .Xr basename 1 ,
Index: lib/libc/gen/dirname.c
===================================================================
--- lib/libc/gen/dirname.c	(revision 235484)
+++ lib/libc/gen/dirname.c	(working copy)
@@ -26,18 +26,11 @@
 #include <sys/param.h>
 
 char *
-dirname(const char *path)
+dirname_r(const char *path, char *dname)
 {
-	static char *dname = NULL;
+	const char *endp;
 	size_t len;
-	const char *endp;
 
-	if (dname == NULL) {
-		dname = (char *)malloc(MAXPATHLEN);
-		if (dname == NULL)
-			return(NULL);
-	}
-
 	/* Empty or NULL string gets treated as "." */
 	if (path == NULL || *path == '\0') {
 		dname[0] = '.';
@@ -75,3 +68,16 @@
 	dname[len] = '\0';
 	return (dname);
 }
+
+char *
+dirname(const char *path)
+{
+	static char *dname = NULL;
+
+	if (dname == NULL) {
+		dname = (char *)malloc(MAXPATHLEN);
+		if (dname == NULL)
+			return(NULL);
+	}
+	return (dirname_r(path, dname));
+}
Index: lib/libc/gen/Symbol.map
===================================================================
--- lib/libc/gen/Symbol.map	(revision 235484)
+++ lib/libc/gen/Symbol.map	(working copy)
@@ -381,7 +381,8 @@
 };
 
 FBSD_1.3 {
-	 fdlopen;
+	dirname_r;
+	fdlopen;
 	__FreeBSD_libc_enter_restricted_mode;
 	getcontextx;
 };


>Release-Note:
>Audit-Trail:
>Unformatted:



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201205241929.q4OJTAx5062815>