Mail Archives: djgpp-workers/2003/11/09/08:19:52
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
- Raw text -