Browse Source

Kernel: Make FileDescription::seek() return KResultOr<off_t>

This exposed a bunch of places where errors were not propagated,
so this patch is forced to deal with them as well.
Andreas Kling 4 năm trước cách đây
mục cha
commit
d48666489c

+ 17 - 7
Kernel/FileSystem/BlockBasedFileSystem.cpp

@@ -144,7 +144,9 @@ KResult BlockBasedFS::write_block(BlockIndex index, const UserOrKernelBuffer& da
     if (!allow_cache) {
         flush_specific_block_if_needed(index);
         u32 base_offset = index.value() * block_size() + offset;
-        file_description().seek(base_offset, SEEK_SET);
+        auto seek_result = file_description().seek(base_offset, SEEK_SET);
+        if (seek_result.is_error())
+            return seek_result.error();
         auto nwritten = file_description().write(data, count);
         if (nwritten.is_error())
             return nwritten.error();
@@ -171,7 +173,8 @@ bool BlockBasedFS::raw_read(BlockIndex index, UserOrKernelBuffer& buffer)
 {
     LOCKER(m_lock);
     u32 base_offset = index.value() * m_logical_block_size;
-    file_description().seek(base_offset, SEEK_SET);
+    auto seek_result = file_description().seek(base_offset, SEEK_SET);
+    VERIFY(!seek_result.is_error());
     auto nread = file_description().read(buffer, m_logical_block_size);
     VERIFY(!nread.is_error());
     VERIFY(nread.value() == m_logical_block_size);
@@ -182,7 +185,8 @@ bool BlockBasedFS::raw_write(BlockIndex index, const UserOrKernelBuffer& buffer)
 {
     LOCKER(m_lock);
     size_t base_offset = index.value() * m_logical_block_size;
-    file_description().seek(base_offset, SEEK_SET);
+    auto seek_result = file_description().seek(base_offset, SEEK_SET);
+    VERIFY(!seek_result.is_error());
     auto nwritten = file_description().write(buffer, m_logical_block_size);
     VERIFY(!nwritten.is_error());
     VERIFY(nwritten.value() == m_logical_block_size);
@@ -236,7 +240,9 @@ KResult BlockBasedFS::read_block(BlockIndex index, UserOrKernelBuffer* buffer, s
     if (!allow_cache) {
         const_cast<BlockBasedFS*>(this)->flush_specific_block_if_needed(index);
         auto base_offset = index.value() * block_size() + offset;
-        file_description().seek(base_offset, SEEK_SET);
+        auto seek_result = file_description().seek(base_offset, SEEK_SET);
+        if (seek_result.is_error())
+            return seek_result.error();
         auto nread = file_description().read(*buffer, count);
         if (nread.is_error())
             return nread.error();
@@ -247,7 +253,9 @@ KResult BlockBasedFS::read_block(BlockIndex index, UserOrKernelBuffer* buffer, s
     auto& entry = cache().get(index);
     if (!entry.has_data) {
         auto base_offset = index.value() * block_size();
-        file_description().seek(base_offset, SEEK_SET);
+        auto seek_result = file_description().seek(base_offset, SEEK_SET);
+        if (seek_result.is_error())
+            return seek_result.error();
         auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data);
         auto nread = file_description().read(entry_data_buffer, block_size());
         if (nread.is_error())
@@ -288,7 +296,8 @@ void BlockBasedFS::flush_specific_block_if_needed(BlockIndex index)
     cache().for_each_dirty_entry([&](CacheEntry& entry) {
         if (entry.block_index != index) {
             size_t base_offset = entry.block_index.value() * block_size();
-            file_description().seek(base_offset, SEEK_SET);
+            auto seek_result = file_description().seek(base_offset, SEEK_SET);
+            VERIFY(!seek_result.is_error());
             // FIXME: Should this error path be surfaced somehow?
             auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data);
             [[maybe_unused]] auto rc = file_description().write(entry_data_buffer, block_size());
@@ -309,7 +318,8 @@ void BlockBasedFS::flush_writes_impl()
     u32 count = 0;
     cache().for_each_dirty_entry([&](CacheEntry& entry) {
         u32 base_offset = entry.block_index.value() * block_size();
-        file_description().seek(base_offset, SEEK_SET);
+        auto seek_result = file_description().seek(base_offset, SEEK_SET);
+        VERIFY(!seek_result.is_error());
         // FIXME: Should this error path be surfaced somehow?
         auto entry_data_buffer = UserOrKernelBuffer::for_kernel_buffer(entry.data);
         [[maybe_unused]] auto rc = file_description().write(entry_data_buffer, block_size());

+ 7 - 7
Kernel/FileSystem/FileDescription.cpp

@@ -126,11 +126,11 @@ KResult FileDescription::stat(::stat& buffer)
     return m_file->stat(buffer);
 }
 
-off_t FileDescription::seek(off_t offset, int whence)
+KResultOr<off_t> FileDescription::seek(off_t offset, int whence)
 {
     LOCKER(m_lock);
     if (!m_file->is_seekable())
-        return -ESPIPE;
+        return ESPIPE;
 
     off_t new_offset;
 
@@ -140,21 +140,21 @@ off_t FileDescription::seek(off_t offset, int whence)
         break;
     case SEEK_CUR:
         if (Checked<off_t>::addition_would_overflow(m_current_offset, offset))
-            return -EOVERFLOW;
+            return EOVERFLOW;
         new_offset = m_current_offset + offset;
         break;
     case SEEK_END:
         if (!metadata().is_valid())
-            return -EIO;
+            return EIO;
         new_offset = metadata().size;
         break;
     default:
-        return -EINVAL;
+        return EINVAL;
     }
 
     if (new_offset < 0)
-        return -EINVAL;
-    // FIXME: Return -EINVAL if attempting to seek past the end of a seekable device.
+        return EINVAL;
+    // FIXME: Return EINVAL if attempting to seek past the end of a seekable device.
 
     m_current_offset = new_offset;
 

+ 1 - 1
Kernel/FileSystem/FileDescription.h

@@ -66,7 +66,7 @@ public:
 
     KResult close();
 
-    off_t seek(off_t, int whence);
+    KResultOr<off_t> seek(off_t, int whence);
     KResultOr<size_t> read(UserOrKernelBuffer&, size_t);
     KResultOr<size_t> write(const UserOrKernelBuffer& data, size_t);
     KResult stat(::stat&);

+ 2 - 1
Kernel/Syscalls/execve.cpp

@@ -559,7 +559,8 @@ KResult Process::do_exec(NonnullRefPtr<FileDescription> main_program_description
     if (interpreter_description) {
         main_program_fd = alloc_fd();
         VERIFY(main_program_fd >= 0);
-        main_program_description->seek(0, SEEK_SET);
+        auto seek_result = main_program_description->seek(0, SEEK_SET);
+        VERIFY(!seek_result.is_error());
         main_program_description->set_readable(true);
         m_fds[main_program_fd].set(move(main_program_description), FD_CLOEXEC);
     }

+ 5 - 6
Kernel/Syscalls/lseek.cpp

@@ -38,13 +38,12 @@ KResultOr<int> Process::sys$lseek(int fd, Userspace<off_t*> userspace_offset, in
     off_t offset;
     if (!copy_from_user(&offset, userspace_offset))
         return EFAULT;
-    offset = description->seek(offset, whence);
-    if (!copy_to_user(userspace_offset, &offset))
+    auto seek_result = description->seek(offset, whence);
+    if (seek_result.is_error())
+        return seek_result.error();
+    if (!copy_to_user(userspace_offset, &seek_result.value()))
         return EFAULT;
-    if (offset < 0)
-        return offset;
-    else
-        return 0;
+    return KSuccess;
 }
 
 }

+ 5 - 2
Kernel/Syscalls/write.cpp

@@ -84,8 +84,11 @@ KResultOr<ssize_t> Process::do_write(FileDescription& description, const UserOrK
             return EAGAIN;
     }
 
-    if (description.should_append())
-        description.seek(0, SEEK_END);
+    if (description.should_append()) {
+        auto seek_result = description.seek(0, SEEK_END);
+        if (seek_result.is_error())
+            return seek_result.error();
+    }
 
     while ((size_t)total_nwritten < data_size) {
         if (!description.can_write()) {