From 2bba743c2443612309ccbcd6a478ecdfa16f18b3 Mon Sep 17 00:00:00 2001 From: Lucas CHOLLET Date: Sat, 14 Jan 2023 22:04:20 -0500 Subject: [PATCH] HexEditor: Propagate errors when using "Save as" --- .../Applications/HexEditor/HexDocument.cpp | 35 ++++++++----------- Userland/Applications/HexEditor/HexDocument.h | 4 +-- Userland/Applications/HexEditor/HexEditor.cpp | 11 +++--- Userland/Applications/HexEditor/HexEditor.h | 2 +- .../HexEditor/HexEditorWidget.cpp | 4 +-- 5 files changed, 25 insertions(+), 31 deletions(-) diff --git a/Userland/Applications/HexEditor/HexDocument.cpp b/Userland/Applications/HexEditor/HexDocument.cpp index 8498e733e4..7b27003b9f 100644 --- a/Userland/Applications/HexEditor/HexDocument.cpp +++ b/Userland/Applications/HexEditor/HexDocument.cpp @@ -58,17 +58,15 @@ void HexDocumentMemory::clear_changes() m_changes.clear(); } -bool HexDocumentMemory::write_to_file(Core::Stream::File& file) +ErrorOr HexDocumentMemory::write_to_file(Core::Stream::File& file) { - if (file.seek(0, SeekMode::SetPosition).is_error()) - return false; - if (file.write(m_buffer).is_error()) - return false; + TRY(file.seek(0, SeekMode::SetPosition)); + TRY(file.write(m_buffer)); for (auto& change : m_changes) { - file.seek(change.key, SeekMode::SetPosition).release_value_but_fixme_should_propagate_errors(); - file.write({ &change.value, 1 }).release_value_but_fixme_should_propagate_errors(); + TRY(file.seek(change.key, SeekMode::SetPosition)); + TRY(file.write({ &change.value, 1 })); } - return true; + return {}; } ErrorOr> HexDocumentFile::create(NonnullOwnPtr file) @@ -96,30 +94,27 @@ void HexDocumentFile::write_to_file() m_buffer_file_pos = m_file_size + 1; } -bool HexDocumentFile::write_to_file(Core::Stream::File& file) +ErrorOr HexDocumentFile::write_to_file(Core::Stream::File& file) { - if (file.truncate(size()).is_error()) { - return false; - } + TRY(file.truncate(size())); - if (file.seek(0, SeekMode::SetPosition).is_error() || m_file->seek(0, SeekMode::SetPosition).is_error()) { - return false; - } + TRY(file.seek(0, SeekMode::SetPosition)); + TRY(m_file->seek(0, SeekMode::SetPosition)); while (true) { Array buffer; - auto copy_buffer = m_file->read(buffer).release_value_but_fixme_should_propagate_errors(); + auto copy_buffer = TRY(m_file->read(buffer)); if (copy_buffer.size() == 0) break; - file.write(copy_buffer).release_value_but_fixme_should_propagate_errors(); + TRY(file.write(copy_buffer)); } for (auto& change : m_changes) { - file.seek(change.key, SeekMode::SetPosition).release_value_but_fixme_should_propagate_errors(); - file.write({ &change.value, 1 }).release_value_but_fixme_should_propagate_errors(); + TRY(file.seek(change.key, SeekMode::SetPosition)); + TRY(file.write({ &change.value, 1 })); } - return true; + return {}; } HexDocument::Cell HexDocumentFile::get(size_t position) diff --git a/Userland/Applications/HexEditor/HexDocument.h b/Userland/Applications/HexEditor/HexDocument.h index e18d87c1f3..00fe630868 100644 --- a/Userland/Applications/HexEditor/HexDocument.h +++ b/Userland/Applications/HexEditor/HexDocument.h @@ -50,7 +50,7 @@ public: size_t size() const override; Type type() const override; void clear_changes() override; - bool write_to_file(Core::Stream::File& file); + ErrorOr write_to_file(Core::Stream::File& file); private: ByteBuffer m_buffer; @@ -67,7 +67,7 @@ public: void set_file(NonnullOwnPtr file); NonnullOwnPtr const& file() const; void write_to_file(); - bool write_to_file(Core::Stream::File& file); + ErrorOr write_to_file(Core::Stream::File& file); Cell get(size_t position) override; u8 get_unchanged(size_t position) override; size_t size() const override; diff --git a/Userland/Applications/HexEditor/HexEditor.cpp b/Userland/Applications/HexEditor/HexEditor.cpp index ab20f05d6b..f5cb6b6b88 100644 --- a/Userland/Applications/HexEditor/HexEditor.cpp +++ b/Userland/Applications/HexEditor/HexEditor.cpp @@ -135,23 +135,22 @@ void HexEditor::set_selection(size_t position, size_t length) scroll_position_into_view(position); update_status(); } -bool HexEditor::save_as(NonnullOwnPtr new_file) + +ErrorOr HexEditor::save_as(NonnullOwnPtr new_file) { if (m_document->type() == HexDocument::Type::File) { auto& file_document = static_cast(*m_document); - if (!file_document.write_to_file(*new_file)) - return false; + TRY(file_document.write_to_file(*new_file)); file_document.set_file(move(new_file)); } else { auto& memory_document = static_cast(*m_document); - if (!memory_document.write_to_file(*new_file)) - return false; + TRY(memory_document.write_to_file(*new_file)); m_document = HexDocumentFile::create(move(new_file)).release_value_but_fixme_should_propagate_errors(); } update(); - return true; + return {}; } bool HexEditor::save() diff --git a/Userland/Applications/HexEditor/HexEditor.h b/Userland/Applications/HexEditor/HexEditor.h index 7207134e4d..35ff1dfb8c 100644 --- a/Userland/Applications/HexEditor/HexEditor.h +++ b/Userland/Applications/HexEditor/HexEditor.h @@ -38,7 +38,7 @@ public: void open_file(NonnullOwnPtr file); ErrorOr fill_selection(u8 fill_byte); Optional get_byte(size_t position); - bool save_as(NonnullOwnPtr); + ErrorOr save_as(NonnullOwnPtr); bool save(); bool undo(); diff --git a/Userland/Applications/HexEditor/HexEditorWidget.cpp b/Userland/Applications/HexEditor/HexEditorWidget.cpp index 0ac4bc9471..e433d56db1 100644 --- a/Userland/Applications/HexEditor/HexEditorWidget.cpp +++ b/Userland/Applications/HexEditor/HexEditorWidget.cpp @@ -146,8 +146,8 @@ HexEditorWidget::HexEditorWidget() if (response.is_error()) return; auto file = response.release_value(); - if (!m_editor->save_as(file.release_stream())) { - GUI::MessageBox::show(window(), "Unable to save file.\n"sv, "Error"sv, GUI::MessageBox::Type::Error); + if (auto result = m_editor->save_as(file.release_stream()); result.is_error()) { + GUI::MessageBox::show(window(), DeprecatedString::formatted("Unable to save file: {}\n"sv, result.error()), "Error"sv, GUI::MessageBox::Type::Error); return; }