From bda0c935c26867f3a613723fd3bc7a84aef110ba Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Tue, 22 Jan 2019 07:03:44 +0100 Subject: [PATCH] Add unlink() syscall and /bin/rm. This patch adds most of the plumbing for working file deletion in Ext2FS. Directory entries are removed and inode link counts updated. We don't yet update the inode or block bitmaps, I will do that separately. --- Kernel/Process.cpp | 10 ++++ Kernel/Process.h | 1 + Kernel/Syscall.cpp | 2 + Kernel/Syscall.h | 1 + Kernel/sync.sh | 1 + LibC/unistd.cpp | 5 +- Userland/.gitignore | 1 + Userland/Makefile | 9 +++- Userland/rm.cpp | 18 +++++++ VirtualFileSystem/Ext2FileSystem.cpp | 58 +++++++++++++++++++++++ VirtualFileSystem/Ext2FileSystem.h | 3 ++ VirtualFileSystem/FileSystem.h | 1 + VirtualFileSystem/SyntheticFileSystem.cpp | 8 ++++ VirtualFileSystem/SyntheticFileSystem.h | 1 + VirtualFileSystem/VirtualFileSystem.cpp | 26 ++++++++++ VirtualFileSystem/VirtualFileSystem.h | 3 +- 16 files changed, 142 insertions(+), 6 deletions(-) create mode 100644 Userland/rm.cpp diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index b7b2461e11..409755b837 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -2009,3 +2009,13 @@ Inode* Process::cwd_inode() m_cwd = VFS::the().root_inode(); return m_cwd.ptr(); } + +int Process::sys$unlink(const char* pathname) +{ + if (!validate_read_str(pathname)) + return -EFAULT; + int error; + if (!VFS::the().unlink(String(pathname), *cwd_inode(), error)) + return error; + return 0; +} diff --git a/Kernel/Process.h b/Kernel/Process.h index dfb46a998d..7a8dad052e 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -191,6 +191,7 @@ public: int sys$mkdir(const char* pathname, mode_t mode); Unix::clock_t sys$times(Unix::tms*); int sys$utime(const char* pathname, const struct Unix::utimbuf*); + int sys$unlink(const char* pathname); int gui$create_window(const GUI_WindowParameters*); int gui$destroy_window(int window_id); diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index 3e8d5ba9a6..841ceb0c71 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -191,6 +191,8 @@ static dword handle(RegisterDump& regs, dword function, dword arg1, dword arg2, return current->sys$utime((const char*)arg1, (const Unix::utimbuf*)arg2); case Syscall::SC_sync: return sync(); + case Syscall::SC_unlink: + return current->sys$unlink((const char*)arg1); case Syscall::SC_gui_create_window: return current->gui$create_window((const GUI_WindowParameters*)arg1); case Syscall::SC_gui_destroy_window: diff --git a/Kernel/Syscall.h b/Kernel/Syscall.h index 51387ae88a..36c347c750 100644 --- a/Kernel/Syscall.h +++ b/Kernel/Syscall.h @@ -74,6 +74,7 @@ __ENUMERATE_SYSCALL(select) \ __ENUMERATE_SYSCALL(gui_get_window_parameters) \ __ENUMERATE_SYSCALL(gui_set_window_parameters) \ + __ENUMERATE_SYSCALL(unlink) \ struct fd_set; diff --git a/Kernel/sync.sh b/Kernel/sync.sh index 808f625a86..6b83e1549a 100755 --- a/Kernel/sync.sh +++ b/Kernel/sync.sh @@ -42,6 +42,7 @@ cp -v ../Userland/mkdir mnt/bin/mkdir cp -v ../Userland/touch mnt/bin/touch cp -v ../Userland/sync mnt/bin/sync cp -v ../Userland/more mnt/bin/more +cp -v ../Userland/rm mnt/bin/rm cp -v ../Userland/guitest mnt/bin/guitest cp -v ../Userland/guitest2 mnt/bin/guitest2 cp -v ../Userland/sysctl mnt/bin/sysctl diff --git a/LibC/unistd.cpp b/LibC/unistd.cpp index 0e10e7811b..d685d7db0c 100644 --- a/LibC/unistd.cpp +++ b/LibC/unistd.cpp @@ -199,9 +199,10 @@ int link(const char*, const char*) assert(false); } -int unlink(const char*) +int unlink(const char* pathname) { - assert(false); + int rc = syscall(SC_unlink, pathname); + __RETURN_WITH_ERRNO(rc, rc, -1); } int isatty(int fd) diff --git a/Userland/.gitignore b/Userland/.gitignore index daf846e52b..425fa3f830 100644 --- a/Userland/.gitignore +++ b/Userland/.gitignore @@ -24,3 +24,4 @@ more guitest guitest2 sysctl +rm diff --git a/Userland/Makefile b/Userland/Makefile index f0240668ed..a774dd3e40 100644 --- a/Userland/Makefile +++ b/Userland/Makefile @@ -21,7 +21,8 @@ OBJS = \ more.o \ guitest.o \ guitest2.o \ - sysctl.o + sysctl.o \ + rm.o APPS = \ id \ @@ -47,7 +48,8 @@ APPS = \ more \ guitest \ guitest2 \ - sysctl + sysctl \ + rm ARCH_FLAGS = STANDARD_FLAGS = -std=c++17 -nostdinc++ -nostdlib -nostdinc @@ -139,6 +141,9 @@ guitest2: guitest2.o sysctl: sysctl.o $(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a +rm: rm.o + $(LD) -o $@ $(LDFLAGS) $< ../LibC/LibC.a + .cpp.o: @echo "CXX $<"; $(CXX) $(CXXFLAGS) -o $@ -c $< diff --git a/Userland/rm.cpp b/Userland/rm.cpp new file mode 100644 index 0000000000..606d92d7d9 --- /dev/null +++ b/Userland/rm.cpp @@ -0,0 +1,18 @@ +#include +#include +#include + +int main(int argc, char** argv) +{ + if (argc != 2) { + fprintf(stderr, "usage: rm \n"); + return 1; + } + int rc = unlink(argv[1]); + if (rc < 0) { + perror("unlink"); + return 1; + } + return 0; +} + diff --git a/VirtualFileSystem/Ext2FileSystem.cpp b/VirtualFileSystem/Ext2FileSystem.cpp index 71b20b8dd3..4f4759ba51 100644 --- a/VirtualFileSystem/Ext2FileSystem.cpp +++ b/VirtualFileSystem/Ext2FileSystem.cpp @@ -219,6 +219,11 @@ Ext2FSInode::Ext2FSInode(Ext2FS& fs, unsigned index, const ext2_inode& raw_inode Ext2FSInode::~Ext2FSInode() { + if (m_raw_inode.i_links_count == 0) { + dbgprintf("Ext2FS: inode %u has no more links, time to delete!\n", index()); + // FIXME: Implement! + ASSERT_NOT_REACHED(); + } } InodeMetadata Ext2FSInode::metadata() const @@ -443,6 +448,50 @@ bool Ext2FSInode::add_child(InodeIdentifier child_id, const String& name, byte f return success; } +bool Ext2FSInode::remove_child(const String& name, int& error) +{ + ASSERT(is_directory()); + + unsigned child_inode_index; + { + LOCKER(m_lock); + auto it = m_lookup_cache.find(name); + if (it == m_lookup_cache.end()) { + error = -ENOENT; + return false; + } + child_inode_index = (*it).value; + } + InodeIdentifier child_id { fsid(), child_inode_index }; + +//#ifdef EXT2_DEBUG + dbgprintf("Ext2FS: Removing '%s' in directory %u\n", name.characters(), index()); +//#endif + + Vector entries; + traverse_as_directory([&] (auto& entry) { + if (entry.inode != child_id) + entries.append(entry); + return true; + }); + + bool success = fs().write_directory_inode(index(), move(entries)); + if (!success) { + // FIXME: Plumb error from write_directory_inode(). + error = -EIO; + return false; + } + + { + LOCKER(m_lock); + m_lookup_cache.remove(name); + } + + auto child_inode = fs().get_inode(child_id); + child_inode->decrement_link_count(); + return success; +} + bool Ext2FS::write_directory_inode(unsigned directoryInode, Vector&& entries) { dbgprintf("Ext2FS: New directory inode %u contents to write:\n", directoryInode); @@ -1050,7 +1099,16 @@ int Ext2FSInode::decrement_link_count() { if (fs().is_readonly()) return -EROFS; + ASSERT(m_raw_inode.i_links_count); --m_raw_inode.i_links_count; + if (m_raw_inode.i_links_count == 0) + fs().uncache_inode(index()); set_metadata_dirty(true); return 0; } + +void Ext2FS::uncache_inode(InodeIndex index) +{ + LOCKER(m_inode_cache_lock); + m_inode_cache.remove(index); +} diff --git a/VirtualFileSystem/Ext2FileSystem.h b/VirtualFileSystem/Ext2FileSystem.h index 6ac87333bb..74d57ecee7 100644 --- a/VirtualFileSystem/Ext2FileSystem.h +++ b/VirtualFileSystem/Ext2FileSystem.h @@ -34,6 +34,7 @@ private: virtual void flush_metadata() override; virtual bool write(const ByteBuffer&) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; + virtual bool remove_child(const String& name, int& error) override; virtual RetainPtr parent() const override; virtual int set_atime(Unix::time_t) override; virtual int set_ctime(Unix::time_t) override; @@ -105,6 +106,8 @@ private: bool set_inode_allocation_state(unsigned inode, bool); bool set_block_allocation_state(GroupIndex, BlockIndex, bool); + void uncache_inode(InodeIndex); + unsigned m_blockGroupCount { 0 }; mutable ByteBuffer m_cached_super_block; diff --git a/VirtualFileSystem/FileSystem.h b/VirtualFileSystem/FileSystem.h index 9aff339819..110fd313e7 100644 --- a/VirtualFileSystem/FileSystem.h +++ b/VirtualFileSystem/FileSystem.h @@ -85,6 +85,7 @@ public: virtual String reverse_lookup(InodeIdentifier) = 0; virtual bool write(const ByteBuffer&) = 0; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) = 0; + virtual bool remove_child(const String& name, int& error) = 0; virtual RetainPtr parent() const = 0; bool is_metadata_dirty() const { return m_metadata_dirty; } diff --git a/VirtualFileSystem/SyntheticFileSystem.cpp b/VirtualFileSystem/SyntheticFileSystem.cpp index 579f02525a..eec8f6ec24 100644 --- a/VirtualFileSystem/SyntheticFileSystem.cpp +++ b/VirtualFileSystem/SyntheticFileSystem.cpp @@ -290,6 +290,14 @@ bool SynthFSInode::add_child(InodeIdentifier child_id, const String& name, byte return false; } +bool SynthFSInode::remove_child(const String& name, int& error) +{ + (void) name; + (void) error; + ASSERT_NOT_REACHED(); + return false; +} + SynthFSInodeCustomData::~SynthFSInodeCustomData() { } diff --git a/VirtualFileSystem/SyntheticFileSystem.h b/VirtualFileSystem/SyntheticFileSystem.h index ec440cd2e1..ee3744e9df 100644 --- a/VirtualFileSystem/SyntheticFileSystem.h +++ b/VirtualFileSystem/SyntheticFileSystem.h @@ -62,6 +62,7 @@ private: virtual void flush_metadata() override; virtual bool write(const ByteBuffer&) override; virtual bool add_child(InodeIdentifier child_id, const String& name, byte file_type, int& error) override; + virtual bool remove_child(const String& name, int& error) override; virtual RetainPtr parent() const override; SynthFS& fs(); diff --git a/VirtualFileSystem/VirtualFileSystem.cpp b/VirtualFileSystem/VirtualFileSystem.cpp index c7d7d80df8..e5663068b0 100644 --- a/VirtualFileSystem/VirtualFileSystem.cpp +++ b/VirtualFileSystem/VirtualFileSystem.cpp @@ -222,6 +222,32 @@ bool VFS::mkdir(const String& path, mode_t mode, InodeIdentifier base, int& erro return false; } +bool VFS::unlink(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; + auto inode_id = resolve_path(path, base.identifier(), error, 0, &parent_dir); + if (!inode_id.is_valid()) { + error = -ENOENT; + return false; + } + + auto parent_inode = get_inode(parent_dir); + // 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; + return true; +} + InodeIdentifier VFS::resolve_symbolic_link(InodeIdentifier base, Inode& symlink_inode, int& error) { auto symlink_contents = symlink_inode.read_entire(); diff --git a/VirtualFileSystem/VirtualFileSystem.h b/VirtualFileSystem/VirtualFileSystem.h index 543d9bd4d4..9bd8274e38 100644 --- a/VirtualFileSystem/VirtualFileSystem.h +++ b/VirtualFileSystem/VirtualFileSystem.h @@ -67,8 +67,7 @@ public: RetainPtr open(const String& path, int& error, int options, mode_t mode, InodeIdentifier base = InodeIdentifier()); RetainPtr create(const String& path, int& error, int options, mode_t mode, InodeIdentifier base); bool mkdir(const String& path, mode_t mode, InodeIdentifier base, int& error); - - bool touch(const String&path); + bool unlink(const String& path, Inode& base, int& error); void register_character_device(CharacterDevice&);