Eric Blake <ebb9 <at> byu.net> writes:
> Part of the problem on mingw is that the fchdir module relies on
> canonicalize doing the right thing, but mingw's paths (with drive letters
> and \, rather than leading / for absolute) throws canonicalize for a loop;
> and cross-compilation tries to replace getcwd, even though mingw's getcwd
> always works. At that point, the populated dirs[fd].name mapping have
> invalid entries, which hurt attempts to do fstat(dfd). It doesn't help
> that mingw stat() reports st_ino == 0 for every directory, rendering
> SAME_INODE useless. Oh well, I'll have to spend more time porting those
> pieces to mingw.
Among other things, testing shows that mingw supports getcwd(NULL,0), so the
cross-compilation guess needs to be improved there. stat() has the annoying
property that a trailing slash changes semantics (but / and \\ are
interchangeable), and in an inconsistent manner (you can't always stat the
result of getcwd!):
getcwd reports: c:\
stat("c:",&st) => fails
stat("c:/",&st) => passes
getcwd reports: c:\dir
stat("c:/dir",&st) => passes
stat("c:/dir/",&st) => fails
getcwd reports: \\machine\share
stat("\\\\machine\\share",&st) => fails
stat("\\\\machine\\share\\",&st) => passes
getcwd reports: \\machine\share\dir
stat("\\\\machine\\share\\dir",&st) => passes
stat("\\\\machine\\share\\dir\\",&st) => fails
And it doesn't help that canonicalize is not consistently using ISSLASH macros
to munge '/' and '\\' into a canonical form. Yep, definitely some groundwork
to lay here before linkat and renameat will work on mingw.
Meanwhile, I noticed that the link module has a bug on mingw, where link
("a","b/.") created the regular file "b" rather than failing with ENOENT (I
noticed it because cygwin 1.5 had the same bug, and I noticed that while
enhancing the link module to work around Solaris 8 handling of trailing / in
link). I'll probably be applying this soon...
From: Eric Blake <e...@byu.net>
Date: Wed, 9 Sep 2009 11:06:44 -0600
Subject: [PATCH 1/2] test-link: consolidate into single C program, test more
cases
* tests/test-link.sh: Delete.
* tests/test-link.c: Test more error conditions. Exposes bugs on
at least Cygwin and Solaris.
* modules/link-tests (Files): Remove unused file.
(Depends-on): Add errno, sys_stat.
(Makefile.am): Simplify.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 8 ++++
modules/link-tests | 6 ++--
tests/test-link.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++---
tests/test-link.sh | 37 -------------------
4 files changed, 109 insertions(+), 45 deletions(-)
delete mode 100755 tests/test-link.sh
diff --git a/ChangeLog b/ChangeLog
index 08fe19a..8372ebe 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,13 @@
2009-09-09 Eric Blake <e...@byu.net>
+ test-link: consolidate into single C program, test more cases
+ * tests/test-link.sh: Delete.
+ * tests/test-link.c: Test more error conditions. Exposes bugs on
+ at least Cygwin and Solaris.
+ * modules/link-tests (Files): Remove unused file.
+ (Depends-on): Add errno, sys_stat.
+ (Makefile.am): Simplify.
+
dirname: add library-safe mdir_name
* lib/dirname.h (mdir_name): New prototype.
* lib/dirname.c (dir_name): Move guts...
diff --git a/modules/link-tests b/modules/link-tests
index c1ec52c..ca61deb 100644
--- a/modules/link-tests
+++ b/modules/link-tests
@@ -1,12 +1,12 @@
Files:
-tests/test-link.sh
tests/test-link.c
Depends-on:
+errno
+sys_stat
configure.ac:
Makefile.am:
-TESTS += test-link.sh
-TESTS_ENVIRONMENT += EXEEXT='@EXEEXT@'
+TESTS += test-link
check_PROGRAMS += test-link
diff --git a/tests/test-link.c b/tests/test-link.c
index 250a821..fbc794a 100644
--- a/tests/test-link.c
+++ b/tests/test-link.c
@@ -19,8 +19,12 @@
#include <unistd.h>
#include <errno.h>
+#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <unistd.h>
#define ASSERT(expr) \
do \
@@ -34,30 +38,119 @@
} \
while (0)
+#define BASE "test-link.t"
+
int
main (int argc, char **argv)
{
+ int fd;
int ret;
- ASSERT (argc == 3);
+ /* Remove any garbage left from previous partial runs. */
+ ASSERT (system ("rm -rf " BASE "*") == 0);
+
+ /* Create first file. */
+ fd = open (BASE "a", O_CREAT | O_EXCL | O_WRONLY, 0600);
+ ASSERT (0 <= fd);
+ ASSERT (write (fd, "hello", 5) == 5);
+ ASSERT (close (fd) == 0);
- ret = link (argv[1], argv[2]);
- if (ret < 0)
+ /* Not all file systems support link. Mingw doesn't have reliable
+ st_nlink on hard links, but our implementation does fail with
+ EPERM on poor file systems, and we can detect the inferior stat()
+ via st_ino. Cygwin 1.5.x copies rather than links files on those
+ file systems, but there, st_nlink and st_ino are reliable. */
+ ret = link (BASE "a", BASE "b");
+ if (!ret)
+ {
+ struct stat st;
+ ASSERT (stat (BASE "b", &st) == 0);
+ if (st.st_ino && st.st_nlink != 2)
+ {
+ ASSERT (unlink (BASE "b") == 0);
+ errno = EPERM;
+ ret = -1;
+ }
+ }
+ if (ret == -1)
{
/* If the device does not support hard links, errno is
EPERM on Linux, EOPNOTSUPP on FreeBSD. */
switch (errno)
{
case EPERM:
-#ifdef EOPNOTSUPP
case EOPNOTSUPP:
-#endif
+ fputs ("skipping test: "
+ "hard links are not supported on this file system\n", stderr);
+ ASSERT (unlink (BASE "a") == 0);
return 77;
default:
perror ("link");
return 1;
}
}
+ ASSERT (ret == 0);
+
+ /* Now, for some behavior tests. Modify the contents of 'b', and
+ ensure that 'a' can see it, both while 'b' exists and after. */
+ fd = open (BASE "b", O_APPEND | O_WRONLY);
+ ASSERT (0 <= fd);
+ ASSERT (write (fd, "world", 5) == 5);
+ ASSERT (close (fd) == 0);
+ {
+ char buf[11] = { 0 };
+ fd = open (BASE "a", O_RDONLY);
+ ASSERT (0 <= fd);
+ ASSERT (read (fd, buf, 10) == 10);
+ ASSERT (strcmp (buf, "helloworld") == 0);
+ ASSERT (close (fd) == 0);
+ ASSERT (unlink (BASE "b") == 0);
+ fd = open (BASE "a", O_RDONLY);
+ ASSERT (0 <= fd);
+ ASSERT (read (fd, buf, 10) == 10);
+ ASSERT (strcmp (buf, "helloworld") == 0);
+ ASSERT (close (fd) == 0);
+ }
+
+ /* Test for various error conditions. Assumes hard links to
+ directories are not permitted. */
+ ASSERT (mkdir (BASE "d", 0700) == 0);
+ errno = 0;
+ ASSERT (link (BASE "a", ".") == -1);
+ ASSERT (errno == EEXIST || errno == EINVAL);
+ errno = 0;
+ ASSERT (link (BASE "a", BASE "a") == -1);
+ ASSERT (errno == EEXIST);
+ ASSERT (link (BASE "a", BASE "b") == 0);
+ errno = 0;
+ ASSERT (link (BASE "a", BASE "b") == -1);
+ ASSERT (errno == EEXIST);
+ errno = 0;
+ ASSERT (link (BASE "a", BASE "d") == -1);
+ ASSERT (errno == EEXIST);
+ errno = 0;
+ ASSERT (link (BASE "c", BASE "e") == -1);
+ ASSERT (errno == ENOENT);
+ errno = 0;
+ ASSERT (link (BASE "a", BASE "c/.") == -1);
+ ASSERT (errno == ENOENT);
+ errno = 0;
+ ASSERT (link (BASE "a/", BASE "c") == -1);
+ ASSERT (errno == ENOTDIR);
+ errno = 0;
+ ASSERT (link (BASE "a", BASE "c/") == -1);
+ ASSERT (errno == ENOTDIR);
+ errno = 0;
+ ASSERT (link (BASE "d", BASE "c") == -1);
+ ASSERT (errno == EPERM || errno == EACCES);
+
+ /* Clean up. */
+ ASSERT (unlink (BASE "a") == 0);
+ ASSERT (unlink (BASE "b") == 0);
+ errno = 0;
+ ASSERT (unlink (BASE "c") == -1);
+ ASSERT (errno == ENOENT);
+ ASSERT (rmdir (BASE "d") == 0);
return 0;
}
diff --git a/tests/test-link.sh b/tests/test-link.sh
deleted file mode 100755
index 1519f9d..0000000
--- a/tests/test-link.sh
+++ /dev/null
@@ -1,37 +0,0 @@
-#!/bin/sh
-
-tmpfiles="test-link-a.txt test-link-b.txt test-link-c.txt"
-trap 'rm -fr $tmpfiles' 1 2 3 15
-
-# Create a file.
-echo "hello" > test-link-a.txt || exit 1
-
-# Use link() to create a new name for it.
-./test-link${EXEEXT} test-link-a.txt test-link-b.txt
-case $? in
- 0) ;;
- 77)
- echo "Skipping test: hard links are not supported on this file system"
- rm -fr $tmpfiles
- exit 77
- ;;
- *) exit 1 ;;
-esac
-cmp test-link-a.txt test-link-b.txt || exit 1
-
-# Modify the contents of the first file.
-echo "world" >> test-link-a.txt || exit 1
-cmp test-link-a.txt test-link-b.txt || exit 1
-
-# Modify the contents of the second file.
-echo "some text" >> test-link-b.txt || exit 1
-cmp test-link-a.txt test-link-b.txt || exit 1
-
-# Delete the first file, then verity the second file still has the same
-# contents.
-cp test-link-a.txt test-link-c.txt || exit 1
-rm test-link-a.txt || exit 1
-cmp test-link-b.txt test-link-c.txt || exit 1
-
-rm -fr $tmpfiles
-exit 0
--
1.6.3.2
>From 9101691fafa16a95b3fb0a95abe9c43da26fa907 Mon Sep 17 00:00:00 2001
From: Eric Blake <e...@byu.net>
Date: Wed, 9 Sep 2009 15:25:26 -0600
Subject: [PATCH 2/2] link: fix platform bugs
* m4/link.m4 (gl_FUNC_LINK): Detect Solaris and Cygwin bugs.
* lib/link.c (link): Work around them. Fix related mingw bug.
* m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add REPLACE_LINK.
* modules/unistd (Makefile.am): Substitute it.
* lib/unistd.in.h (link): Declare replacement.
* doc/posix-functions/link.texi (link): Document this.
* modules/link (Depends-on): Add strdup-posix, sys_stat.
Signed-off-by: Eric Blake <e...@byu.net>
---
ChangeLog | 9 +++
doc/posix-functions/link.texi | 4 ++
lib/link.c | 110 +++++++++++++++++++++++++++++++++++++---
lib/unistd.in.h | 5 ++-
m4/link.m4 | 21 ++++++--
m4/unistd_h.m4 | 3 +-
modules/link | 4 +-
modules/unistd | 1 +
8 files changed, 140 insertions(+), 17 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 8372ebe..d35aba7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
2009-09-09 Eric Blake <e...@byu.net>
+ link: fix platform bugs
+ * m4/link.m4 (gl_FUNC_LINK): Detect Solaris and Cygwin bugs.
+ * lib/link.c (link): Work around them. Fix related mingw bug.
+ * m4/unistd_h.m4 (gl_UNISTD_H_DEFAULTS): Add REPLACE_LINK.
+ * modules/unistd (Makefile.am): Substitute it.
+ * lib/unistd.in.h (link): Declare replacement.
+ * doc/posix-functions/link.texi (link): Document this.
+ * modules/link (Depends-on): Add strdup-posix, sys_stat.
+
test-link: consolidate into single C program, test more cases
* tests/test-link.sh: Delete.
* tests/test-link.c: Test more error conditions. Exposes bugs on
diff --git a/doc/posix-functions/link.texi b/doc/posix-functions/link.texi
index ace07cd..c785371 100644
--- a/doc/posix-functions/link.texi
+++ b/doc/posix-functions/link.texi
@@ -11,6 +11,10 @@ link
@item
This function is missing on some platforms:
mingw.
+...@item
+This function fails to reject trailing slashes on non-directories on
+some platforms:
+Solaris, Cygwin 1.5.x.
@end itemize
Portability problems not fixed by Gnulib:
diff --git a/lib/link.c b/lib/link.c
index 2b5a97f..72b8600 100644
--- a/lib/link.c
+++ b/lib/link.c
@@ -18,13 +18,18 @@
#include <config.h>
-#if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
-
-#define WIN32_LEAN_AND_MEAN
#include <unistd.h>
-#include <windows.h>
#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/stat.h>
+
+#if !HAVE_LINK
+# if (defined _WIN32 || defined __WIN32__) && ! defined __CYGWIN__
+
+# define WIN32_LEAN_AND_MEAN
+# include <windows.h>
/* CreateHardLink was introduced only in Windows 2000. */
typedef BOOL (WINAPI * CreateHardLinkFuncType) (LPCTSTR lpFileName,
@@ -46,8 +51,11 @@ initialize (void)
}
int
-link (const char *path1, const char *path2)
+link (const char *file1, const char *file2)
{
+ char *dir;
+ size_t len1 = strlen (file1);
+ size_t len2 = strlen (file2);
if (!initialized)
initialize ();
if (CreateHardLinkFunc == NULL)
@@ -56,7 +64,39 @@ link (const char *path1, const char *path2)
errno = EPERM;
return -1;
}
- if (CreateHardLinkFunc (path2, path1, NULL) == 0)
+ /* Reject trailing slashes on non-directories; mingw does not
+ support hard-linking directories. */
+ if ((len1 && (file1[len1 - 1] == '/' || file1[len1 - 1] == '\\'))
+ || (len2 && (file2[len2 - 1] == '/' || file2[len2 - 1] == '\\')))
+ {
+ struct stat st;
+ if (stat (file1, &st) == 0 && S_ISDIR (st.st_mode))
+ errno = EPERM;
+ else
+ errno = ENOTDIR;
+ return -1;
+ }
+ /* CreateHardLink("b/.","a",NULL) creates file "b", so we must check
+ that dirname(file2) exists. */
+ dir = strdup (file2);
+ if (!dir)
+ return -1;
+ {
+ struct stat st;
+ char *p = strchr (dir, '\0');
+ while (dir < p && (*--p != '/' && *p != '\\'));
+ *p = '\0';
+ if (p != dir && stat (dir, &st) == -1)
+ {
+ int saved_errno = errno;
+ free (dir);
+ errno = saved_errno;
+ return -1;
+ }
+ free (dir);
+ }
+ /* Now create the link. */
+ if (CreateHardLinkFunc (file2, file1, NULL) == 0)
{
/* It is not documented which errors CreateHardLink() can produce.
* The following conversions are based on tests on a Windows XP SP2
@@ -102,8 +142,60 @@ link (const char *path1, const char *path2)
return 0;
}
-#else /* !Windows */
+# else /* !Windows */
+
+# error "This platform lacks a link function, and Gnulib doesn't provide a
replacement. This is a bug in Gnulib."
+
+# endif /* !Windows */
+#else /* HAVE_LINK */
-#error "This platform lacks a link function, and Gnulib doesn't provide a
replacement. This is a bug in Gnulib."
+# undef link
-#endif /* !Windows */
+/* Create a hard link from FILE1 to FILE2, working around platform bugs. */
+int
+rpl_link (char const *file1, char const *file2)
+{
+ /* Reject trailing slashes on non-directories. */
+ size_t len1 = strlen (file1);
+ size_t len2 = strlen (file2);
+ if ((len1 && file1[len1 - 1] == '/')
+ || (len2 && file2[len2 - 1] == '/'))
+ {
+ /* Let link() decide whether hard-linking directories is legal.
+ If stat() fails, link() will probably fail for the same
+ reason; so we only have to worry about successful stat() and
+ non-directory. */
+ struct stat st;
+ if (stat (file1, &st) == 0 && !S_ISDIR (st.st_mode))
+ {
+ errno = ENOTDIR;
+ return -1;
+ }
+ }
+ else
+ {
+ /* Fix Cygwin 1.5.x bug where link("a","b/.") creates file "b". */
+ char *dir = strdup (file2);
+ struct stat st;
+ char *p;
+ if (!dir)
+ return -1;
+ /* We already know file2 does not end in slash. Strip off the
+ basename, then check that the dirname exists. */
+ p = strrchr (dir, '/');
+ if (p)
+ {
+ *p = '\0';
+ if (stat (dir, &st) == -1)
+ {
+ int saved_errno = errno;
+ free (dir);
+ errno = saved_errno;
+ return -1;
+ }
+ }
+ free (dir);
+ }
+ return link (file1, file2);
+}
+#endif /* HAVE_LINK */
diff --git a/lib/unistd.in.h b/lib/unistd.in.h
index 902b025..f191412 100644
--- a/lib/unistd.in.h
+++ b/lib/unistd.in.h
@@ -595,11 +595,14 @@ extern int lchown (char const *file, uid_t owner, gid_t
group);
#if @GNULIB_LINK@
+# if @REPLACE_LINK@
+# define link rpl_link
+# endif
/* Create a new hard link for an existing file.
Return 0 if successful, otherwise -1 and errno set.
See POSIX:2001 specification
<http://www.opengroup.org/susv3xsh/link.html>. */
-# if !...@have_link@
+# if !...@have_link@ || @REPLACE_LINK@
extern int link (const char *path1, const char *path2);
# endif
#elif defined GNULIB_POSIXCHECK
diff --git a/m4/link.m4 b/m4/link.m4
index 349d537..fc071cd 100644
--- a/m4/link.m4
+++ b/m4/link.m4
@@ -1,4 +1,4 @@
-# link.m4 serial 1
+# link.m4 serial 2
dnl Copyright (C) 2009 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,9 +11,20 @@ AC_DEFUN([gl_FUNC_LINK],
if test $ac_cv_func_link = no; then
HAVE_LINK=0
AC_LIBOBJ([link])
- gl_PREREQ_LINK
+ else
+ AC_CACHE_CHECK([whether link handles trailing slash correctly],
+ [gl_cv_func_link_works],
+ [touch conftest.a
+ AC_RUN_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <unistd.h>
+]], [[return !link ("conftest.a", "conftest.b/");]])],
+ [gl_cv_func_link_works=yes], [gl_cv_func_link_works=no],
+ [gl_cv_func_link_works="guessing no"])
+ rm -f conftest.a conftest.b])
+ if test $gl_cv_func_link_works != yes; then
+ REPLACE_LINK=1
+ AC_LIBOBJ([link])
+ fi
fi
])
-
-# Prerequisites of lib/link.c.
-AC_DEFUN([gl_PREREQ_LINK], [:])
diff --git a/m4/unistd_h.m4 b/m4/unistd_h.m4
index 84f0755..7347e38 100644
--- a/m4/unistd_h.m4
+++ b/m4/unistd_h.m4
@@ -1,4 +1,4 @@
-# unistd_h.m4 serial 24
+# unistd_h.m4 serial 25
dnl Copyright (C) 2006-2009 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -94,6 +94,7 @@ AC_DEFUN([gl_UNISTD_H_DEFAULTS],
REPLACE_GETCWD=0; AC_SUBST([REPLACE_GETCWD])
REPLACE_GETPAGESIZE=0; AC_SUBST([REPLACE_GETPAGESIZE])
REPLACE_LCHOWN=0; AC_SUBST([REPLACE_LCHOWN])
+ REPLACE_LINK=0; AC_SUBST([REPLACE_LINK])
REPLACE_LSEEK=0; AC_SUBST([REPLACE_LSEEK])
REPLACE_WRITE=0; AC_SUBST([REPLACE_WRITE])
UNISTD_H_HAVE_WINSOCK2_H=0; AC_SUBST([UNISTD_H_HAVE_WINSOCK2_H])
diff --git a/modules/link b/modules/link
index 31d1af4..9492950 100644
--- a/modules/link
+++ b/modules/link
@@ -6,6 +6,8 @@ lib/link.c
m4/link.m4
Depends-on:
+strdup-posix
+sys_stat
unistd
configure.ac:
@@ -21,4 +23,4 @@ License:
LGPLv2+
Maintainer:
-Martin Lambers
+Martin Lambers, Eric Blake
diff --git a/modules/unistd b/modules/unistd
index 1f8b29e..37ecaaa 100644
--- a/modules/unistd
+++ b/modules/unistd
@@ -86,6 +86,7 @@ unistd.h: unistd.in.h
-e 's|@''REPLACE_GETCWD''@|$(REPLACE_GETCWD)|g' \
-e 's|@''REPLACE_GETPAGESIZE''@|$(REPLACE_GETPAGESIZE)|g' \
-e 's|@''REPLACE_LCHOWN''@|$(REPLACE_LCHOWN)|g' \
+ -e 's|@''REPLACE_LINK''@|$(REPLACE_LINK)|g' \
-e 's|@''REPLACE_LSEEK''@|$(REPLACE_LSEEK)|g' \
-e 's|@''REPLACE_WRITE''@|$(REPLACE_WRITE)|g' \
-e 's|@''UNISTD_H_HAVE_WINSOCK2_H''@|$(UNISTD_H_HAVE_WINSOCK2_H)
|g' \
--
1.6.3.2
