[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bugs in dirname module
|
From: |
Eric Blake |
|
Subject: |
bugs in dirname module |
|
Date: |
Sat, 05 Nov 2005 22:01:49 -0700 |
|
User-agent: |
Mozilla Thunderbird 1.0.2 (Windows/20050317) |
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
There are several bugs in the dirname module. Here's my first cut at a
patch for all of the issues, although I would like a second set of eyes
looking over everything.
First, FILE_SYSTEM_PREFIX_LEN was hardwired in an AH_VERBATIM in m4/dos.m4
to ignore the first character, meaning that base_name("1:") returns ""
instead of "1:" (this is not necessarily a problem in mingw or djgpp,
where : is invalid as a filename character except in a drive letter, but
is a definite bug in cygwin managed mounts). My approach was to add a
dependency on the c-ctype module to use c_isalpha; but since I couldn't
get that to work in config.h, I moved FILE_SYSTEM_PREFIX_LEN out of
config.h and into dirname.h. The only drawback to this is that
applications that used the macro directly from config.h now need to be
sure and #include "dirname.h".
Second, cygwin currently normalizes all drive letters as absolute paths,
which is different from djgpp (I think, but am not sure, that mingw falls
in with the djgpp camp). It comes down to whether there is a notion of 1
current working directory (cygwin) or 27 (djgpp, with one cwd with no
drive letter prefix, then one cwd per drive for names with a drive letter
but no leading slash). If c:\dir1 and c:\dir2 exist and are empty, "cd
c:/dir1; cd c:dir2" fails on djgpp (c:dir2 is relative to the current dir
within c:, but c:\dir1\dir2 does not exist) but succeeds on cygwin (the
path is normalized, and treated as the absolute c:\dir2). I added this
concept to m4/dos.m4, as FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE, to affect
dirname.h's definition of IS_ABSOLUTE_FILE_NAME.
POSIX requires dirname(NULL) and basename(NULL) to both return ".", but
since we purposefully differ from POSIX semantics with dir_name and
base_name, I just documented the limitation that NULL is not a valid argument.
Finally, base_name and dir_name do not properly respect // when it is
distinct from /. I have added a test to m4/dirname.m4 to define
DOUBLE_SLASH_IS_DISTINCT_ROOT to capture this.
I also moved the rudimentary test out of dirname.c into a full-blown
dirname-tests module. I've tested this with 'gnulib-tool --with-tests
- --test dirname' on cygwin, as well as testing the other code paths using
cache-priming techniques (the dirname module is completely string-based,
with no syscalls) such as ac_cv_win_or_dos=no.
ChangeLog
2005-11-05 Eric Blake <address@hidden>
* modules/dirname (Depends-on): Need c-ctype.
* modules/dirname-tests: New module.
* tests/test-dirname.c: New file.
m4/ChangeLog
2005-11-05 Eric Blake <address@hidden>
* dirname.m4 (DOUBLE_SLASH_IS_DISTINCT_ROOT): New define.
* dos.m4 (FILE_SYSTEM_PREFIX_LEN): Move from here to dirname.h.
(FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE): New define.
lib/ChangeLog
2005-11-05 Eric Blake <address@hidden>
* dirname.h (FILE_SYSTEM_PREFIX_LEN): Move here from dos.m4.
Don't treat 1: as a drive prefix.
(IS_ABSOLUTE_FILE_NAME): Treat all drive letters as absolute on
platforms like cygwin with FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE.
* dirname.c (dir_len): Determine when drive letters need a
subsequent slash. Preserve // when it is special.
(dir_name): Don't append dot when drive letter is absolute.
* basename.c (base_name): Preserve // when it is special. Strip
leading slashes if a drive letter was present.
(base_len): Preserve // when it is special.
- --
Life is short - so eat dessert first!
Eric Blake address@hidden
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.1 (Cygwin)
Comment: Public key at home.comcast.net/~ericblake/eblake.gpg
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org
iD8DBQFDbY4984KuGfSFAYARAvdqAJ0bmgCNpCq+Qcc3oCiz3Yu6q7fT9ACgrCMu
EFmzHgqzeY1yIN1EqyzQyb4=
=Fiy1
-----END PGP SIGNATURE-----
Index: lib/basename.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/basename.c,v
retrieving revision 1.26
diff -u -p -p -r1.26 basename.c
--- lib/basename.c 19 Sep 2005 17:28:14 -0000 1.26
+++ lib/basename.c 6 Nov 2005 04:58:33 -0000
@@ -30,12 +30,14 @@
Return the address of the last file name component of NAME. If
NAME has no file name components because it is all slashes, return
- NAME if it is empty, the address of its last slash otherwise. */
+ NAME if it is empty, the address of its last slash otherwise. Unlike
+ POSIX basename(), NAME cannot be NULL, and base_name("") returns "". */
char *
base_name (char const *name)
{
- char const *base = name + FILE_SYSTEM_PREFIX_LEN (name);
+ int prefix_len = FILE_SYSTEM_PREFIX_LEN (name);
+ char const *base = name + prefix_len;
char const *p;
for (p = base; *p; p++)
@@ -47,11 +49,17 @@ base_name (char const *name)
while (ISSLASH (*p));
/* If the file name ends in slash, use the trailing slash as
- the basename if no non-slashes have been found. */
+ the basename if no non-slashes have been found and there was
+ no drive letter prefix. Preserve `//' if it is special. */
if (! *p)
{
if (ISSLASH (*base))
- base = p - 1;
+ {
+ if (prefix_len)
+ base = p;
+ else if (! DOUBLE_SLASH_IS_DISTINCT_ROOT || 2 < p - base)
+ base = p - 1;
+ }
break;
}
@@ -74,6 +82,10 @@ base_len (char const *name)
for (len = strlen (name); 1 < len && ISSLASH (name[len - 1]); len--)
continue;
+
+ if (DOUBLE_SLASH_IS_DISTINCT_ROOT && len == 1
+ && ISSLASH (name[0]) && ISSLASH (name[1]) && ! name[2])
+ return 2;
return len;
}
Index: lib/dirname.c
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/dirname.c,v
retrieving revision 1.35
diff -u -p -p -r1.35 dirname.c
--- lib/dirname.c 19 Sep 2005 17:28:14 -0000 1.35
+++ lib/dirname.c 6 Nov 2005 04:58:33 -0000
@@ -26,7 +26,7 @@
#include <string.h>
#include "xalloc.h"
-/* Return the length of `dirname (FILE)', or zero if FILE is
+/* Return the length of `dir_name (FILE)', or zero if FILE is
in the working directory. Works properly even if
there are trailing slashes (by effectively ignoring them). */
size_t
@@ -40,20 +40,31 @@ dir_len (char const *file)
if (! ISSLASH (file[length - 1]))
return length;
- /* But don't strip the only slash from "/". */
- return prefix_length + ISSLASH (file[prefix_length]);
+ /* Be careful of drive prefixes, where that matters. */
+ if (0 < prefix_length)
+ return (prefix_length
+ + (! FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE
+ && ISSLASH (file[prefix_length])));
+
+ /* Don't strip the only slash from "/", or from "//" where that matters. */
+ return (ISSLASH (file[0])
+ + (DOUBLE_SLASH_IS_DISTINCT_ROOT && ISSLASH (file[0])
+ && ISSLASH (file[1]) && ! ISSLASH (file[2])));
}
-/* Return the leading directories part of FILE,
- allocated with xmalloc.
- Works properly even if there are trailing slashes
- (by effectively ignoring them). */
+/* Return the leading directories part of FILE, allocated with xmalloc.
+ Works properly even if there are trailing slashes (by effectively
+ ignoring them). Unlike POSIX dirname(), FILE cannot be NULL, and
+ dir_name("") returns "". */
char *
dir_name (char const *file)
{
size_t length = dir_len (file);
- bool append_dot = (length == FILE_SYSTEM_PREFIX_LEN (file));
+ bool append_dot = (length == 0
+ || (! FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE
+ && length == FILE_SYSTEM_PREFIX_LEN (file)
+ && file[2] != '\0' && ! ISSLASH (file[2])));
char *dir = xmalloc (length + append_dot + 1);
memcpy (dir, file, length);
if (append_dot)
Index: lib/dirname.h
===================================================================
RCS file: /cvsroot/gnulib/gnulib/lib/dirname.h,v
retrieving revision 1.11
diff -u -p -p -r1.11 dirname.h
--- lib/dirname.h 2 Jun 2005 20:41:05 -0000 1.11
+++ lib/dirname.h 6 Nov 2005 04:58:33 -0000
@@ -21,6 +21,7 @@
# include <stdbool.h>
# include <stddef.h>
+# include "c-ctype.h"
# ifndef DIRECTORY_SEPARATOR
# define DIRECTORY_SEPARATOR '/'
@@ -31,10 +32,28 @@
# endif
# ifndef FILE_SYSTEM_PREFIX_LEN
-# define FILE_SYSTEM_PREFIX_LEN(File_name) 0
+# if FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX
+# define FILE_SYSTEM_PREFIX_LEN(Filename) \
+ (c_isalpha ((Filename)[0]) && (Filename)[1] == ':' ? 2 : 0)
+# else
+# define FILE_SYSTEM_PREFIX_LEN(Filename) 0
+# endif
# endif
-# define IS_ABSOLUTE_FILE_NAME(F) ISSLASH ((F)[FILE_SYSTEM_PREFIX_LEN (F)])
+# ifndef FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE
+# define FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE 0
+# endif
+
+# ifndef DOUBLE_SLASH_IS_DISTINCT_ROOT
+# define DOUBLE_SLASH_IS_DISTINCT_ROOT 0
+# endif
+
+# if FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE
+# define IS_ABSOLUTE_FILE_NAME(F) \
+ (0 < FILE_SYSTEM_PREFIX_LEN (F) || ISSLASH ((F)[0]))
+# else
+# define IS_ABSOLUTE_FILE_NAME(F) ISSLASH ((F)[FILE_SYSTEM_PREFIX_LEN (F)])
+# endif
# define IS_RELATIVE_FILE_NAME(F) (! IS_ABSOLUTE_FILE_NAME (F))
char *base_name (char const *file);
Index: m4/dirname.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/dirname.m4,v
retrieving revision 1.8
diff -u -p -p -r1.8 dirname.m4
--- m4/dirname.m4 21 Mar 2005 22:06:27 -0000 1.8
+++ m4/dirname.m4 6 Nov 2005 04:58:36 -0000
@@ -1,4 +1,4 @@
-# dirname.m4 serial 5
+# dirname.m4 serial 6
dnl Copyright (C) 2002, 2003, 2004, 2005 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -11,6 +11,28 @@ AC_DEFUN([gl_DIRNAME],
dnl Prerequisites of lib/dirname.h.
AC_REQUIRE([gl_AC_DOS])
+ AC_CACHE_CHECK([whether // is distinct from /], [ac_cv_double_slash_root],
+ [ if test x"$cross_compiling" = xyes ; then
+ # When cross-compiling, there is no way to tell whether // is special
+ # short of a list of hosts; so always treat it as special.
+ ac_cv_double_slash_root=yes
+ else
+ set x `ls -di / //`
+ if test $[2] != $[4]; then
+ ac_cv_double_slash_root=yes
+ else
+ ac_cv_double_slash_root=no
+ fi
+ fi])
+ if test x"$ac_cv_double_slash_root" = xyes; then
+ ac_double_slash_root=1
+ else
+ ac_double_slash_root=0
+ fi
+
+ AC_DEFINE_UNQUOTED([DOUBLE_SLASH_IS_DISTINCT_ROOT],
+ $ac_double_slash_root,
+ [Define to 1 if // is a file system root distinct from /.])
dnl No prerequisites of lib/basename.c, lib/dirname.c, lib/stripslash.c.
])
Index: m4/dos.m4
===================================================================
RCS file: /cvsroot/gnulib/gnulib/m4/dos.m4,v
retrieving revision 1.12
diff -u -p -p -r1.12 dos.m4
--- m4/dos.m4 23 Jan 2005 08:06:57 -0000 1.12
+++ m4/dos.m4 6 Nov 2005 04:58:36 -0000
@@ -1,9 +1,9 @@
-#serial 9
+#serial 10
# Define some macros required for proper operation of code in lib/*.c
# on MSDOS/Windows systems.
-# Copyright (C) 2000, 2001, 2004 Free Software Foundation, Inc.
+# Copyright (C) 2000, 2001, 2004, 2005 Free Software Foundation, Inc.
# This file is free software; the Free Software Foundation
# gives unlimited permission to copy and/or distribute it,
# with or without modifications, as long as this notice is preserved.
@@ -14,30 +14,38 @@ AC_DEFUN([gl_AC_DOS],
[
AC_CACHE_CHECK([whether system is Windows or MSDOS], [ac_cv_win_or_dos],
[
- AC_TRY_COMPILE([],
- [#if !defined _WIN32 && !defined __WIN32__ && !defined __MSDOS__ &&
!defined __CYGWIN__
+ AC_TRY_COMPILE([],
+ [#if !defined _WIN32 && !defined __WIN32__ && !defined __MSDOS__ &&
!defined __CYGWIN__
neither MSDOS nor Windows
#endif],
- [ac_cv_win_or_dos=yes],
- [ac_cv_win_or_dos=no])
+ [ac_cv_win_or_dos=yes],
+ [ac_cv_win_or_dos=no])
])
if test x"$ac_cv_win_or_dos" = xyes; then
ac_fs_accepts_drive_letter_prefix=1
ac_fs_backslash_is_file_name_separator=1
+ AC_CACHE_CHECK([whether drive letter can start relative path],
+ [ac_cv_drive_letter_can_be_relative],
+ [
+ AC_TRY_COMPILE([],
+ [#if defined __CYGWIN__
+drive letters are always absolute
+#endif],
+ [ac_cv_drive_letter_can_be_relative=yes],
+ [ac_cv_drive_letter_can_be_relative=no])
+ ])
+ if test x"$ac_cv_drive_letter_can_be_relative" = xyes; then
+ ac_fs_drive_letter_always_absolute=0
+ else
+ ac_fs_drive_letter_always_absolute=1
+ fi
else
ac_fs_accepts_drive_letter_prefix=0
ac_fs_backslash_is_file_name_separator=0
+ ac_fs_drive_letter_always_absolute=0
fi
- AH_VERBATIM(FILE_SYSTEM_PREFIX_LEN,
- [#if FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX
-# define FILE_SYSTEM_PREFIX_LEN(Filename) \
- ((Filename)[0] && (Filename)[1] == ':' ? 2 : 0)
-#else
-# define FILE_SYSTEM_PREFIX_LEN(Filename) 0
-#endif])
-
AC_DEFINE_UNQUOTED([FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX],
$ac_fs_accepts_drive_letter_prefix,
[Define on systems for which file names may have a so-called
@@ -55,4 +63,9 @@ neither MSDOS nor Windows
$ac_fs_backslash_is_file_name_separator,
[Define if the backslash character may also serve as a file name
component separator.])
+
+ AC_DEFINE_UNQUOTED([FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE],
+ $ac_fs_drive_letter_always_absolute,
+ [Define if a drive letter prefix always denotes an absolute path, even
+ when it is not followed by a file name component separator.])
])
Index: modules/dirname
===================================================================
RCS file: /cvsroot/gnulib/gnulib/modules/dirname,v
retrieving revision 1.6
diff -u -p -p -r1.6 dirname
--- modules/dirname 6 May 2005 17:22:45 -0000 1.6
+++ modules/dirname 6 Nov 2005 04:58:36 -0000
@@ -10,6 +10,7 @@ m4/dos.m4
m4/dirname.m4
Depends-on:
+c-ctype
xalloc
stdbool
Index: modules/dirname-tests
===================================================================
RCS file: modules/dirname-tests
diff -N modules/dirname-tests
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ modules/dirname-tests 6 Nov 2005 04:58:36 -0000
@@ -0,0 +1,11 @@
+Files:
+tests/test-dirname.c
+
+Depends-on:
+
+configure.ac:
+
+Makefile.am:
+TESTS += test-dirname
+noinst_PROGRAMS += test-dirname
+test_dirname_SOURCES = test-dirname.c
Index: tests/test-dirname.c
===================================================================
RCS file: tests/test-dirname.c
diff -N tests/test-dirname.c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ tests/test-dirname.c 6 Nov 2005 04:58:36 -0000
@@ -0,0 +1,159 @@
+/* Test the gnulib dirname module.
+ Copyright (C) 2005 Free Software Foundation, Inc.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 2, or (at your option)
+ any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License along
+ with this program; if not, write to the Free Software Foundation,
+ Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. */
+
+#ifdef HAVE_CONFIG_H
+# include <config.h>
+#endif
+
+#include "dirname.h"
+
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+const char *program_name = "test-dirname";
+
+struct test {
+ const char *name;
+ const char *dir;
+ const char *base;
+ bool absolute;
+};
+
+static struct test tests[] = {
+ {"d/f", "d", "f", false},
+ {"/d/f", "/d", "f", true},
+ {"d/f/", "d", "f", false},
+ {"d/f//", "d", "f", false},
+ {"f", ".", "f", false},
+ {"/", "/", "/", true},
+#if DOUBLE_SLASH_IS_DISTINCT_ROOT
+ {"//", "//", "//", true},
+ {"//d", "//", "d", true},
+#else
+ {"//", "/", "/", true},
+ {"//d", "/", "d", true},
+#endif
+ {"///", "/", "/", true},
+ {"///a///", "/", "a", true},
+ /* POSIX requires dirname("") and basename("") to both return ".",
+ but dir_name and base_name are defined differently. */
+ {"", ".", "", false},
+ {".", ".", ".", false},
+ {"..", ".", "..", false},
+#if FILE_SYSTEM_BACKSLASH_IS_FILE_NAME_SEPARATOR
+ {"a\\", ".", "a", false},
+ {"a\\b", "a", "b", false},
+ {"\\", "\\", "\\", true},
+ {"\\/\\", "\\", "\\", true},
+ {"\\\\/", "\\", "/", true},
+ {"//\\", "/", "\\", true},
+#else
+ {"a\\", ".", "a\\", false},
+ {"a\\b", ".", "a\\b", false},
+ {"\\", ".", "\\", false},
+ {"\\/\\", "\\", "\\", false},
+ {"\\\\/", ".", "\\\\", false},
+# if DOUBLE_SLASH_IS_DISTINCT_ROOT
+ {"//\\", "//", "\\", true},
+# else
+ {"//\\", "/", "\\", true},
+# endif
+#endif
+#if FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX
+# if FILE_SYSTEM_DRIVE_PREFIX_IS_ABSOLUTE
+ {"c:", "c:", "", true},
+ {"c:/", "c:", "", true},
+ {"c://", "c:", "", true},
+ {"c:/d", "c:", "d", true},
+ {"c://d", "c:", "d", true},
+ {"c:/d/", "c:", "d", true},
+ {"c:/d/f", "c:/d", "f", true},
+ {"c:d", "c:", "d", true},
+ {"c:d/", "c:", "d", true},
+ {"c:d/f", "c:d", "f", true},
+# else
+ {"c:", "c:", "", false},
+ {"c:/", "c:/", "", true},
+ {"c://", "c:/", "", true},
+ {"c:/d", "c:/", "d", true},
+ {"c://d", "c:/", "d", true},
+ {"c:/d/", "c:/", "d", true},
+ {"c:/d/f", "c:/d", "f", true},
+ {"c:d", "c:.", "d", false},
+ {"c:d/", "c:.", "d", false},
+ {"c:d/f", "c:d", "f", false},
+# endif
+#else /* !FILE_SYSTEM_ACCEPTS_DRIVE_LETTER_PREFIX */
+ {"c:", ".", "c:", false},
+ {"c:/", ".", "c:", false},
+ {"c://", ".", "c:", false},
+ {"c:/d", "c:", "d", false},
+ {"c://d", "c:", "d", false},
+ {"c:/d/", "c:", "d", false},
+ {"c:/d/f", "c:/d", "f", false},
+ {"c:d", ".", "c:d", false},
+ {"c:d/", ".", "c:d", false},
+ {"c:d/f", "c:d", "f", false},
+#endif
+ {"1:", ".", "1:", false},
+ {"1:/", ".", "1:", false},
+ {"/:", "/", ":", true},
+ {"/:/", "/", ":", true},
+ /* End sentinel. */
+ {NULL, NULL, NULL, false}
+};
+
+int
+main ()
+{
+ struct test *t;
+ bool ok = true;
+
+ for (t = tests; t->name; t++)
+ {
+ char *dir = dir_name (t->name);
+ int dirlen = dir_len (t->name);
+ char *base = base_name (t->name);
+ int baselen = base_len (base);
+ bool absolute = IS_ABSOLUTE_FILE_NAME (t->name);
+ if (! (strcmp (dir, t->dir) == 0
+ && (dirlen == strlen (dir)
+ || (dirlen + 1 == strlen (dir) && dir[dirlen] == '.'))))
+ {
+ ok = false;
+ printf ("dir_name `%s': got `%s' len %d, expected `%s' len %d\n",
+ t->name, dir, dirlen, t->dir, strlen (t->dir));
+ }
+ if (strncmp (base, t->base, baselen) || baselen != strlen (t->base))
+ {
+ ok = false;
+ printf ("base_name `%s': got `%s' len %d, expected `%s' len %d\n",
+ t->name, base, baselen, t->base, strlen (t->base));
+ }
+ if (t->absolute != absolute)
+ {
+ ok = false;
+ printf ("`%s': got %s, expected %s\n", t->name,
+ absolute ? "absolute" : "relative",
+ t->absolute ? "absolute" : "relative");
+ }
+ free (dir);
+ }
+ return ok ? 0 : 1;
+}
- bugs in dirname module,
Eric Blake <=
- Re: bugs in dirname module, Paul Eggert, 2005/11/06
- Re: bugs in dirname module, Eric Blake, 2005/11/08
- Re: bugs in dirname module, Bruno Haible, 2005/11/09
- Re: bugs in dirname module, Paul Eggert, 2005/11/09
- Re: bugs in dirname module, Eric Blake, 2005/11/09
- Re: bugs in dirname module, Paul Eggert, 2005/11/10
- Re: bugs in dirname module, Eric Blake, 2005/11/10