Kernel: Add link() syscall to create hard links.

This accidentally grew into a little bit of VFS cleanup as well.

Also add a simple /bin/ln implementation to exercise it.

Kernel/Ext2FileSystem.cpp

@@ -637,12 +637,14 @@ bool Ext2FSInode::add_child(InodeIdentifier child_id, const String& name, byte f
         return false;
     }
 
+    auto child_inode = fs().get_inode(child_id);
+    if (child_inode)
+        child_inode->increment_link_count();
+
     entries.append({ name.characters(), name.length(), child_id, file_type });
     bool success = fs().write_directory_inode(index(), move(entries));
-    if (success) {
+    if (success)
-        LOCKER(m_lock);
         m_lookup_cache.set(name, child_id.index());
-    }
     return success;
 }
 
@@ -672,8 +674,8 @@ bool Ext2FSInode::remove_child(const String& name, int& error)
 
     Vector<FS::DirectoryEntry> entries;
     traverse_as_directory([&] (auto& entry) {
-        if (entry.inode != child_id)
+        if (strcmp(entry.name, name.characters()) != 0)
-        entries.append(entry);
+            entries.append(entry);
         return true;
     });
 
@@ -1336,6 +1338,7 @@ int Ext2FSInode::decrement_link_count()
 void Ext2FS::uncache_inode(InodeIndex index)
 {
     LOCKER(m_lock);
+    m_inode_cache.remove(index);
 }
 
 size_t Ext2FSInode::directory_entry_count() const

Kernel/ProcFS.cpp

@@ -1038,19 +1038,13 @@ ssize_t ProcFSInode::write_bytes(off_t offset, size_t size, const byte* buffer,
 
 bool ProcFSInode::add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error)
 {
-    (void) child_id;
+    error = -EPERM;
-    (void) name;
-    (void) file_type;
-    (void) error;
-    ASSERT_NOT_REACHED();
     return false;
 }
 
 bool ProcFSInode::remove_child(const String& name, int& error)
 {
-    (void) name;
+    error = -EPERM;
-    (void) error;
-    ASSERT_NOT_REACHED();
     return false;
 }
 

Kernel/Process.cpp

@@ -1143,9 +1143,11 @@ int Process::sys$utime(const char* pathname, const utimbuf* buf)
         mtime = now;
         atime = now;
     }
-    inode.set_atime(atime);
+    error = inode.set_atime(atime);
-    inode.set_mtime(mtime);
+    if (error)
-    return 0;
+        return error;
+    error = inode.set_mtime(mtime);
+    return error;
 }
 
 int Process::sys$access(const char* pathname, int mode)
@@ -2099,6 +2101,18 @@ Inode* Process::cwd_inode()
     return m_cwd.ptr();
 }
 
+int Process::sys$link(const char* old_path, const char* new_path)
+{
+    if (!validate_read_str(old_path))
+        return -EFAULT;
+    if (!validate_read_str(new_path))
+        return -EFAULT;
+    int error;
+    if (!VFS::the().link(String(old_path), String(new_path), *cwd_inode(), error))
+        return error;
+    return 0;
+}
+
 int Process::sys$unlink(const char* pathname)
 {
     if (!validate_read_str(pathname))

Kernel/Process.h

@@ -210,6 +210,7 @@ public:
     int sys$mkdir(const char* pathname, mode_t mode);
     clock_t sys$times(tms*);
     int sys$utime(const char* pathname, const struct utimbuf*);
+    int sys$link(const char* old_path, const char* new_path);
     int sys$unlink(const char* pathname);
     int sys$rmdir(const char* pathname);
     int sys$read_tsc(dword* lsw, dword* msw);

Kernel/Syscall.cpp

@@ -193,6 +193,8 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2,
         return current->sys$utime((const char*)arg1, (const utimbuf*)arg2);
     case Syscall::SC_sync:
         return sync();
+    case Syscall::SC_link:
+        return current->sys$link((const char*)arg1, (const char*)arg2);
     case Syscall::SC_unlink:
         return current->sys$unlink((const char*)arg1);
     case Syscall::SC_read_tsc:

Kernel/Syscall.h

@@ -82,6 +82,7 @@
     __ENUMERATE_SYSCALL(create_shared_buffer) \
     __ENUMERATE_SYSCALL(get_shared_buffer) \
     __ENUMERATE_SYSCALL(release_shared_buffer) \
+    __ENUMERATE_SYSCALL(link) \
 
 
 #ifdef SERENITY

Kernel/VirtualFileSystem.cpp

@@ -168,28 +168,22 @@ RetainPtr<FileDescriptor> VFS::create(const String& path, int& error, int option
         mode |= 0100000;
     }
 
-    // FIXME: This won't work nicely across mount boundaries.
+    RetainPtr<Inode> parent_inode;
-    FileSystemPath p(path);
+    auto existing_file = resolve_path_to_inode(path, base, error, &parent_inode);
-    if (!p.is_valid()) {
+    if (existing_file) {
-        error = -EINVAL;
-        return nullptr;
-    }
-
-    InodeIdentifier parent_dir;
-    auto existing_file = resolve_path(path, base.identifier(), error, 0, &parent_dir);
-    if (existing_file.is_valid()) {
         error = -EEXIST;
         return nullptr;
     }
-    if (!parent_dir.is_valid()) {
+    if (!parent_inode) {
         error = -ENOENT;
         return nullptr;
     }
     if (error != -ENOENT) {
         return nullptr;
     }
-    dbgprintf("VFS::create_file: '%s' in %u:%u\n", p.basename().characters(), parent_dir.fsid(), parent_dir.index());
+    FileSystemPath p(path);
-    auto new_file = parent_dir.fs()->create_inode(parent_dir, p.basename(), mode, 0, error);
+    dbgprintf("VFS::create_file: '%s' in %u:%u\n", p.basename().characters(), parent_inode->fsid(), parent_inode->index());
+    auto new_file = parent_inode->fs().create_inode(parent_inode->identifier(), p.basename(), mode, 0, error);
     if (!new_file)
         return nullptr;
 
@@ -200,28 +194,23 @@ RetainPtr<FileDescriptor> VFS::create(const String& path, int& error, int option
 bool VFS::mkdir(const String& path, mode_t mode, Inode& base, int& error)
 {
     error = -EWHYTHO;
-    // FIXME: This won't work nicely across mount boundaries.
-    FileSystemPath p(path);
-    if (!p.is_valid()) {
-        error = -EINVAL;
-        return false;
-    }
 
-    InodeIdentifier parent_dir;
+    RetainPtr<Inode> parent_inode;
-    auto existing_dir = resolve_path(path, base.identifier(), error, 0, &parent_dir);
+    auto existing_dir = resolve_path_to_inode(path, base, error, &parent_inode);
-    if (existing_dir.is_valid()) {
+    if (existing_dir) {
         error = -EEXIST;
         return false;
     }
-    if (!parent_dir.is_valid()) {
+    if (!parent_inode) {
         error = -ENOENT;
         return false;
     }
     if (error != -ENOENT) {
         return false;
     }
-    dbgprintf("VFS::mkdir: '%s' in %u:%u\n", p.basename().characters(), parent_dir.fsid(), parent_dir.index());
+    FileSystemPath p(path);
-    auto new_dir = parent_dir.fs()->create_directory(parent_dir, p.basename(), mode, error);
+    dbgprintf("VFS::mkdir: '%s' in %u:%u\n", p.basename().characters(), parent_inode->fsid(), parent_inode->index());
+    auto new_dir = parent_inode->fs().create_directory(parent_inode->identifier(), p.basename(), mode, error);
     if (new_dir) {
         error = 0;
         return true;
@@ -232,60 +221,85 @@ bool VFS::mkdir(const String& path, mode_t mode, Inode& base, int& error)
 bool VFS::chmod(const String& path, mode_t mode, Inode& base, int& error)
 {
     error = -EWHYTHO;
-    // FIXME: This won't work nicely across mount boundaries.
+    auto inode = resolve_path_to_inode(path, base, error);
-    FileSystemPath p(path);
+    if (!inode)
-    if (!p.is_valid()) {
-        error = -EINVAL;
         return false;
-    }
-
-    InodeIdentifier parent_dir;
-    auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir);
-    if (!inode_id.is_valid()) {
-        error = -ENOENT;
-        return false;
-    }
-
-    auto inode = get_inode(inode_id);
 
     // FIXME: Permission checks.
 
     // Only change the permission bits.
     mode = (inode->mode() & ~04777) | (mode & 04777);
 
-    kprintf("VFS::chmod(): %u:%u mode %o\n", inode_id.fsid(), inode_id.index(), mode);
+    kprintf("VFS::chmod(): %u:%u mode %o\n", inode->fsid(), inode->index(), mode);
     if (!inode->chmod(mode, error))
         return false;
     error = 0;
     return true;
 }
 
+RetainPtr<Inode> VFS::resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr<Inode>* parent_inode)
+{
+    // FIXME: This won't work nicely across mount boundaries.
+    FileSystemPath p(path);
+    if (!p.is_valid()) {
+        error = -EINVAL;
+        return nullptr;
+    }
+    InodeIdentifier parent_id;
+    auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_id);
+    if (parent_inode && parent_id.is_valid())
+        *parent_inode = get_inode(parent_id);
+    if (!inode_id.is_valid()) {
+        error = -ENOENT;
+        return nullptr;
+    }
+    return get_inode(inode_id);
+}
+
+bool VFS::link(const String& old_path, const String& new_path, Inode& base, int& error)
+{
+    auto old_inode = resolve_path_to_inode(old_path, base, error);
+    if (!old_inode)
+        return false;
+
+    RetainPtr<Inode> parent_inode;
+    auto new_inode = resolve_path_to_inode(new_path, base, error, &parent_inode);
+    if (new_inode) {
+        error = -EEXIST;
+        return false;
+    }
+    if (!parent_inode) {
+        error = -ENOENT;
+        return false;
+    }
+    if (parent_inode->fsid() != old_inode->fsid()) {
+        error = -EXDEV;
+        return false;
+    }
+    if (parent_inode->fs().is_readonly()) {
+        error = -EROFS;
+        return false;
+    }
+
+    if (!parent_inode->add_child(old_inode->identifier(), FileSystemPath(old_path).basename(), 0, error))
+        return false;
+    error = 0;
+    return true;
+}
+
 bool VFS::unlink(const String& path, Inode& base, int& error)
 {
-    error = -EWHYTHO;
+    RetainPtr<Inode> parent_inode;
-    // FIXME: This won't work nicely across mount boundaries.
+    auto inode = resolve_path_to_inode(path, base, error, &parent_inode);
-    FileSystemPath p(path);
+    if (!inode)
-    if (!p.is_valid()) {
-        error = -EINVAL;
         return false;
-    }
 
-    InodeIdentifier parent_dir;
-    auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir);
-    if (!inode_id.is_valid()) {
-        error = -ENOENT;
-        return false;
-    }
-
-    auto inode = get_inode(inode_id);
     if (inode->is_directory()) {
         error = -EISDIR;
         return false;
     }
 
-    auto parent_inode = get_inode(parent_dir);
+    if (!parent_inode->remove_child(FileSystemPath(path).basename(), error))
-    // FIXME: The reverse_lookup here can definitely be avoided.
-    if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode_id), error))
         return false;
 
     error = 0;
@@ -295,21 +309,13 @@ bool VFS::unlink(const String& path, Inode& base, int& error)
 bool VFS::rmdir(const String& path, Inode& base, int& error)
 {
     error = -EWHYTHO;
-    // FIXME: This won't work nicely across mount boundaries.
-    FileSystemPath p(path);
-    if (!p.is_valid()) {
-        error = -EINVAL;
-        return false;
-    }
 
-    InodeIdentifier parent_dir;
+    RetainPtr<Inode> parent_inode;
-    auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir);
+    auto inode = resolve_path_to_inode(path, base, error, &parent_inode);
-    if (!inode_id.is_valid()) {
+    if (!inode)
-        error = -ENOENT;
         return false;
-    }
 
-    if (inode_id.fs()->is_readonly()) {
+    if (inode->fs().is_readonly()) {
         error = -EROFS;
         return false;
     }
@@ -317,7 +323,6 @@ bool VFS::rmdir(const String& path, Inode& base, int& error)
     // FIXME: We should return EINVAL if the last component of the path is "."
     // FIXME: We should return ENOTEMPTY if the last component of the path is ".."
 
-    auto inode = get_inode(inode_id);
     if (!inode->is_directory()) {
         error = -ENOTDIR;
         return false;
@@ -328,10 +333,7 @@ bool VFS::rmdir(const String& path, Inode& base, int& error)
         return false;
     }
 
-    auto parent_inode = get_inode(parent_dir);
+    dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode->fsid(), inode->index(), parent_inode->fsid(), parent_inode->index());
-    ASSERT(parent_inode);
-
-    dbgprintf("VFS::rmdir: Removing inode %u:%u from parent %u:%u\n", inode_id.fsid(), inode_id.index(), parent_dir.fsid(), parent_dir.index());
 
     // To do:
     // - Remove '.' in target (--child.link_count)
@@ -344,7 +346,7 @@ bool VFS::rmdir(const String& path, Inode& base, int& error)
         return false;
 
     // FIXME: The reverse_lookup here can definitely be avoided.
-    if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode_id), error))
+    if (!parent_inode->remove_child(parent_inode->reverse_lookup(inode->identifier()), error))
         return false;
 
     error = 0;

Kernel/VirtualFileSystem.h

@@ -66,6 +66,7 @@ public:
     RetainPtr<FileDescriptor> open(const String& path, int& error, int options, mode_t mode, Inode& base);
     RetainPtr<FileDescriptor> create(const String& path, int& error, int options, mode_t mode, Inode& base);
     bool mkdir(const String& path, mode_t mode, Inode& base, int& error);
+    bool link(const String& old_path, const String& new_path, Inode& base, int& error);
     bool unlink(const String& path, Inode& base, int& error);
     bool rmdir(const String& path, Inode& base, int& error);
     bool chmod(const String& path, mode_t, Inode& base, int& error);
@@ -96,6 +97,7 @@ private:
 
     void traverse_directory_inode(Inode&, Function<bool(const FS::DirectoryEntry&)>);
     InodeIdentifier resolve_path(const String& path, InodeIdentifier base, int& error, int options = 0, InodeIdentifier* parent_id = nullptr);
+    RetainPtr<Inode> resolve_path_to_inode(const String& path, Inode& base, int& error, RetainPtr<Inode>* parent_id = nullptr);
     InodeIdentifier resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error);
 
     Mount* find_mount_for_host(InodeIdentifier);

Kernel/sync.sh

@@ -63,6 +63,7 @@ cp -v ../Userland/pape mnt/bin/pape
 cp -v ../Userland/dmesg mnt/bin/dmesg
 cp -v ../Userland/chmod mnt/bin/chmod
 cp -v ../Userland/top mnt/bin/top
+cp -v ../Userland/ln mnt/bin/ln
 cp -v ../Applications/Terminal/Terminal mnt/bin/Terminal
 cp -v ../Applications/FontEditor/FontEditor mnt/bin/FontEditor
 cp -v ../Applications/Launcher/Launcher mnt/bin/Launcher

LibC/unistd.cpp

@@ -229,9 +229,10 @@ off_t lseek(int fd, off_t offset, int whence)
     __RETURN_WITH_ERRNO(rc, rc, -1);
 }
 
-int link(const char*, const char*)
+int link(const char* old_path, const char* new_path)
 {
-    assert(false);
+    int rc = syscall(SC_link, old_path, new_path);
+    __RETURN_WITH_ERRNO(rc, rc, -1);
 }
 
 int unlink(const char* pathname)

Userland/.gitignore

@@ -31,3 +31,4 @@ dmesg
 top
 chmod
 pape
+ln

Userland/Makefile

@@ -27,6 +27,7 @@ OBJS = \
        dmesg.o \
        chmod.o \
        top.o \
+       ln.o \
        rm.o
 
 APPS = \
@@ -59,6 +60,7 @@ APPS = \
        dmesg \
        chmod \
        top \
+       ln \
        rm
 
 ARCH_FLAGS =
@@ -169,6 +171,9 @@ chmod: chmod.o
 top: top.o
 	$(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a
 
+ln: ln.o
+	$(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a
+
 .cpp.o:
 	@echo "CXX $<"; $(CXX) $(CXXFLAGS) -o $@ -c $<
 

Userland/ln.cpp

@@ -0,0 +1,18 @@
+#include <stdio.h>
+#include <errno.h>
+#include <unistd.h>
+
+int main(int argc, char** argv)
+{
+    if (argc != 3) {
+        fprintf(stderr, "usage: ln <old-path> <new-path>\n");
+        return 1;
+    }
+    int rc = link(argv[1], argv[2]);
+    if (rc < 0) {
+        perror("link");
+        return 1;
+    }
+    return 0;
+}
+