Merge pull request #1388 from inolen/unlink_fixes

misc unlink / rmdir fixes

5 files changed, 147 insertions, 103 deletions

diff --git a/src/library.js b/src/library.js
index f6da38c1..5aa370a7 100644
--- a/src/library.js
+++ b/src/library.js
@@ -1867,42 +1867,42 @@ LibraryManager.library = {
   rmdir: function(path) {
     // int rmdir(const char *path);
     // http://pubs.opengroup.org/onlinepubs/000095399/functions/rmdir.html
-    path = FS.analyzePath(Pointer_stringify(path));
+    path = Pointer_stringify(path);
+    path = FS.analyzePath(path, true);
     if (!path.parentExists || !path.exists) {
       ___setErrNo(path.error);
       return -1;
-    } else if (!path.object.write || path.isRoot) {
+    } else if (!path.parentObject.write) {
       ___setErrNo(ERRNO_CODES.EACCES);
       return -1;
     } else if (!path.object.isFolder) {
       ___setErrNo(ERRNO_CODES.ENOTDIR);
       return -1;
+    } else if (path.isRoot || path.path == FS.currentPath) {
+      ___setErrNo(ERRNO_CODES.EBUSY);
+      return -1;
     } else {
       for (var i in path.object.contents) {
         ___setErrNo(ERRNO_CODES.ENOTEMPTY);
         return -1;
       }
-      if (path.path == FS.currentPath) {
-        ___setErrNo(ERRNO_CODES.EBUSY);
-        return -1;
-      } else {
-        delete path.parentObject.contents[path.name];
-        return 0;
-      }
+      delete path.parentObject.contents[path.name];
+      return 0;
     }
   },
   unlink__deps: ['$FS', '__setErrNo', '$ERRNO_CODES'],
   unlink: function(path) {
     // int unlink(const char *path);
     // http://pubs.opengroup.org/onlinepubs/000095399/functions/unlink.html
-    path = FS.analyzePath(Pointer_stringify(path));
+    path = Pointer_stringify(path);
+    path = FS.analyzePath(path, true);
     if (!path.parentExists || !path.exists) {
       ___setErrNo(path.error);
       return -1;
     } else if (path.object.isFolder) {
-      ___setErrNo(ERRNO_CODES.EISDIR);
+      ___setErrNo(ERRNO_CODES.EPERM);
       return -1;
-    } else if (!path.object.write) {
+    } else if (!path.parentObject.write) {
       ___setErrNo(ERRNO_CODES.EACCES);
       return -1;
     } else {
diff --git a/tests/runner.py b/tests/runner.py
index 9e04c929..339ce68c 100755
--- a/tests/runner.py
+++ b/tests/runner.py
@@ -7377,18 +7377,8 @@ def process(filename):
       self.do_run(src, expected)
 
     def test_unistd_unlink(self):
-      add_pre_run = '''
-def process(filename):
-  import tools.shared as shared
-  src = open(filename, 'r').read().replace(
-    '// {{PRE_RUN_ADDITIONS}}',
-    open(shared.path_from_root('tests', 'unistd', 'unlink.js'), 'r').read()
-  )
-  open(filename, 'w').write(src)
-'''
       src = open(path_from_root('tests', 'unistd', 'unlink.c'), 'r').read()
-      expected = open(path_from_root('tests', 'unistd', 'unlink.out'), 'r').read()
-      self.do_run(src, expected, post_build=add_pre_run)
+      self.do_run(src, 'success', force_c=True)
 
     def test_unistd_links(self):
       add_pre_run = '''
diff --git a/tests/unistd/unlink.c b/tests/unistd/unlink.c
index 3f7d84b6..87252da2 100644
--- a/tests/unistd/unlink.c
+++ b/tests/unistd/unlink.c
@@ -1,35 +1,138 @@
-#include <stdio.h>
+#include <assert.h>
 #include <errno.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
 #include <unistd.h>
+#include <sys/stat.h>
 
-int main() {
-  char* files[] = {"/device", "/file", "/file-forbidden", "/noexist"};
-  char* folders[] = {"/empty", "/empty-forbidden", "/full"};
-  int i;
-
-  for (i = 0; i < sizeof files / sizeof files[0]; i++) {
-    printf("access(%s) before: %d\n", files[i], access(files[i], F_OK));
-    rmdir(files[i]);
-    printf("errno: %d\n", errno);
-    errno = 0;
-    printf("access(%s) after rmdir: %d\n", files[i], access(files[i], F_OK));
-    unlink(files[i]);
-    printf("errno: %d\n", errno);
-    errno = 0;
-    printf("access(%s) after unlink: %d\n\n", files[i], access(files[i], F_OK));
-  }
-
-  for (i = 0; i < sizeof folders / sizeof folders[0]; i++) {
-    printf("access(%s) before: %d\n", folders[i], access(folders[i], F_OK));
-    unlink(folders[i]);
-    printf("errno: %d\n", errno);
-    errno = 0;
-    printf("access(%s) after unlink: %d\n", folders[i], access(folders[i], F_OK));
-    rmdir(folders[i]);
-    printf("errno: %d\n", errno);
-    errno = 0;
-    printf("access(%s) after rmdir: %d\n\n", folders[i], access(folders[i], F_OK));
-  }
-
-  return 0;
+static void create_file(const char *path, const char *buffer, int mode) {
+  int fd = open(path, O_WRONLY | O_CREAT | O_EXCL, mode);
+  assert(fd >= 0);
+
+  int err = write(fd, buffer, sizeof(char) * strlen(buffer));
+  assert(err ==  (sizeof(char) * strlen(buffer)));
+
+  close(fd);
+}
+
+void setup() {
+  create_file("file", "test", 0777);
+  create_file("file1", "test", 0777);
+  symlink("file1", "file1-link");
+  mkdir("dir-empty", 0777);
+  symlink("dir-empty", "dir-empty-link");
+  mkdir("dir-readonly", 0777);
+  create_file("dir-readonly/anotherfile", "test", 0777);
+  mkdir("dir-readonly/anotherdir", 0777);
+  chmod("dir-readonly", 0555);
+  mkdir("dir-full", 0777);
+  create_file("dir-full/anotherfile", "test", 0777);
+}
+
+void cleanup() {
+  unlink("file");
+  unlink("file1");
+  unlink("file1-link");
+  rmdir("dir-empty");
+  unlink("dir-empty-link");
+  chmod("dir-readonly", 0777);
+  unlink("dir-readonly/anotherfile");
+  rmdir("dir-readonly/anotherdir");
+  rmdir("dir-readonly");
+  unlink("dir-full/anotherfile");
+  rmdir("dir-full");
+}
+
+void test() {
+  int err;
+  char buffer[512];
+
+  //
+  // test unlink
+  //
+  err = unlink("noexist");
+  assert(err == -1);
+  assert(errno == ENOENT);
+
+  err = unlink("dir-readonly");
+  assert(err == -1);
+#ifdef __linux__
+  assert(errno == EISDIR);
+#else
+  assert(errno == EPERM);
+#endif
+
+  err = unlink("dir-readonly/anotherfile");
+  assert(err == -1);
+  assert(errno == EACCES);
+
+  // try unlinking the symlink first to make sure
+  // we don't follow the link
+  err = unlink("file1-link");
+  assert(!err);
+  err = access("file1", F_OK);
+  assert(!err);
+  err = access("file1-link", F_OK);
+  assert(err == -1);
+
+  err = unlink("file");
+  assert(!err);
+  err = access("file", F_OK);
+  assert(err == -1);
+
+  //
+  // test rmdir
+  //
+  err = rmdir("noexist");
+  assert(err == -1);
+  assert(errno == ENOENT);
+
+  err = rmdir("file1");
+  assert(err == -1);
+  assert(errno == ENOTDIR);
+
+  err = rmdir("dir-readonly/anotherdir");
+  assert(err == -1);
+  assert(errno == EACCES);
+
+  err = rmdir("dir-full");
+  assert(err == -1);
+  assert(errno == ENOTEMPTY);
+
+  // test removing the cwd / root. The result isn't specified by
+  // POSIX, but Linux seems to set EBUSY in both cases.
+#ifndef __APPLE__
+  getcwd(buffer, sizeof(buffer));
+  err = rmdir(buffer);
+  assert(err == -1);
+  assert(errno == EBUSY);
+#endif
+  err = rmdir("/");
+  assert(err == -1);
+#ifdef __APPLE__
+  assert(errno == EISDIR);
+#else
+  assert(errno == EBUSY);
+#endif
+
+  err = rmdir("dir-empty-link");
+  assert(err == -1);
+  assert(errno == ENOTDIR);
+
+  err = rmdir("dir-empty");
+  assert(!err);
+  err = access("dir-empty", F_OK);
+  assert(err == -1);
+
+  puts("success");
 }
+
+int main() {
+  atexit(cleanup);
+  signal(SIGABRT, cleanup);
+  setup();
+  test();
+  return EXIT_SUCCESS;
+}
\ No newline at end of file
diff --git a/tests/unistd/unlink.js b/tests/unistd/unlink.js
deleted file mode 100644
index c2366080..00000000
--- a/tests/unistd/unlink.js
+++ /dev/null
@@ -1,7 +0,0 @@
-FS.createDevice('/', 'device', function() {}, function() {});
-FS.createDataFile('/', 'file', 'test', true, true);
-FS.createDataFile('/', 'file-forbidden', 'test', true, false);
-FS.createFolder('/', 'empty', true, true);
-FS.createFolder('/', 'empty-forbidden', true, false);
-FS.createFolder('/', 'full', true, true);
-FS.createFolder('/full', 'junk', true, true);
diff --git a/tests/unistd/unlink.out b/tests/unistd/unlink.out
deleted file mode 100644
index f7a894cb..00000000
--- a/tests/unistd/unlink.out
+++ /dev/null
@@ -1,42 +0,0 @@
-access(/device) before: 0
-errno: 20
-access(/device) after rmdir: 0
-errno: 0
-access(/device) after unlink: -1
-
-access(/file) before: 0
-errno: 20
-access(/file) after rmdir: 0
-errno: 0
-access(/file) after unlink: -1
-
-access(/file-forbidden) before: 0
-errno: 13
-access(/file-forbidden) after rmdir: 0
-errno: 13
-access(/file-forbidden) after unlink: 0
-
-access(/noexist) before: -1
-errno: 2
-access(/noexist) after rmdir: -1
-errno: 2
-access(/noexist) after unlink: -1
-
-access(/empty) before: 0
-errno: 21
-access(/empty) after unlink: 0
-errno: 0
-access(/empty) after rmdir: -1
-
-access(/empty-forbidden) before: 0
-errno: 21
-access(/empty-forbidden) after unlink: 0
-errno: 13
-access(/empty-forbidden) after rmdir: 0
-
-access(/full) before: 0
-errno: 21
-access(/full) after unlink: 0
-errno: 90
-access(/full) after rmdir: 0
-