bug-gnulib
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

link, linkat, and trailing slash (was: Re: gnulib new testdir / current


From: Bruno Haible
Subject: link, linkat, and trailing slash (was: Re: gnulib new testdir / current build messages)
Date: Sun, 1 Aug 2010 16:59:32 +0200
User-agent: KMail/1.9.9

Hello Eric, Jim,

Rainer Tammer wrote:
> >> FAIL: test-linkat
> >> main(), line 183 in "test-linkat.c"
> ...
> That's the result:
> 
> # ./test-linkat
> errno=20 (AIX errno.h: ENOTDIR)
> test-linkat.c:184: assertion failed

The error code ENOTDIR here comes from my handling of the trailing slash
bug, in lib/linkat.c, code copied from lib/link.c. Let's look at the two
cases:

1a) When "a" is a symlink, and "b" is a directory, what is the errno value for

      linkat (fd1, "a", fd2, "b/", 0)

1b) When "a" is a regular file, and "b" is a directory, what is the errno value
    for

      link ("a", "b/")
    and
      linkat (fd1, "a", fd2, "b/", 0)

2a) When "a" is a symlink, and "b" is nonexistent, what is the errno value for

      linkat (fd1, "a", fd2, "b/", 0)

2b) When "a" is a regular file, and "b" is nonexistent, what is the errno value
    for

      link ("a", "b/")
    and
      linkat (fd1, "a", fd2, "b/", 0)

From reading POSIX
<http://www.opengroup.org/onlinepubs/9699919799/functions/link.html>,
especially the descriptions of ENOTDIR and EEXIST, I think that the right
answer is:
  1a), 1b): EEXIST,
  2a), 2b): ENOENT.

So the test at test-linkat.c:183 is fine, and the test at test-link.h:121
should be strengthened to
  ASSERT (errno == ENOENT);

Here is a suggested fix to
  - make the tests stricter,
  - change the workaround in link.c so that case 2b yields ENOENT on platforms
    like Solaris 10,
  - change the workaround in linkat.c so that cases 1 and 2 hopefully yield
    EEXIST and ENOENT instead of ENOTDIR (verified to work on Linux with
    gl_cv_func_link_works=no gl_cv_func_linkat_slash=no, but needs to be
    tested by Rainer too).

Opinions?


2010-08-01  Bruno Haible  <address@hidden>

        link, linkat: Avoid returning ENOTDIR when file2 ends in a slash.
        * lib/link.c (rpl_link): When file2 ends in a slash and file1 doesn't,
        strip the trailing slash and test whether the base directory exists.
        * lib/linkat.c (rpl_linkat): Likewise.
        * modules/linkat (Dependencies): Add strdup-posix.
        * tests/test-link.h (test_link): Expect ENOENT when file2 ends in "/"
        or "/." or "//.".
        * tests/test-linkat.c (main): Likewise.

--- lib/link.c.orig     Sun Aug  1 15:47:05 2010
+++ lib/link.c  Sun Aug  1 15:29:50 2010
@@ -158,8 +158,7 @@
   /* 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] == '/'))
+  if (len1 && file1[len1 - 1] == '/')
     {
       /* Let link() decide whether hard-linking directories is legal.
          If stat() fails, then link() should fail for the same reason
@@ -176,24 +175,30 @@
     }
   else
     {
-      /* Fix Cygwin 1.5.x bug where link("a","b/.") creates file "b".  */
+      /* Fix Solaris 10 bug where link("a","b/") creates file "b".
+         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.  */
+      /* Strip off the basename and the slashes before it, then check that
+         the dirname exists.  */
       p = strrchr (dir, '/');
       if (p)
         {
-          *p = '\0';
-          if (stat (dir, &st) == -1)
+          while (p > dir && p[-1] == '/')
+            p--;
+          if (p > dir)
             {
-              int saved_errno = errno;
-              free (dir);
-              errno = saved_errno;
-              return -1;
+              *p = '\0';
+              if (stat (dir, &st) == -1)
+                {
+                  int saved_errno = errno;
+                  free (dir);
+                  errno = saved_errno;
+                  return -1;
+                }
             }
         }
       free (dir);
--- lib/linkat.c.orig   Sun Aug  1 15:47:05 2010
+++ lib/linkat.c        Sun Aug  1 15:29:50 2010
@@ -273,8 +273,7 @@
   {
     size_t len1 = strlen (file1);
     size_t len2 = strlen (file2);
-    if ((len1 && file1[len1 - 1] == '/')
-        || (len2 && file2[len2 - 1] == '/'))
+    if (len1 && file1[len1 - 1] == '/')
       {
         /* Let linkat() decide whether hard-linking directories is legal.
            If fstatat() fails, then linkat() should fail for the same reason;
@@ -288,6 +287,35 @@
             return -1;
           }
       }
+    else if (len2 && file2[len2 - 1] == '/')
+      {
+        /* Fix AIX 7.1 bug where link(fd1,"a",fd2,"b/",0) creates file "b".  */
+        char *dir = strdup (file2);
+        struct stat st;
+        char *p;
+        if (!dir)
+          return -1;
+        /* Strip off the basename and the slashes before it, then check that
+           the dirname exists.  */
+        p = strrchr (dir, '/');
+        if (p)
+          {
+            while (p > dir && p[-1] == '/')
+              p--;
+            if (p > dir)
+              {
+                *p = '\0';
+                if (fstatat (fd2, dir, &st, 0) == -1)
+                  {
+                    int saved_errno = errno;
+                    free (dir);
+                    errno = saved_errno;
+                    return -1;
+                  }
+              }
+          }
+        free (dir);
+      }
   }
 #endif
 
--- modules/linkat.orig Sun Aug  1 15:47:05 2010
+++ modules/linkat      Sun Aug  1 15:29:50 2010
@@ -21,6 +21,7 @@
 readlink
 same-inode
 stpcpy
+strdup-posix
 symlink
 symlinkat
 unistd
--- tests/test-link.h.orig      Sun Aug  1 15:47:05 2010
+++ tests/test-link.h   Sun Aug  1 15:29:50 2010
@@ -1,5 +1,5 @@
 /* Test of link() function.
-   Copyright (C) 2009, 2010 Free Software Foundation, Inc.
+   Copyright (C) 2009-2010 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
@@ -111,14 +111,17 @@
   ASSERT (func (BASE "c", BASE "e") == -1);
   ASSERT (errno == ENOENT);
   errno = 0;
+  ASSERT (func (BASE "a/", BASE "c") == -1);
+  ASSERT (errno == ENOTDIR);
+  errno = 0;
   ASSERT (func (BASE "a", BASE "c/.") == -1);
   ASSERT (errno == ENOENT);
   errno = 0;
-  ASSERT (func (BASE "a/", BASE "c") == -1);
-  ASSERT (errno == ENOTDIR);
+  ASSERT (func (BASE "a", BASE "c//.") == -1);
+  ASSERT (errno == ENOENT);
   errno = 0;
   ASSERT (func (BASE "a", BASE "c/") == -1);
-  ASSERT (errno == ENOTDIR || errno == ENOENT);
+  ASSERT (errno == ENOENT);
 
   /* Most platforms reject hard links to directories, and even on
      those that do permit it, most users can't create them.  We assume
--- tests/test-linkat.c.orig    Sun Aug  1 15:47:05 2010
+++ tests/test-linkat.c Sun Aug  1 15:30:44 2010
@@ -253,6 +253,17 @@
                   AT_SYMLINK_FOLLOW) == -1);
   ASSERT (errno == ENOENT);
 
+  /* Trailing slash handling in file2 argument.  */
+  errno = 0;
+  ASSERT (linkat (dfd, BASE "link1", dfd, BASE "link5/.", 0) == -1);
+  ASSERT (errno == ENOENT);
+  errno = 0;
+  ASSERT (linkat (dfd, BASE "link1", dfd, BASE "link5//.", 0) == -1);
+  ASSERT (errno == ENOENT);
+  errno = 0;
+  ASSERT (linkat (dfd, BASE "link1", dfd, BASE "link5/", 0) == -1);
+  ASSERT (errno == ENOENT);
+
   /* Check for hard links to symlinks.  */
   ASSERT (linkat (dfd, BASE "link1", dfd, BASE "link5", 0) == 0);
   check_same_link (BASE "link1", BASE "link5");



reply via email to

[Prev in Thread] Current Thread [Next in Thread]