From 779c44c7b8138a0a4bc630f09aa30ed99b84510c Mon Sep 17 00:00:00 2001 From: Andrew Gunnerson Date: Sun, 25 Nov 2018 16:40:13 -0500 Subject: [PATCH] libmbcommon: Remove concept of fatal state from mb::File Signed-off-by: Andrew Gunnerson --- libmbbootimg/src/format/android_reader.cpp | 51 ++++---------- libmbbootimg/src/format/android_writer.cpp | 24 ++----- libmbbootimg/src/format/loki.cpp | 50 +++----------- libmbbootimg/src/format/loki_reader.cpp | 73 +++++---------------- libmbbootimg/src/format/loki_writer.cpp | 30 ++------- libmbbootimg/src/format/mtk_reader.cpp | 8 +-- libmbbootimg/src/format/mtk_writer.cpp | 57 +++------------- libmbbootimg/src/format/segment_reader.cpp | 19 ++---- libmbbootimg/src/format/segment_writer.cpp | 16 ++--- libmbbootimg/src/format/sony_elf_reader.cpp | 35 ++-------- libmbbootimg/src/format/sony_elf_writer.cpp | 18 +---- libmbbootimg/src/reader.cpp | 12 +--- libmbcommon/include/mbcommon/file.h | 2 - libmbcommon/include/mbcommon/file_p.h | 3 +- libmbcommon/src/file.cpp | 30 +-------- libmbcommon/src/file/callbacks.cpp | 3 +- libmbcommon/src/file_util.cpp | 1 - libmbcommon/tests/test_file.cpp | 18 ----- libmbsparse/src/sparse.cpp | 30 ++------- libmbsparse/tests/test_sparse.cpp | 9 --- 20 files changed, 89 insertions(+), 400 deletions(-) diff --git a/libmbbootimg/src/format/android_reader.cpp b/libmbbootimg/src/format/android_reader.cpp index c1f48c4b1..2d8a6a9f9 100644 --- a/libmbbootimg/src/format/android_reader.cpp +++ b/libmbbootimg/src/format/android_reader.cpp @@ -246,20 +246,13 @@ AndroidFormatReader::find_header(Reader &reader, File &file, return AndroidError::InvalidArgument; } - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); - auto n = file_read_retry(file, buf, static_cast(max_header_offset) - + sizeof(AndroidHeader)); - if (!n) { - if (file.is_fatal()) { reader.set_fatal(); } - return n.as_failure(); - } + OUTCOME_TRY(n, file_read_retry(file, buf, + static_cast(max_header_offset) + + sizeof(AndroidHeader))); - auto buf_end = buf + n.value(); + auto buf_end = buf + n; auto it = std2::search( buf, buf_end, std2::boyer_moore_searcher( BOOT_MAGIC, BOOT_MAGIC + BOOT_MAGIC_SIZE)); @@ -271,7 +264,7 @@ AndroidFormatReader::find_header(Reader &reader, File &file, offset = static_cast(it - buf); - if (n.value() - offset < sizeof(AndroidHeader)) { + if (n - offset < sizeof(AndroidHeader)) { //DEBUG("Android header at %" MB_PRIzu " exceeds file size", offset); return AndroidError::HeaderOutOfBounds; } @@ -329,20 +322,12 @@ AndroidFormatReader::find_samsung_seandroid_magic(Reader &reader, File &file, pos += hdr.dt_size; pos += align_page_size(pos, hdr.page_size); - auto seek_ret = file.seek(static_cast(pos), SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(pos), SEEK_SET)); - auto n = file_read_retry(file, buf, sizeof(buf)); - if (!n) { - if (file.is_fatal()) { reader.set_fatal(); } - return n.as_failure(); - } + OUTCOME_TRY(n, file_read_retry(file, buf, sizeof(buf))); - if (n.value() != SAMSUNG_SEANDROID_MAGIC_SIZE - || memcmp(buf, SAMSUNG_SEANDROID_MAGIC, n.value()) != 0) { + if (n != SAMSUNG_SEANDROID_MAGIC_SIZE + || memcmp(buf, SAMSUNG_SEANDROID_MAGIC, n) != 0) { //DEBUG("SEAndroid magic not found in last %" MB_PRIzu " bytes", // SAMSUNG_SEANDROID_MAGIC_SIZE); return AndroidError::SamsungMagicNotFound; @@ -397,20 +382,12 @@ AndroidFormatReader::find_bump_magic(Reader &reader, File &file, pos += hdr.dt_size; pos += align_page_size(pos, hdr.page_size); - auto seek_ret = file.seek(static_cast(pos), SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(pos), SEEK_SET)); - auto n = file_read_retry(file, buf, sizeof(buf)); - if (!n) { - if (file.is_fatal()) { reader.set_fatal(); } - return n.as_failure(); - } + OUTCOME_TRY(n, file_read_retry(file, buf, sizeof(buf))); - if (n.value() != bump::BUMP_MAGIC_SIZE - || memcmp(buf, bump::BUMP_MAGIC, n.value()) != 0) { + if (n != bump::BUMP_MAGIC_SIZE + || memcmp(buf, bump::BUMP_MAGIC, n) != 0) { //DEBUG("Bump magic not found in last %" MB_PRIzu " bytes", // bump::BUMP_MAGIC_SIZE); return AndroidError::BumpMagicNotFound; diff --git a/libmbbootimg/src/format/android_writer.cpp b/libmbbootimg/src/format/android_writer.cpp index 36cebc7f5..af35fd382 100644 --- a/libmbbootimg/src/format/android_writer.cpp +++ b/libmbbootimg/src/format/android_writer.cpp @@ -108,11 +108,7 @@ oc::result AndroidFormatWriter::close(File &file) ? bump::BUMP_MAGIC_SIZE : SAMSUNG_SEANDROID_MAGIC_SIZE; - auto ret = file_write_exact(file, magic, magic_size); - if (!ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, magic, magic_size)); // Set ID unsigned char digest[SHA_DIGEST_LENGTH]; @@ -126,18 +122,10 @@ oc::result AndroidFormatWriter::close(File &file) android_fix_header_byte_order(m_hdr); // Seek back to beginning to write header - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); // Write header - ret = file_write_exact(file, &m_hdr, sizeof(m_hdr)); - if (!ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &m_hdr, sizeof(m_hdr))); } } @@ -223,11 +211,7 @@ oc::result AndroidFormatWriter::write_header(File &file, OUTCOME_TRYV(m_seg->set_entries(std::move(entries))); // Start writing after first page - auto seek_ret = file.seek(m_hdr.page_size, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(m_hdr.page_size, SEEK_SET)); return oc::success(); } diff --git a/libmbbootimg/src/format/loki.cpp b/libmbbootimg/src/format/loki.cpp index 3fd28e142..09255d891 100644 --- a/libmbbootimg/src/format/loki.cpp +++ b/libmbbootimg/src/format/loki.cpp @@ -146,17 +146,9 @@ static oc::result _loki_read_android_header(Writer &writer, File &file, android::AndroidHeader &ahdr) { - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); - auto ret = file_read_exact(file, &ahdr, sizeof(ahdr)); - if (!ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, &ahdr, sizeof(ahdr))); android_fix_header_byte_order(ahdr); @@ -171,17 +163,9 @@ _loki_write_android_header(Writer &writer, File &file, android_fix_header_byte_order(dup); - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); - auto ret = file_write_exact(file, &dup, sizeof(dup)); - if (!ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &dup, sizeof(dup))); return oc::success(); } @@ -194,17 +178,9 @@ _loki_write_loki_header(Writer &writer, File &file, loki_fix_header_byte_order(dup); - auto seek_ret = file.seek(LOKI_MAGIC_OFFSET, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(LOKI_MAGIC_OFFSET, SEEK_SET)); - auto ret = file_write_exact(file, &dup, sizeof(dup)); - if (!ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &dup, sizeof(dup))); return oc::success(); } @@ -214,11 +190,9 @@ _loki_move_dt_image(Writer &writer, File &file, uint64_t aboot_offset, uint32_t fake_size, uint32_t dt_size) { // Move DT image - auto n = file_move(file, aboot_offset, aboot_offset + fake_size, dt_size); - if (!n) { - if (file.is_fatal()) { writer.set_fatal(); } - return n.as_failure(); - } else if (n.value() != dt_size) { + OUTCOME_TRY(n, file_move(file, aboot_offset, aboot_offset + fake_size, + dt_size)); + if (n != dt_size) { // Non-recoverable writer.set_fatal(); return FileError::UnexpectedEof; @@ -239,11 +213,7 @@ _loki_write_aboot(Writer &writer, File &file, return LokiError::AbootFunctionOutOfRange; } - auto seek_ret = file.seek(static_cast(aboot_offset), SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(aboot_offset), SEEK_SET)); auto ret = file_write_exact(file, aboot + aboot_func_offset, fake_size); if (!ret) { diff --git a/libmbbootimg/src/format/loki_reader.cpp b/libmbbootimg/src/format/loki_reader.cpp index 4261b9f36..64f4dee05 100644 --- a/libmbbootimg/src/format/loki_reader.cpp +++ b/libmbbootimg/src/format/loki_reader.cpp @@ -216,18 +216,13 @@ oc::result LokiFormatReader::find_loki_header(Reader &reader, File &file, { LokiHeader header; - auto seek_ret = file.seek(LOKI_MAGIC_OFFSET, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(LOKI_MAGIC_OFFSET, SEEK_SET)); auto ret = file_read_exact(file, &header, sizeof(header)); if (!ret) { if (ret.error() == FileError::UnexpectedEof) { return LokiError::LokiHeaderTooSmall; } else { - if (file.is_fatal()) { reader.set_fatal(); } return ret.as_failure(); } } @@ -282,12 +277,8 @@ LokiFormatReader::find_ramdisk_address(Reader &reader, File &file, return FileSearchAction::Continue; }; - auto ret = file_search(file, {}, {}, 0, LOKI_SHELLCODE, - LOKI_SHELLCODE_SIZE - 9, 1, result_cb); - if (!ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_search(file, {}, {}, 0, LOKI_SHELLCODE, + LOKI_SHELLCODE_SIZE - 9, 1, result_cb)); if (offset == 0) { return LokiError::ShellcodeNotFound; @@ -295,18 +286,9 @@ LokiFormatReader::find_ramdisk_address(Reader &reader, File &file, offset += LOKI_SHELLCODE_SIZE - 5; - auto seek_ret = file.seek(static_cast(offset), SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(offset), SEEK_SET)); - auto read_ret = file_read_exact(file, &ramdisk_addr, - sizeof(ramdisk_addr)); - if (!read_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return read_ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, &ramdisk_addr, sizeof(ramdisk_addr))); ramdisk_addr = mb_le32toh(ramdisk_addr); } else { @@ -409,12 +391,8 @@ LokiFormatReader::find_gzip_offset_old(Reader &reader, File &file, return FileSearchAction::Continue; }; - auto ret = file_search(file, start_offset, {}, 0, gzip_deflate_magic, - sizeof(gzip_deflate_magic), {}, result_cb); - if (!ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_search(file, start_offset, {}, 0, gzip_deflate_magic, + sizeof(gzip_deflate_magic), {}, result_cb)); // Prefer gzip header with original filename flag since most loki'd boot // images will have been compressed manually with the gzip tool @@ -468,43 +446,30 @@ LokiFormatReader::find_ramdisk_size_old(Reader &reader, File &file, aboot_size = 0x200; } - auto aboot_offset = file.seek(-aboot_size, SEEK_END); - if (!aboot_offset) { - if (file.is_fatal()) { reader.set_fatal(); } - return aboot_offset.as_failure(); - } + OUTCOME_TRY(aboot_offset, file.seek(-aboot_size, SEEK_END)); - if (ramdisk_offset > aboot_offset.value()) { + if (ramdisk_offset > aboot_offset) { return LokiError::RamdiskOffsetGreaterThanAbootOffset; } // Ignore zero padding as we might strip away too much #if 1 - ramdisk_size_out = static_cast( - aboot_offset.value() - ramdisk_offset); + ramdisk_size_out = static_cast(aboot_offset - ramdisk_offset); return oc::success(); #else char buf[1024]; // Search backwards to find non-zero byte - uint64_t cur_offset = aboot_offset.value(); + uint64_t cur_offset = aboot_offset; while (cur_offset > ramdisk_offset) { size_t to_read = std::min( sizeof(buf), cur_offset - ramdisk_offset); cur_offset -= to_read; - auto seek_ret = file.seek(cur_offset, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(cur_offset, SEEK_SET)); - auto ret = file_read_exact(file, buf, to_read); - if (!ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, buf, to_read)); for (size_t i = to_read; i-- > 0; ) { if (buf[i] != '\0') { @@ -548,17 +513,9 @@ LokiFormatReader::find_linux_kernel_size(Reader &reader,File &file, // shellcode). The size is stored in the kernel image's header though, so // we'll use that. // http://www.simtec.co.uk/products/SWLINUX/files/booting_article.html#d0e309 - auto seek_ret = file.seek(kernel_offset + 0x2c, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(kernel_offset + 0x2c, SEEK_SET)); - auto ret = file_read_exact(file, &kernel_size, sizeof(kernel_size)); - if (!ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, &kernel_size, sizeof(kernel_size))); kernel_size_out = mb_le32toh(kernel_size); return oc::success(); diff --git a/libmbbootimg/src/format/loki_writer.cpp b/libmbbootimg/src/format/loki_writer.cpp index 5ba9230c7..415c49cff 100644 --- a/libmbbootimg/src/format/loki_writer.cpp +++ b/libmbbootimg/src/format/loki_writer.cpp @@ -96,18 +96,10 @@ oc::result LokiFormatWriter::close(File &file) // If successful, finish up the boot image if (swentry == m_seg->entries().end()) { - auto file_size = file.seek(0, SEEK_CUR); - if (!file_size) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return file_size.as_failure(); - } + OUTCOME_TRY(file_size, file.seek(0, SEEK_CUR)); // Truncate to set size - auto truncate_ret = file.truncate(file_size.value()); - if (!truncate_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return truncate_ret.as_failure(); - } + OUTCOME_TRYV(file.truncate(file_size)); // Set ID unsigned char digest[SHA_DIGEST_LENGTH]; @@ -121,18 +113,10 @@ oc::result LokiFormatWriter::close(File &file) android_fix_header_byte_order(m_hdr); // Seek back to beginning to write header - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); // Write header - auto ret = file_write_exact(file, &m_hdr, sizeof(m_hdr)); - if (!ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &m_hdr, sizeof(m_hdr))); // Patch with Loki OUTCOME_TRYV(_loki_patch_file(m_writer, file, m_aboot.data(), @@ -222,11 +206,7 @@ oc::result LokiFormatWriter::write_header(File &file, OUTCOME_TRYV(m_seg->set_entries(std::move(entries))); // Start writing after first page - auto seek_ret = file.seek(m_hdr.page_size, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(m_hdr.page_size, SEEK_SET)); return oc::success(); } diff --git a/libmbbootimg/src/format/mtk_reader.cpp b/libmbbootimg/src/format/mtk_reader.cpp index 3e63f16b8..2ecd385ea 100644 --- a/libmbbootimg/src/format/mtk_reader.cpp +++ b/libmbbootimg/src/format/mtk_reader.cpp @@ -71,12 +71,7 @@ read_mtk_header(Reader &reader, File &file, { MtkHeader mtkhdr; - auto seek_ret = file.seek(static_cast(offset), SEEK_SET); - if (!seek_ret) { - //DEBUG("Failed to seek to MTK header at %" PRIu64, offset); - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(offset), SEEK_SET)); auto ret = file_read_exact(file, &mtkhdr, sizeof(mtkhdr)); if (!ret) { @@ -84,7 +79,6 @@ read_mtk_header(Reader &reader, File &file, //DEBUG("MTK header not found at %" PRIu64, offset); return MtkError::MtkHeaderNotFound; } else { - if (file.is_fatal()) { reader.set_fatal(); } return ret.as_failure(); } } diff --git a/libmbbootimg/src/format/mtk_writer.cpp b/libmbbootimg/src/format/mtk_writer.cpp index eee149910..33650de33 100644 --- a/libmbbootimg/src/format/mtk_writer.cpp +++ b/libmbbootimg/src/format/mtk_writer.cpp @@ -57,18 +57,10 @@ _mtk_header_update_size(Writer &writer, File &file, return MtkError::MtkHeaderOffsetTooLarge; } - auto seek_ret = file.seek( - static_cast(offset + offsetof(MtkHeader, size)), SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek( + static_cast(offset + offsetof(MtkHeader, size)), SEEK_SET)); - auto ret = file_write_exact(file, &le32_size, sizeof(le32_size)); - if (!ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &le32_size, sizeof(le32_size))); return oc::success(); } @@ -90,22 +82,13 @@ _mtk_compute_sha1(Writer &writer, SegmentWriter &seg, for (auto const &entry : seg.entries()) { uint64_t remain = *entry.size; - auto seek_ret = file.seek(static_cast(entry.offset), - SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(entry.offset), SEEK_SET)); // Update checksum with data while (remain > 0) { auto to_read = std::min(remain, sizeof(buf)); - auto ret = file_read_exact(file, buf, static_cast(to_read)); - if (!ret) { - if (writer.is_fatal()) { writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, buf, static_cast(to_read))); if (!SHA1_Update(&sha_ctx, buf, static_cast(to_read))) { return android::AndroidError::Sha1UpdateError; @@ -194,18 +177,10 @@ oc::result MtkFormatWriter::close(File &file) // If successful, finish up the boot image if (swentry == m_seg->entries().end()) { - auto file_size = file.seek(0, SEEK_CUR); - if (!file_size) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return file_size.as_failure(); - } + OUTCOME_TRY(file_size, file.seek(0, SEEK_CUR)); // Truncate to set size - auto truncate_ret = file.truncate(file_size.value()); - if (!truncate_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return truncate_ret.as_failure(); - } + OUTCOME_TRYV(file.truncate(file_size)); // Update MTK header sizes for (auto const &entry : m_seg->entries()) { @@ -234,18 +209,10 @@ oc::result MtkFormatWriter::close(File &file) android_fix_header_byte_order(m_hdr); // Seek back to beginning to write header - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); // Write header - auto ret = file_write_exact(file, &m_hdr, sizeof(m_hdr)); - if (!ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, &m_hdr, sizeof(m_hdr))); } } @@ -332,11 +299,7 @@ oc::result MtkFormatWriter::write_header(File &file, const Header &header) OUTCOME_TRYV(m_seg->set_entries(std::move(entries))); // Start writing after first page - auto seek_ret = file.seek(m_hdr.page_size, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(m_hdr.page_size, SEEK_SET)); return oc::success(); } diff --git a/libmbbootimg/src/format/segment_reader.cpp b/libmbbootimg/src/format/segment_reader.cpp index 08311e2f9..68491c379 100644 --- a/libmbbootimg/src/format/segment_reader.cpp +++ b/libmbbootimg/src/format/segment_reader.cpp @@ -75,11 +75,8 @@ oc::result SegmentReader::move_to_entry(File &file, Entry &entry, uint64_t read_cur_offset = read_start_offset; if (m_read_cur_offset != srentry->offset) { - auto ret = file.seek(static_cast(read_start_offset), SEEK_SET); - if (!ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(read_start_offset), + SEEK_SET)); } entry.set_type(srentry->type); @@ -154,16 +151,12 @@ oc::result SegmentReader::read_data(File &file, void *buf, } // We allow truncation for certain things, so we can't use file_read_exact() - auto n = file_read_retry(file, buf, to_copy); - if (!n) { - if (file.is_fatal()) { reader.set_fatal(); } - return n.as_failure(); - } + OUTCOME_TRY(n, file_read_retry(file, buf, to_copy)); - m_read_cur_offset += n.value(); + m_read_cur_offset += n; // Fail if we reach EOF early - if (n.value() < to_copy && m_read_cur_offset != m_read_end_offset + if (n < to_copy && m_read_cur_offset != m_read_end_offset && !m_entry->can_truncate) { //DEBUG("Entry is truncated (expected %" PRIu64 " more bytes)", // m_read_end_offset - m_read_cur_offset); @@ -171,7 +164,7 @@ oc::result SegmentReader::read_data(File &file, void *buf, return FileError::UnexpectedEof; } - return n.value(); + return n; } } diff --git a/libmbbootimg/src/format/segment_writer.cpp b/libmbbootimg/src/format/segment_writer.cpp index 3ed1b389e..70bbae060 100644 --- a/libmbbootimg/src/format/segment_writer.cpp +++ b/libmbbootimg/src/format/segment_writer.cpp @@ -78,13 +78,9 @@ oc::result SegmentWriter::get_entry(File &file, Entry &entry, Writer &writer) { if (!m_pos) { - auto pos = file.seek(0, SEEK_CUR); - if (!pos) { - if (file.is_fatal()) { writer.set_fatal(); } - return pos.as_failure(); - } + OUTCOME_TRY(pos, file.seek(0, SEEK_CUR)); - m_pos = pos.value(); + m_pos = pos; } // Get next entry @@ -168,13 +164,9 @@ oc::result SegmentWriter::finish_entry(File &file, Writer &writer) if (m_entry->align > 0) { auto skip = align_page_size(*m_pos, m_entry->align); - auto new_pos = file.seek(static_cast(skip), SEEK_CUR); - if (!new_pos) { - if (file.is_fatal()) { writer.set_fatal(); } - return new_pos.as_failure(); - } + OUTCOME_TRY(new_pos, file.seek(static_cast(skip), SEEK_CUR)); - m_pos = new_pos.value(); + m_pos = new_pos; } return oc::success(); diff --git a/libmbbootimg/src/format/sony_elf_reader.cpp b/libmbbootimg/src/format/sony_elf_reader.cpp index 2bba47a5d..a95f164d9 100644 --- a/libmbbootimg/src/format/sony_elf_reader.cpp +++ b/libmbbootimg/src/format/sony_elf_reader.cpp @@ -129,19 +129,9 @@ oc::result SonyElfFormatReader::read_header(File &file, Header &header) for (Elf32_Half i = 0; i < m_hdr.e_phnum; ++i) { Sony_Elf32_Phdr phdr; - auto seek_ret = file.seek(static_cast(pos), SEEK_SET); - if (!seek_ret) { - //DEBUG("Failed to seek to segment %" PRIu16 " at %" PRIu64, i, pos); - if (file.is_fatal()) { m_reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(static_cast(pos), SEEK_SET)); - auto ret = file_read_exact(file, &phdr, sizeof(phdr)); - if (!ret) { - //DEBUG("Failed to read segment %" PRIu16, i); - if (file.is_fatal()) { m_reader.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, &phdr, sizeof(phdr))); // Account for program header pos += sizeof(phdr); @@ -157,19 +147,9 @@ oc::result SonyElfFormatReader::read_header(File &file, Header &header) return SonyElfError::KernelCmdlineTooLong; } - seek_ret = file.seek(phdr.p_offset, SEEK_SET); - if (!seek_ret) { - //DEBUG("Failed to seek to cmdline"); - if (file.is_fatal()) { m_reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(phdr.p_offset, SEEK_SET)); - ret = file_read_exact(file, cmdline, phdr.p_memsz); - if (!ret) { - //DEBUG("Failed to read cmdline"); - if (file.is_fatal()) { m_reader.set_fatal(); }; - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(file, cmdline, phdr.p_memsz)); cmdline[phdr.p_memsz] = '\0'; @@ -268,18 +248,13 @@ SonyElfFormatReader::find_sony_elf_header(Reader &reader, File &file, { Sony_Elf32_Ehdr header; - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { reader.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); auto ret = file_read_exact(file, &header, sizeof(header)); if (!ret) { if (ret.error() == FileError::UnexpectedEof) { return SonyElfError::SonyElfHeaderTooSmall; } else { - if (file.is_fatal()) { reader.set_fatal(); } return ret.as_failure(); } } diff --git a/libmbbootimg/src/format/sony_elf_writer.cpp b/libmbbootimg/src/format/sony_elf_writer.cpp index 40bc454e7..e034ce7dc 100644 --- a/libmbbootimg/src/format/sony_elf_writer.cpp +++ b/libmbbootimg/src/format/sony_elf_writer.cpp @@ -122,20 +122,12 @@ oc::result SonyElfFormatWriter::close(File &file) sony_elf_fix_phdr_byte_order(m_hdr_appsbl); // Seek back to beginning to write headers - auto seek_ret = file.seek(0, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(0, SEEK_SET)); // Write headers for (auto const &header : headers) { if (header.can_write) { - auto ret = file_write_exact(file, header.ptr, header.size); - if (!ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return ret.as_failure(); - } + OUTCOME_TRYV(file_write_exact(file, header.ptr, header.size)); } } } @@ -259,11 +251,7 @@ oc::result SonyElfFormatWriter::write_header(File &file, OUTCOME_TRYV(m_seg->set_entries(std::move(entries))); // Start writing at offset 4096 - auto seek_ret = file.seek(4096, SEEK_SET); - if (!seek_ret) { - if (file.is_fatal()) { m_writer.set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file.seek(4096, SEEK_SET)); return oc::success(); } diff --git a/libmbbootimg/src/reader.cpp b/libmbbootimg/src/reader.cpp index 324b00a23..aa121f22d 100644 --- a/libmbbootimg/src/reader.cpp +++ b/libmbbootimg/src/reader.cpp @@ -381,11 +381,7 @@ oc::result Reader::open(File *file) if (!m_format) { for (auto &f : m_formats) { // Seek to beginning - auto seek_ret = file->seek(0, SEEK_SET); - if (!seek_ret) { - if (file->is_fatal()) { set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(file->seek(0, SEEK_SET)); auto close_f = finally([&] { (void) f->close(*file); @@ -480,11 +476,7 @@ oc::result Reader::read_header(Header &header) ENSURE_STATE_OR_RETURN_ERROR(ReaderState::Header); // Seek to beginning - auto seek_ret = m_file->seek(0, SEEK_SET); - if (!seek_ret) { - if (m_file->is_fatal()) { set_fatal(); } - return seek_ret.as_failure(); - } + OUTCOME_TRYV(m_file->seek(0, SEEK_SET)); header.clear(); diff --git a/libmbcommon/include/mbcommon/file.h b/libmbcommon/include/mbcommon/file.h index 97f52c62e..5ec342d2e 100644 --- a/libmbcommon/include/mbcommon/file.h +++ b/libmbcommon/include/mbcommon/file.h @@ -53,8 +53,6 @@ class MB_EXPORT File // File state bool is_open(); - bool is_fatal(); - void set_fatal(); protected: File(File &&other) noexcept; diff --git a/libmbcommon/include/mbcommon/file_p.h b/libmbcommon/include/mbcommon/file_p.h index 36c9530c2..2c6becdc2 100644 --- a/libmbcommon/include/mbcommon/file_p.h +++ b/libmbcommon/include/mbcommon/file_p.h @@ -31,8 +31,7 @@ enum class FileState : uint8_t { New = 1u << 0, Opened = 1u << 1, - Fatal = 1u << 2, - Moved = 1u << 3, + Moved = 1u << 2, }; MB_DECLARE_FLAGS(FileStates, FileState) MB_DECLARE_OPERATORS_FOR_FLAGS(FileStates) diff --git a/libmbcommon/src/file.cpp b/libmbcommon/src/file.cpp index fa187f767..8b62d53d8 100644 --- a/libmbcommon/src/file.cpp +++ b/libmbcommon/src/file.cpp @@ -296,34 +296,6 @@ bool File::is_open() return m_state == FileState::Opened; } -/*! - * \brief Check whether file is fatal state - * - * If the file is in the fatal state, the only valid operation is to call - * close(). - * - * \return Whether file is in fatal state - */ -bool File::is_fatal() -{ - return m_state == FileState::Fatal; -} - -/*! - * \brief Set whether file is fatal state - * - * This function only has an effect if the file is opened. - * - * If the file is in the fatal state, the only valid operation is to call - * close(). - */ -void File::set_fatal() -{ - if (m_state == FileState::Opened) { - m_state = FileState::Fatal; - } -} - /*! * \brief Get current state of the File handle * @@ -378,7 +350,7 @@ oc::result File::on_open() * * * open() fails * * close() is explicitly called, regardless if the file handle is in the - * opened or fatal state + * opened state * * the file handle is destroyed * * This method should return: diff --git a/libmbcommon/src/file/callbacks.cpp b/libmbcommon/src/file/callbacks.cpp index 452c42394..82c56635c 100644 --- a/libmbcommon/src/file/callbacks.cpp +++ b/libmbcommon/src/file/callbacks.cpp @@ -53,8 +53,7 @@ using namespace detail; * \brief File close callback * * This callback, if registered, will be called once and only once once to clean - * up the resources, regardless of the current state. In other words, this - * callback will be called even if is_fatal() returns true. If any memory, file + * up the resources, regardless of the current state. If any memory, file * handles, or other resources need to be freed, this callback is the place to * do so. * diff --git a/libmbcommon/src/file_util.cpp b/libmbcommon/src/file_util.cpp index 6b00755a4..86df5f978 100644 --- a/libmbcommon/src/file_util.cpp +++ b/libmbcommon/src/file_util.cpp @@ -341,7 +341,6 @@ oc::result file_search(File &file, if (discarded != offset) { // Reached EOF before starting offset - file.set_fatal(); return FileError::ArgumentOutOfRange; } } else { diff --git a/libmbcommon/tests/test_file.cpp b/libmbcommon/tests/test_file.cpp index 4f6c9cd1e..3509378ad 100644 --- a/libmbcommon/tests/test_file.cpp +++ b/libmbcommon/tests/test_file.cpp @@ -180,23 +180,6 @@ TEST(FileTest, FreeClosedFile) ASSERT_EQ(counters.n_close, 1u); } -TEST(FileTest, FreeFatalFile) -{ - TestFileCounters counters; - - { - NiceMock file(&counters); - - // Open file - ASSERT_TRUE(file.open()); - - file.set_state(FileState::Fatal); - } - - // Ensure that the close callback was called - ASSERT_EQ(counters.n_close, 1u); -} - TEST(FileTest, OpenReturnFailure) { NiceMock file; @@ -478,6 +461,5 @@ TEST(FileTest, TruncateReturnFailure) // Truncate file ASSERT_FALSE(file.truncate(INITIAL_BUF_SIZE + 1)); - ASSERT_FALSE(file.is_fatal()); ASSERT_EQ(file.state(), FileState::Opened); } diff --git a/libmbsparse/src/sparse.cpp b/libmbsparse/src/sparse.cpp index ca7a61208..84b4ea6f4 100644 --- a/libmbsparse/src/sparse.cpp +++ b/libmbsparse/src/sparse.cpp @@ -531,11 +531,7 @@ void SparseFile::clear() oc::result SparseFile::wread(void *buf, size_t size) { - auto ret = file_read_exact(*m_file, buf, size); - if (!ret) { - set_fatal(); - return ret.as_failure(); - } + OUTCOME_TRYV(file_read_exact(*m_file, buf, size)); m_cur_src_offset += size; return oc::success(); @@ -573,7 +569,6 @@ oc::result SparseFile::skip_bytes(uint64_t bytes) if (discarded != bytes) { DEBUG("Reached EOF when skipping bytes"); - set_fatal(); return FileError::UnexpectedEof; } @@ -914,7 +909,6 @@ oc::result SparseFile::move_to_chunk(uint64_t offset) DEBUG("Internal error: src_begin (%" PRIu64 ")" " < cur_src_offset (%" PRIu64 ")", src_begin, m_cur_src_offset); - set_fatal(); return SparseFileError::InternalError; } @@ -922,7 +916,6 @@ oc::result SparseFile::move_to_chunk(uint64_t offset) if (!skip_ret) { DEBUG("Failed to skip to chunk #%" MB_PRIzu ": %s", chunk_num, skip_ret.error().message().c_str()); - set_fatal(); return skip_ret.as_failure(); } @@ -932,7 +925,6 @@ oc::result SparseFile::move_to_chunk(uint64_t offset) if (!read_ret) { DEBUG("Failed to read chunk header for chunk %" MB_PRIzu ": %s", chunk_num, read_ret.error().message().c_str()); - set_fatal(); return read_ret.as_failure(); } @@ -949,32 +941,25 @@ oc::result SparseFile::move_to_chunk(uint64_t offset) DEBUG("Failed to skip extra bytes in chunk #%" MB_PRIzu "'s header: %s", chunk_num, skip_ret.error().message().c_str()); - set_fatal(); return skip_ret.as_failure(); } - auto chunk_info = process_chunk(chdr, tgt_begin); - if (!chunk_info) { - set_fatal(); - return chunk_info.as_failure(); - } - auto &&ci = chunk_info.value(); + OUTCOME_TRY(chunk_info, process_chunk(chdr, tgt_begin)); OPER("Chunk #%" MB_PRIzu " covers source range (%" PRIu64 " - %" PRIu64 ")", - chunk_num, ci.src_begin, ci.src_end); + chunk_num, chunk_info.src_begin, chunk_info.src_end); OPER("Chunk #%" MB_PRIzu " covers output range (%" PRIu64 " - %" PRIu64 ")", - chunk_num, ci.begin, ci.end); + chunk_num, chunk_info.begin, chunk_info.end); // Make sure the chunk does not end after the header-specified file size - if (ci.end > m_file_size) { + if (chunk_info.end > m_file_size) { DEBUG("Chunk #%" MB_PRIzu " ends (%" PRIu64 ") after the file size " "specified in the sparse header (%" PRIu64 ")", - chunk_num, ci.end, m_file_size); - set_fatal(); + chunk_num, chunk_info.end, m_file_size); return SparseFileError::InvalidChunkBounds; } - m_chunks.push_back(std::move(ci)); + m_chunks.push_back(std::move(chunk_info)); // If we just read the last chunk, make sure it ends at the same // position as specified in the sparse header @@ -983,7 +968,6 @@ oc::result SparseFile::move_to_chunk(uint64_t offset) DEBUG("Last chunk does not end (%" PRIu64 ") at position" " specified by sparse header (%" PRIu64 ")", m_chunks.back().end, m_file_size); - set_fatal(); return SparseFileError::InvalidChunkBounds; } diff --git a/libmbsparse/tests/test_sparse.cpp b/libmbsparse/tests/test_sparse.cpp index 788c70205..fdc4feb8b 100644 --- a/libmbsparse/tests/test_sparse.cpp +++ b/libmbsparse/tests/test_sparse.cpp @@ -344,7 +344,6 @@ TEST_F(SparseTest, CheckInvalidRawChunkFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidRawChunk); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckInvalidFillChunkFatal) @@ -378,7 +377,6 @@ TEST_F(SparseTest, CheckInvalidFillChunkFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidFillChunk); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckInvalidSkipChunkFatal) @@ -412,7 +410,6 @@ TEST_F(SparseTest, CheckInvalidSkipChunkFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidSkipChunk); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckInvalidCrc32ChunkFatal) @@ -446,7 +443,6 @@ TEST_F(SparseTest, CheckInvalidCrc32ChunkFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidCrc32Chunk); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckInvalidChunkTotalSizeFatal) @@ -480,7 +476,6 @@ TEST_F(SparseTest, CheckInvalidChunkTotalSizeFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidChunkSize); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckInvalidChunkTypeFatal) @@ -514,7 +509,6 @@ TEST_F(SparseTest, CheckInvalidChunkTypeFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidChunkType); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckReadTruncatedChunkHeaderFatal) @@ -548,7 +542,6 @@ TEST_F(SparseTest, CheckReadTruncatedChunkHeaderFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), FileError::UnexpectedEof); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckReadOversizedChunkDataFatal) @@ -583,7 +576,6 @@ TEST_F(SparseTest, CheckReadOversizedChunkDataFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidChunkBounds); - ASSERT_TRUE(_file.is_fatal()); } TEST_F(SparseTest, CheckReadUndersizedChunkDataFatal) @@ -618,7 +610,6 @@ TEST_F(SparseTest, CheckReadUndersizedChunkDataFatal) auto n = _file.read(buf, sizeof(buf)); ASSERT_FALSE(n); ASSERT_EQ(n.error(), SparseFileError::InvalidChunkBounds); - ASSERT_TRUE(_file.is_fatal()); } // All further tests use a valid sparse file