X-Authentication-Warning: delorie.com: mail set sender to djgpp-workers-bounces using -f
Date: Sun, 09 Nov 2003 13:30:50 +0000
From: "Richard Dawe" <rich AT phekda DOT freeserve DOT co DOT uk>
Sender: rich AT phekda DOT freeserve DOT co DOT uk
To: djgpp-workers AT delorie DOT com
X-Mailer: Emacs 21.3.50 (via feedmail 8.3.emacs20_6 I) and Blat ver 1.8.6
Subject: open, O_CREAT|O_EXCL and symlinks, revision 2 [PATCH]
Message-Id: <E1AIpTQ-0000MC-00@phekda.freeserve.co.uk>
Reply-To: djgpp-workers AT delorie DOT com

Hello.

Here's revision 2 of my patch to fix open w.r.t. symlinks. I previously
wrote:

"open(symlink, O_CREAT|O_EXCL, ...) should fail for a symlink
according to SUSv3. Currently it doesn't. Below is a patch to fix that,
including an update to the open() tests."

Changes since the previous version:

* Start again: My previous patch called __solve_symlinks
  and __solve_dir_symlinks every time open was called. This made open
  slower.

  Now it only calls both of them if we're creating a file.
  So the general case is faster - only creating a file is slower.
  I think this is acceptable. (*)

* Added an extra test case to the test program. Now we test
  O_CREAT|O_EXCL|O_NOLINK too.

I tested this patch with fileutils 4.1's test suite. There were
no regressions.

OK to commit?

Bye, Rich

PS: There's a bug in __internal_readlink. You can't pass it
file descriptor 0, since it uses 0 to mean "no file descriptor".
I'll fix that sometime. The fix is simple.

Index: src/libc/posix/fcntl/open.c
===================================================================
RCS file: /cvs/djgpp/djgpp/src/libc/posix/fcntl/open.c,v
retrieving revision 1.12
diff -p -u -3 -r1.12 open.c
--- src/libc/posix/fcntl/open.c	26 Mar 2003 19:42:00 -0000	1.12
+++ src/libc/posix/fcntl/open.c	9 Nov 2003 13:13:36 -0000
@@ -113,17 +113,19 @@ open(const char* filename, int oflag, ..
   int fd, dmode, bintext, dont_have_share;
   char real_name[FILENAME_MAX + 1];
   int should_create = (oflag & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL);
+  int dirs_solved = 0; /* Only directories resolved in real_name? */
 
   /* Solve symlinks and honor O_NOLINK flag  */
   if (oflag & O_NOLINK)
   {
      if (!__solve_dir_symlinks(filename, real_name))
         return -1; /* errno from from __solve_dir_symlinks() */
+     dirs_solved = 1;
   }
   else
   {
      if (!__solve_symlinks(filename, real_name))
-        return -1; 
+        return -1; /* errno from from __solve_symlinks() */
   }
 
   /* Honor O_NOFOLLOW flag. */
@@ -163,12 +165,49 @@ open(const char* filename, int oflag, ..
 
   /* Check this up front, to reduce cost and minimize effect */
   if (should_create)
+  {
+    /* Symlink: We're not allowed to follow a symlink, when creating a file
+     * with the same name as the symlink.
+     */
+    char temp_name[FILENAME_MAX + 1];
+#define IRD_BUF_SIZE FILENAME_MAX + 1
+    char ird_buf[IRD_BUF_SIZE];
+    const size_t ird_bufsize = IRD_BUF_SIZE;
+#undef IRD_BUF_SIZE
+
+    if (!dirs_solved)
+    {
+      if (!__solve_dir_symlinks(filename, temp_name))
+        return -1; /* errno from from __solve_dir_symlinks() */
+    }
+    else
+    {
+      strcpy(temp_name, real_name);
+    }
+
+    if (__internal_readlink(temp_name, 0, ird_buf, ird_bufsize) < 0)
+    {
+      /* If the error is something other than "doesn't exist"
+       * or "isn't a symlink", return it.
+       */
+      if ((errno != ENOENT) && (errno != EINVAL))
+	return -1; /* errno from __internal_readlink() */
+    }
+    else
+    {
+      /* It's a symlink. */
+      errno = EEXIST;
+      return -1;
+    }
+
+    /* Normal file */
     if (__file_exists(real_name))
     {
       /* file exists and we didn't want it to */
       errno = EEXIST;
       return -1;
     }
+  }
 
   /* figure out what mode we're opening the file in */
   bintext = oflag & (O_TEXT | O_BINARY);
Index: src/libc/posix/fcntl/open.txh
===================================================================
RCS file: /cvs/djgpp/djgpp/src/libc/posix/fcntl/open.txh,v
retrieving revision 1.9
diff -p -u -3 -r1.9 open.txh
--- src/libc/posix/fcntl/open.txh	26 Mar 2003 19:42:00 -0000	1.9
+++ src/libc/posix/fcntl/open.txh	9 Nov 2003 13:13:38 -0000
@@ -39,7 +39,9 @@ If the file does exist, it is truncated 
 @item O_EXCL
 
 If the file exists, and @code{O_CREAT} is also specified, the
-@code{open} call will fail. 
+@code{open} call will fail.  If the file is a symlink and @code{O_CREAT}
+is also specified, then the contents of the symlink are ignored -
+the @code{open} call will fail.
 
 @item O_APPEND
 
Index: tests/libc/posix/fcntl/open.c
===================================================================
RCS file: /cvs/djgpp/djgpp/tests/libc/posix/fcntl/open.c,v
retrieving revision 1.4
diff -p -u -3 -r1.4 open.c
--- tests/libc/posix/fcntl/open.c	11 Mar 2001 05:22:18 -0000	1.4
+++ tests/libc/posix/fcntl/open.c	9 Nov 2003 13:13:43 -0000
@@ -9,24 +9,28 @@
  *        we really have opened a referred file.
  *   4. Open symlink in a symlink subdir with O_NOFOLLOW flag. Should fail with
  *        ELOOP. 
- *   5. Open a symlink with O_NOLINK but with symlinks in leading dirs.
- *   6. Open a symlink file in a simple subdir. Check if we really have opened
+ *   5. Open an existing symlink with O_CREAT|O_EXCL. Should fail
+ *        with EEXIST.
+ *   6. Open an existing symlink with O_CREAT|O_EXCL|O_NOLINK. Should fail
+ *        with EEXIST.
+ *   7. Open a symlink with O_NOLINK but with symlinks in leading dirs.
+ *   8. Open a symlink file in a simple subdir. Check if we really have opened
  *        the referred file. 
- *   7. Open and close a file with O_TEMPORARY. Verify the file is deleted.
- *   8. Open a file with O_TEMPORARY. Duplicate the file handle. Close both
-        handles. Verify the file is deleted at the correct time.
-     9. Open a file with O_TEMPORARY. Duplicate the file handle to an
-        unused file handle. Close both handles. Verify the file is deleted
-        at the correct time.
-     10. Open a file with O_TEMPORARY. Open the same file without O_TEMPORARY.
-         Close both handles. Verify the file is deleted at the correct time.
-     11. Open a file without O_TEMPORARY. Open the same file with O_TEMPORARY.
-         Duplicate the handle opened without O_TEMPORARY on to the handle
-         opened with O_TEMPORARY. Close both handles. Verify the file is
-         deleted at the correct time.
-     12. Open a file with O_TEMPORARY. Open it again with O_TEMPORARY.
-         Duplicate one handle on to the other. Close both handles. Verify the
-         file is deleted at the correct time.
+ *   9. Open and close a file with O_TEMPORARY. Verify the file is deleted.
+ *   10. Open a file with O_TEMPORARY. Duplicate the file handle. Close both
+ *       handles. Verify the file is deleted at the correct time.
+ *   11. Open a file with O_TEMPORARY. Duplicate the file handle to an
+ *       unused file handle. Close both handles. Verify the file is deleted
+ *       at the correct time.
+ *   12. Open a file with O_TEMPORARY. Open the same file without O_TEMPORARY.
+ *       Close both handles. Verify the file is deleted at the correct time.
+ *   13. Open a file without O_TEMPORARY. Open the same file with O_TEMPORARY.
+ *       Duplicate the handle opened without O_TEMPORARY on to the handle
+ *       opened with O_TEMPORARY. Close both handles. Verify the file is
+ *       deleted at the correct time.
+ *   14. Open a file with O_TEMPORARY. Open it again with O_TEMPORARY.
+ *       Duplicate one handle on to the other. Close both handles. Verify the
+ *       file is deleted at the correct time.
  */
 #include <errno.h>
 #include <fcntl.h>
@@ -45,32 +49,79 @@ static int testnum;
 int main(void)
 {
    int fd;
+
    if (!__file_exists("file1") || !__file_exists("test1") || 
        !__file_exists("test2") || !__file_exists("dir/file1"))
    {
       fprintf(stderr, "Required data file is missing\n");
-      exit(1);
+      exit(EXIT_FAILURE);
+   }
+
+   if (__file_exists("doesnotexist"))
+   {
+      fprintf(stderr,
+	      "File 'doesnotexist' is in the way - "
+	      "please remove it!\n");
+      exit(EXIT_FAILURE);
    }
+
    test_success("test1", O_RDONLY, "file1");
    test_success("test1", O_RDONLY | O_NOLINK, "!<symlink>");
    test_success("test2/file1", O_RDONLY | O_NOFOLLOW, "file1");
+
    ++testnum;
    fd = open("test2/test1", O_RDONLY | O_NOFOLLOW);
    if (fd != -1)
    {
-      fprintf(stderr, "Test 4 failed - unexpected open() success.\n");
-      exit(1);
+      fprintf(stderr, "Test %d failed - unexpected open() success.\n",
+	      testnum);
+      exit(EXIT_FAILURE);
    }
    if (errno != ELOOP)
    { 
-      perror("Test 4 failed - wrong errno returned ");
-      exit(1);
+      fprintf(stderr, "Test %d failed - wrong errno returned: %s\n",
+	      testnum, strerror(errno));
+      exit(EXIT_FAILURE);
+   }
+   printf("Test %d passed\n", testnum);
+
+   ++testnum;
+   fd = open("test3", O_RDONLY | O_CREAT | O_EXCL);
+   if (fd != -1)
+   {
+      fprintf(stderr, "Test %d failed - unexpected open() success.\n",
+	      testnum);
+      exit(EXIT_FAILURE);
+   }
+   if (errno != EEXIST)
+   { 
+      fprintf(stderr, "Test %d failed - wrong errno returned: %s\n",
+	      testnum, strerror(errno));
+      exit(EXIT_FAILURE);
+   }
+   printf("Test %d passed\n", testnum);
+
+   ++testnum;
+   fd = open("test3", O_RDONLY | O_CREAT | O_EXCL | O_NOLINK);
+   if (fd != -1)
+   {
+      fprintf(stderr, "Test %d failed - unexpected open() success.\n",
+	      testnum);
+      exit(EXIT_FAILURE);
+   }
+   if (errno != EEXIST)
+   { 
+      fprintf(stderr, "Test %d failed - wrong errno returned: %s\n",
+	      testnum, strerror(errno));
+      exit(EXIT_FAILURE);
    }
-   printf("Test 4 passed\n");
+   printf("Test %d passed\n", testnum);
+
    test_success("test2/test1", O_RDONLY | O_NOLINK, "!<symlink>");
    test_success("dir/test2", O_RDONLY, "tstlink2");
    test_o_temporary();
-   return 0;
+   puts("PASS");
+   return EXIT_SUCCESS;
 } 
 
 static void test_success(const char * fn, int flags,
@@ -87,7 +138,7 @@ static void test_success(const char * fn
    {            
       sprintf(err_buf, "Test %d failed - unexpected open() failure ", testnum);
       perror(err_buf);
-      exit(1);
+      exit(EXIT_FAILURE);
    }
    bytes_read = read(fd, buffer, data_size);
    if (bytes_read == -1)
@@ -95,7 +146,7 @@ static void test_success(const char * fn
       sprintf(err_buf, "Test %d failed - unexpected read() failure ", testnum);
       perror(err_buf);
       close(fd);
-      exit(1);
+      exit(EXIT_FAILURE);
    }
    if (bytes_read != data_size)
    {
@@ -104,14 +155,14 @@ static void test_success(const char * fn
       buffer[bytes_read] = '\0';
       printf("buffer = %s\n", buffer);
       close(fd);
-      exit(1);
+      exit(EXIT_FAILURE);
    }
    if (strncmp(buffer, data, data_size))
    {
       fprintf(stderr, "Test %d failed - read() returned wrong file data.\n", 
               testnum);
       close(fd);
-      exit(1);
+      exit(EXIT_FAILURE);
    }
    close(fd);
    printf("Test %d passed\n", testnum);
@@ -128,7 +179,7 @@ int open_temp_test_file(int flags)
   {
     sprintf(err_buf, "Test %d failed - unexpected open() failure ", testnum);
     perror(err_buf);
-    exit(1);
+    exit(EXIT_FAILURE);
   }
   return fd;
 }
@@ -157,7 +208,7 @@ void close_temp_test_file(int fd, int sh
   if (exists ^ should_exist)
   {
     fprintf(stderr, "Test %d failed - %s\n", testnum, msg[should_exist]);
-    exit(1);
+    exit(EXIT_FAILURE);
   }
 }
 
--- /dev/null	2003-11-09 13:25:21.000000000 +0000
+++ tests/libc/posix/fcntl/test3	2003-11-09 10:52:12.000000000 +0000
@@ -0,0 +1,2 @@
+!<symlink>doesnotexist
+This is just a text to force symlink file to be 510 bytes long. Do not delete it nor spaces following it.                                                                                                                                                                                                                                                                                                                                                                                              
\ No newline at end of file