From 8d80841e9cf747deaa224cc7255a6bde33874ed9 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Mon, 15 Jan 2024 17:24:00 +0000 Subject: [PATCH] LibFileSystem+Everywhere: Return ByteString from read_link() --- Tests/Kernel/TestKernelFilePermissions.cpp | 4 +--- Tests/LibC/TestIo.cpp | 2 +- Tests/LibC/TestLibCMkTemp.cpp | 12 ++++------ Tests/LibELF/test-elf.cpp | 15 +++++-------- .../FileManager/PropertiesWindow.cpp | 6 +++-- .../Libraries/LibFileSystem/FileSystem.cpp | 22 +++++++++---------- Userland/Libraries/LibFileSystem/FileSystem.h | 4 ++-- .../Libraries/LibGUI/FileIconProvider.cpp | 2 +- Userland/Libraries/LibGUI/FileSystemModel.cpp | 2 +- Userland/Services/LaunchServer/Launcher.cpp | 2 +- Userland/Shell/Builtin.cpp | 6 ++--- 11 files changed, 34 insertions(+), 43 deletions(-) diff --git a/Tests/Kernel/TestKernelFilePermissions.cpp b/Tests/Kernel/TestKernelFilePermissions.cpp index 70da9deb497..04955c7204a 100644 --- a/Tests/Kernel/TestKernelFilePermissions.cpp +++ b/Tests/Kernel/TestKernelFilePermissions.cpp @@ -81,9 +81,7 @@ TEST_CASE(test_change_file_location) ftruncate(fd, 0); EXPECT(fchmod(fd, 06755) != -1); - auto suid_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - - auto suid_path = suid_path_string.to_byte_string(); + auto suid_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(suid_path.characters()); auto new_path = ByteString::formatted("{}.renamed", suid_path); diff --git a/Tests/LibC/TestIo.cpp b/Tests/LibC/TestIo.cpp index 2ab4398da67..4f9c2794444 100644 --- a/Tests/LibC/TestIo.cpp +++ b/Tests/LibC/TestIo.cpp @@ -218,7 +218,7 @@ TEST_CASE(unlink_symlink) } auto target = TRY_OR_FAIL(FileSystem::read_link("/tmp/linky"sv)); - EXPECT_EQ(target.bytes_as_string_view(), "/proc/2/foo"sv); + EXPECT_EQ(target, "/proc/2/foo"sv); rc = unlink("/tmp/linky"); EXPECT(rc >= 0); diff --git a/Tests/LibC/TestLibCMkTemp.cpp b/Tests/LibC/TestLibCMkTemp.cpp index ccbd2cf38cf..f5362b1fc01 100644 --- a/Tests/LibC/TestLibCMkTemp.cpp +++ b/Tests/LibC/TestLibCMkTemp.cpp @@ -86,8 +86,7 @@ TEST_CASE(test_mkstemp_unique_filename) auto fd = mkstemp(path); EXPECT_NE(fd, -1); - auto temp_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto temp_path = temp_path_string.to_byte_string(); + auto temp_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(temp_path.characters()); close(fd); @@ -105,8 +104,7 @@ TEST_CASE(test_mkstemp_unique_filename) auto fd = mkstemp(path); EXPECT(fd != -1); - auto path2_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto path2 = path2_string.to_byte_string(); + auto path2 = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(path2.characters()); close(fd); @@ -128,8 +126,7 @@ TEST_CASE(test_mkstemps_unique_filename) auto fd = mkstemps(path, 6); EXPECT_NE(fd, -1); - auto temp_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto temp_path = temp_path_string.to_byte_string(); + auto temp_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(temp_path.characters()); close(fd); @@ -151,8 +148,7 @@ TEST_CASE(test_mkstemps_unique_filename) auto fd = mkstemps(path, 6); EXPECT(fd != -1); - auto path2_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto path2 = path2_string.to_byte_string(); + auto path2 = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(path2.characters()); close(fd); diff --git a/Tests/LibELF/test-elf.cpp b/Tests/LibELF/test-elf.cpp index e808c50263b..31d0be5333b 100644 --- a/Tests/LibELF/test-elf.cpp +++ b/Tests/LibELF/test-elf.cpp @@ -58,8 +58,7 @@ TEST_CASE(test_interp_header_tiny_p_filesz) int nwritten = write(fd, buffer, sizeof(buffer)); EXPECT(nwritten); - auto elf_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto elf_path = elf_path_string.to_byte_string(); + auto elf_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(elf_path.characters()); int rc = execl(elf_path.characters(), "test-elf", nullptr); @@ -113,8 +112,7 @@ TEST_CASE(test_interp_header_p_filesz_larger_than_p_memsz) int nwritten = write(fd, buffer, sizeof(buffer)); EXPECT(nwritten); - auto elf_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto elf_path = elf_path_string.to_byte_string(); + auto elf_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(elf_path.characters()); int rc = execl(elf_path.characters(), "test-elf", nullptr); @@ -172,8 +170,7 @@ TEST_CASE(test_interp_header_p_filesz_plus_p_offset_overflow_p_memsz) int nwritten = write(fd, buffer, sizeof(buffer)); EXPECT(nwritten); - auto elf_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto elf_path = elf_path_string.to_byte_string(); + auto elf_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(elf_path.characters()); int rc = execl(elf_path.characters(), "test-elf", nullptr); @@ -228,8 +225,7 @@ TEST_CASE(test_load_header_p_memsz_zero) int nwritten = write(fd, buffer, sizeof(buffer)); EXPECT(nwritten); - auto elf_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto elf_path = elf_path_string.to_byte_string(); + auto elf_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(elf_path.characters()); int rc = execl(elf_path.characters(), "test-elf", nullptr); @@ -284,8 +280,7 @@ TEST_CASE(test_load_header_p_memsz_not_equal_to_p_align) int nwritten = write(fd, buffer, sizeof(buffer)); EXPECT(nwritten); - auto elf_path_string = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); - auto elf_path = elf_path_string.to_byte_string(); + auto elf_path = TRY_OR_FAIL(FileSystem::read_link(ByteString::formatted("/proc/{}/fd/{}", getpid(), fd))); EXPECT(elf_path.characters()); int rc = execl(elf_path.characters(), "test-elf", nullptr); diff --git a/Userland/Applications/FileManager/PropertiesWindow.cpp b/Userland/Applications/FileManager/PropertiesWindow.cpp index 74ba4d9283b..91db076fc62 100644 --- a/Userland/Applications/FileManager/PropertiesWindow.cpp +++ b/Userland/Applications/FileManager/PropertiesWindow.cpp @@ -170,9 +170,11 @@ ErrorOr PropertiesWindow::create_general_tab(GUI::TabWidget& tab_widget, b } else { auto link_destination = link_destination_or_error.release_value(); auto* link_location = general_tab.find_descendant_of_type_named("link_location"); - link_location->set_text(link_destination); + // FIXME: How do we safely display some text that might not be utf8? + auto link_destination_string = TRY(String::from_byte_string(link_destination)); + link_location->set_text(link_destination_string); link_location->on_click = [link_destination] { - auto link_directory = LexicalPath(link_destination.to_byte_string()); + auto link_directory = LexicalPath(link_destination); Desktop::Launcher::open(URL::create_with_file_scheme(link_directory.dirname(), link_directory.basename())); }; } diff --git a/Userland/Libraries/LibFileSystem/FileSystem.cpp b/Userland/Libraries/LibFileSystem/FileSystem.cpp index 662b1c43c45..2486e55ea5c 100644 --- a/Userland/Libraries/LibFileSystem/FileSystem.cpp +++ b/Userland/Libraries/LibFileSystem/FileSystem.cpp @@ -173,17 +173,17 @@ bool is_link(int fd) return S_ISLNK(st.st_mode); } -static ErrorOr get_duplicate_file_name(StringView path) +static ErrorOr get_duplicate_file_name(StringView path) { int duplicate_count = 0; LexicalPath lexical_path(path); auto parent_path = LexicalPath::canonicalized_path(lexical_path.dirname()); auto basename = lexical_path.basename(); - auto current_name = TRY(String::from_byte_string(LexicalPath::join(parent_path, basename).string())); + auto current_name = LexicalPath::join(parent_path, basename).string(); while (exists(current_name)) { ++duplicate_count; - current_name = TRY(String::from_byte_string(LexicalPath::join(parent_path, TRY(String::formatted("{} ({})", basename, duplicate_count))).string())); + current_name = LexicalPath::join(parent_path, ByteString::formatted("{} ({})", basename, duplicate_count)).string(); } return current_name; @@ -196,7 +196,7 @@ ErrorOr copy_file(StringView destination_path, StringView source_path, str if (destination_or_error.error().code() != EISDIR) return destination_or_error.release_error(); - auto destination_dir_path = TRY(String::formatted("{}/{}", destination_path, LexicalPath::basename(source_path))); + auto destination_dir_path = ByteString::formatted("{}/{}", destination_path, LexicalPath::basename(source_path)); destination_or_error = TRY(Core::File::open(destination_dir_path, Core::File::OpenMode::Write, 0666)); } auto destination = destination_or_error.release_value(); @@ -257,10 +257,10 @@ ErrorOr copy_directory(StringView destination_path, StringView source_path return di.error(); while (di.has_next()) { - auto filename = TRY(String::from_byte_string(di.next_path())); + auto filename = di.next_path(); TRY(copy_file_or_directory( - TRY(String::formatted("{}/{}", destination_path, filename)), - TRY(String::formatted("{}/{}", source_path, filename)), + ByteString::formatted("{}/{}", destination_path, filename), + ByteString::formatted("{}/{}", source_path, filename), RecursionMode::Allowed, link, AddDuplicateFileMarker::Yes, preserve_mode)); } @@ -290,11 +290,11 @@ ErrorOr copy_directory(StringView destination_path, StringView source_path ErrorOr copy_file_or_directory(StringView destination_path, StringView source_path, RecursionMode recursion_mode, LinkMode link_mode, AddDuplicateFileMarker add_duplicate_file_marker, PreserveMode preserve_mode) { - String final_destination_path; + ByteString final_destination_path; if (add_duplicate_file_marker == AddDuplicateFileMarker::Yes) final_destination_path = TRY(get_duplicate_file_name(destination_path)); else - final_destination_path = TRY(String::from_utf8(destination_path)); + final_destination_path = destination_path; auto source = TRY(Core::File::open(source_path, Core::File::OpenMode::Read)); @@ -383,9 +383,9 @@ bool can_delete_or_move(StringView path) return user_id == 0 || directory_stat.st_uid == user_id || stat_or_empty(path).st_uid == user_id; } -ErrorOr read_link(StringView link_path) +ErrorOr read_link(StringView link_path) { - return TRY(String::from_byte_string(TRY(Core::System::readlink(link_path)))); + return Core::System::readlink(link_path); } ErrorOr link_file(StringView destination_path, StringView source_path) diff --git a/Userland/Libraries/LibFileSystem/FileSystem.h b/Userland/Libraries/LibFileSystem/FileSystem.h index b7c22fdf825..d7508800780 100644 --- a/Userland/Libraries/LibFileSystem/FileSystem.h +++ b/Userland/Libraries/LibFileSystem/FileSystem.h @@ -7,8 +7,8 @@ #pragma once +#include #include -#include #include #include #include @@ -74,7 +74,7 @@ ErrorOr remove(StringView path, RecursionMode); ErrorOr size(StringView path); bool can_delete_or_move(StringView path); -ErrorOr read_link(StringView link_path); +ErrorOr read_link(StringView link_path); ErrorOr link_file(StringView destination_path, StringView source_path); bool looks_like_shared_library(StringView path); diff --git a/Userland/Libraries/LibGUI/FileIconProvider.cpp b/Userland/Libraries/LibGUI/FileIconProvider.cpp index fb6e6767f30..b9310a86227 100644 --- a/Userland/Libraries/LibGUI/FileIconProvider.cpp +++ b/Userland/Libraries/LibGUI/FileIconProvider.cpp @@ -259,7 +259,7 @@ Icon FileIconProvider::icon_for_path(StringView path, mode_t mode) auto raw_symlink_target_or_error = FileSystem::read_link(path); if (raw_symlink_target_or_error.is_error()) return s_symlink_icon; - auto raw_symlink_target = raw_symlink_target_or_error.release_value().to_byte_string(); + auto raw_symlink_target = raw_symlink_target_or_error.release_value(); ByteString target_path; if (raw_symlink_target.starts_with('/')) { diff --git a/Userland/Libraries/LibGUI/FileSystemModel.cpp b/Userland/Libraries/LibGUI/FileSystemModel.cpp index 422c77a2b77..5d40e942d41 100644 --- a/Userland/Libraries/LibGUI/FileSystemModel.cpp +++ b/Userland/Libraries/LibGUI/FileSystemModel.cpp @@ -68,7 +68,7 @@ bool FileSystemModel::Node::fetch_data(ByteString const& full_path, bool is_root if (sym_link_target_or_error.is_error()) perror("readlink"); else { - symlink_target = sym_link_target_or_error.release_value().to_byte_string(); + symlink_target = sym_link_target_or_error.release_value(); if (symlink_target.is_empty()) perror("readlink"); } diff --git a/Userland/Services/LaunchServer/Launcher.cpp b/Userland/Services/LaunchServer/Launcher.cpp index 21a9a785167..aaffa0c2890 100644 --- a/Userland/Services/LaunchServer/Launcher.cpp +++ b/Userland/Services/LaunchServer/Launcher.cpp @@ -309,7 +309,7 @@ void Launcher::for_each_handler_for_path(ByteString const& path, Function find_matching_executables_in_path(StringView filename, FollowSymlinks follow_symlinks = FollowSymlinks::No) +static Vector find_matching_executables_in_path(StringView filename, FollowSymlinks follow_symlinks = FollowSymlinks::No) { // Edge cases in which there are guaranteed no solutions if (filename.is_empty() || filename.contains('/')) @@ -65,10 +65,10 @@ static Vector find_matching_executables_in_path(StringView filename, Fol if (path_str != nullptr) // maybe && *path_str path = { path_str, strlen(path_str) }; - Vector executables; + Vector executables; auto directories = path.split_view(':'); for (auto directory : directories) { - auto file = String::formatted("{}/{}", directory, filename).release_value_but_fixme_should_propagate_errors(); + auto file = ByteString::formatted("{}/{}", directory, filename); if (follow_symlinks == FollowSymlinks::Yes) { auto path_or_error = FileSystem::read_link(file);