Userland+Tests: Convert File::read_link() from String to ErrorOr<String>

This converts the return value of File::read_link() from String to
ErrorOr<String>.

The rest of the change is to support the potential of an Error being
returned and subsequent release of the value when no Error is returned.
Unfortunately at this stage none of the places affected can utililize
our TRY() macro.

Tests/Kernel/TestKernelFilePermissions.cpp

@@ -81,7 +81,10 @@ TEST_CASE(test_change_file_location)
     ftruncate(fd, 0);
     EXPECT(fchmod(fd, 06755) != -1);
 
-    auto suid_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto suid_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!suid_path_or_error.is_error());
+
+    auto suid_path = suid_path_or_error.release_value();
     EXPECT(suid_path.characters());
     auto new_path = String::formatted("{}.renamed", suid_path);
 

Tests/LibC/TestIo.cpp

@@ -216,7 +216,10 @@ TEST_CASE(unlink_symlink)
         perror("symlink");
     }
 
-    auto target = Core::File::read_link("/tmp/linky");
+    auto target_or_error = Core::File::read_link("/tmp/linky");
+    EXPECT(!target_or_error.is_error());
+
+    auto target = target_or_error.release_value();
     EXPECT_EQ(target, "/proc/2/foo");
 
     rc = unlink("/tmp/linky");

Tests/LibC/TestLibCMkTemp.cpp

@@ -86,7 +86,10 @@ TEST_CASE(test_mkstemp_unique_filename)
         auto fd = mkstemp(path);
         EXPECT_NE(fd, -1);
 
-        auto temp_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+        auto temp_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+        EXPECT(!temp_path_or_error.is_error());
+
+        auto temp_path = temp_path_or_error.release_value();
         EXPECT(temp_path.characters());
 
         close(fd);
@@ -104,7 +107,10 @@ TEST_CASE(test_mkstemp_unique_filename)
         auto fd = mkstemp(path);
         EXPECT(fd != -1);
 
-        auto path2 = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+        auto path2_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+        EXPECT(!path2_or_error.is_error());
+
+        auto path2 = path2_or_error.release_value();
         EXPECT(path2.characters());
 
         close(fd);

Tests/LibELF/test-elf.cpp

@@ -58,7 +58,10 @@ TEST_CASE(test_interp_header_tiny_p_filesz)
     int nwritten = write(fd, buffer, sizeof(buffer));
     EXPECT(nwritten);
 
-    auto elf_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto elf_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!elf_path_or_error.is_error());
+
+    auto elf_path = elf_path_or_error.release_value();
     EXPECT(elf_path.characters());
 
     int rc = execl(elf_path.characters(), "test-elf", nullptr);
@@ -112,7 +115,10 @@ TEST_CASE(test_interp_header_p_filesz_larger_than_p_memsz)
     int nwritten = write(fd, buffer, sizeof(buffer));
     EXPECT(nwritten);
 
-    auto elf_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto elf_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!elf_path_or_error.is_error());
+
+    auto elf_path = elf_path_or_error.release_value();
     EXPECT(elf_path.characters());
 
     int rc = execl(elf_path.characters(), "test-elf", nullptr);
@@ -170,7 +176,10 @@ 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 = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto elf_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!elf_path_or_error.is_error());
+
+    auto elf_path = elf_path_or_error.release_value();
     EXPECT(elf_path.characters());
 
     int rc = execl(elf_path.characters(), "test-elf", nullptr);
@@ -225,7 +234,10 @@ TEST_CASE(test_load_header_p_memsz_zero)
     int nwritten = write(fd, buffer, sizeof(buffer));
     EXPECT(nwritten);
 
-    auto elf_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto elf_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!elf_path_or_error.is_error());
+
+    auto elf_path = elf_path_or_error.release_value();
     EXPECT(elf_path.characters());
 
     int rc = execl(elf_path.characters(), "test-elf", nullptr);
@@ -280,7 +292,10 @@ TEST_CASE(test_load_header_p_memsz_not_equal_to_p_align)
     int nwritten = write(fd, buffer, sizeof(buffer));
     EXPECT(nwritten);
 
-    auto elf_path = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    auto elf_path_or_error = Core::File::read_link(String::formatted("/proc/{}/fd/{}", getpid(), fd));
+    EXPECT(!elf_path_or_error.is_error());
+
+    auto elf_path = elf_path_or_error.release_value();
     EXPECT(elf_path.characters());
 
     int rc = execl(elf_path.characters(), "test-elf", nullptr);

Userland/Applications/FileManager/PropertiesWindow.cpp

@@ -95,10 +95,11 @@ PropertiesWindow::PropertiesWindow(String const& path, bool disable_rename, Wind
     };
 
     if (S_ISLNK(m_mode)) {
-        auto link_destination = Core::File::read_link(path);
-        if (link_destination.is_null()) {
+        auto link_destination_or_error = Core::File::read_link(path);
+        if (link_destination_or_error.is_error()) {
             perror("readlink");
         } else {
+            auto link_destination = link_destination_or_error.release_value();
             auto link_location = general_tab.find_descendant_of_type_named<GUI::LinkLabel>("link_location");
             link_location->set_text(link_destination);
             link_location->on_click = [link_destination] {

Userland/Libraries/LibCore/File.cpp

@@ -247,28 +247,28 @@ String File::absolute_path(String const& path)
 
 #ifdef __serenity__
 
-String File::read_link(String const& link_path)
+ErrorOr<String> File::read_link(String const& link_path)
 {
     // First, try using a 64-byte buffer, that ought to be enough for anybody.
     char small_buffer[64];
 
     int rc = serenity_readlink(link_path.characters(), link_path.length(), small_buffer, sizeof(small_buffer));
     if (rc < 0)
-        return {};
+        return Error::from_errno(errno);
 
     size_t size = rc;
     // If the call was successful, the syscall (unlike the LibC wrapper)
     // returns the full size of the link. Let's see if our small buffer
     // was enough to read the whole link.
     if (size <= sizeof(small_buffer))
-        return { small_buffer, size };
+        return String { small_buffer, size };
     // Nope, but at least now we know the right size.
     char* large_buffer_ptr;
     auto large_buffer = StringImpl::create_uninitialized(size, large_buffer_ptr);
 
     rc = serenity_readlink(link_path.characters(), link_path.length(), large_buffer_ptr, size);
     if (rc < 0)
-        return {};
+        return Error::from_errno(errno);
 
     size_t new_size = rc;
     if (new_size == size)
@@ -277,31 +277,31 @@ String File::read_link(String const& link_path)
     // If we're here, the symlink has changed while we were looking at it.
     // If it became shorter, our buffer is valid, we just have to trim it a bit.
     if (new_size < size)
-        return { large_buffer_ptr, new_size };
+        return String { large_buffer_ptr, new_size };
     // Otherwise, here's not much we can do, unless we want to loop endlessly
     // in this case. Let's leave it up to the caller whether to loop.
     errno = EAGAIN;
-    return {};
+    return Error::from_errno(errno);
 }
 
 #else
 
 // This is a sad version for other systems. It has to always make a copy of the
 // link path, and to always make two syscalls to get the right size first.
-String File::read_link(String const& link_path)
+ErrorOr<String> File::read_link(String const& link_path)
 {
     struct stat statbuf = {};
     int rc = lstat(link_path.characters(), &statbuf);
     if (rc < 0)
-        return {};
+        return Error::from_errno(errno);
     char* buffer_ptr;
     auto buffer = StringImpl::create_uninitialized(statbuf.st_size, buffer_ptr);
     if (readlink(link_path.characters(), buffer_ptr, statbuf.st_size) < 0)
-        return {};
+        return Error::from_errno(errno);
     // (See above.)
     if (rc == statbuf.st_size)
         return { *buffer };
-    return { buffer_ptr, (size_t)rc };
+    return String { buffer_ptr, (size_t)rc };
 }
 
 #endif

Userland/Libraries/LibCore/File.h

@@ -76,7 +76,7 @@ public:
     static ErrorOr<void, CopyError> copy_file_or_directory(String const& dst_path, String const& src_path, RecursionMode = RecursionMode::Allowed, LinkMode = LinkMode::Disallowed, AddDuplicateFileMarker = AddDuplicateFileMarker::Yes, PreserveMode = PreserveMode::Nothing);
 
     static String real_path_for(String const& filename);
-    static String read_link(String const& link_path);
+    static ErrorOr<String> read_link(String const& link_path);
     static ErrorOr<void> link_file(String const& dst_path, String const& src_path);
 
     struct RemoveError : public Error {

Userland/Libraries/LibGUI/FileIconProvider.cpp

@@ -232,7 +232,11 @@ Icon FileIconProvider::icon_for_path(const String& path, mode_t mode)
         return s_directory_icon;
     }
     if (S_ISLNK(mode)) {
-        auto raw_symlink_target = Core::File::read_link(path);
+        auto raw_symlink_target_or_error = Core::File::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();
         if (raw_symlink_target.is_null())
             return s_symlink_icon;
 

Userland/Libraries/LibGUI/FileSystemModel.cpp

@@ -61,7 +61,11 @@ bool FileSystemModel::Node::fetch_data(String const& full_path, bool is_root)
     mtime = st.st_mtime;
 
     if (S_ISLNK(mode)) {
-        symlink_target = Core::File::read_link(full_path);
+        auto sym_link_target_or_error = Core::File::read_link(full_path);
+        if (sym_link_target_or_error.is_error())
+            perror("readlink");
+
+        symlink_target = sym_link_target_or_error.release_value();
         if (symlink_target.is_null())
             perror("readlink");
     }

Userland/Services/LaunchServer/Launcher.cpp

@@ -271,7 +271,11 @@ void Launcher::for_each_handler_for_path(const String& path, Function<bool(const
         return;
 
     if (S_ISLNK(st.st_mode)) {
-        auto link_target = LexicalPath { Core::File::read_link(path) };
+        auto link_target_or_error = Core::File::read_link(path);
+        if (link_target_or_error.is_error())
+            perror("read_link");
+
+        auto link_target = LexicalPath { link_target_or_error.release_value() };
         LexicalPath absolute_link_target = link_target.is_absolute() ? link_target : LexicalPath::join(LexicalPath::dirname(path), link_target.string());
         auto real_path = Core::File::real_path_for(absolute_link_target.string());
         return for_each_handler_for_path(real_path, [&](const auto& handler) -> bool {

Userland/Utilities/ls.cpp

@@ -275,11 +275,11 @@ static size_t print_name(const struct stat& st, const String& name, const char*
     }
     if (S_ISLNK(st.st_mode)) {
         if (path_for_link_resolution) {
-            auto link_destination = Core::File::read_link(path_for_link_resolution);
-            if (link_destination.is_null()) {
+            auto link_destination_or_error = Core::File::read_link(path_for_link_resolution);
+            if (link_destination_or_error.is_error()) {
                 perror("readlink");
             } else {
-                nprinted += printf(" -> ") + print_escaped(link_destination.characters());
+                nprinted += printf(" -> ") + print_escaped(link_destination_or_error.value().characters());
             }
         } else {
             if (flag_classify)

Userland/Utilities/readlink.cpp

@@ -25,11 +25,13 @@ int main(int argc, char** argv)
     args_parser.parse(argc, argv);
 
     for (const char* path : paths) {
-        auto destination = Core::File::read_link(path);
-        if (destination.is_null()) {
+        auto destination_or_error = Core::File::read_link(path);
+        if (destination_or_error.is_error()) {
             perror(path);
             return 1;
         }
+
+        auto destination = destination_or_error.release_value();
         out("{}", destination);
         if (!no_newline)
             outln();