From 39f03aee7980a209578c2832fa094a68fd9c1cc3 Mon Sep 17 00:00:00 2001 From: Lukas Lendvorsky Date: Fri, 4 Aug 2023 15:54:02 +0200 Subject: [PATCH 1/8] libgcode: fix warning of not checking result of fread --- src/core/core.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 907f015..c375b34 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -13,8 +13,8 @@ static bool write_to_file(FILE& file, const void* data, size_t data_size) static bool read_from_file(FILE& file, void* data, size_t data_size) { - fread(data, 1, data_size, &file); - return !ferror(&file); + size_t rsize = fread(data, 1, data_size, &file); + return !ferror(&file) && rsize == data_size; } static uint32_t crc32_sw(const uint8_t* buffer, uint32_t length, uint32_t crc) @@ -67,7 +67,7 @@ static EResult checksums_match(FILE& file, const FileHeader& file_header, const // propagate error return res; - // Verify checksum + // Verify checksum if (!curr_cs.matches(read_cs)) return EResult::InvalidChecksum; @@ -284,8 +284,8 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents // check magic number std::array magic; - fread((void*)magic.data(), 1, magic.size(), &file); - if (ferror(&file)) + size_t hsize = fread((void*)magic.data(), 1, magic.size(), &file); + if (ferror(&file) && hsize != magic.size()) return EResult::ReadError; else if (magic != MAGIC) { // restore file position From 45b32717295112bae9ebf8eba15e103c9428df19 Mon Sep 17 00:00:00 2001 From: enricoturri1966 Date: Tue, 8 Aug 2023 09:06:06 +0200 Subject: [PATCH 2/8] Follow-up of 4142152135e7fc23162415f4d1ef887f538f8d58 - check results of fread() --- src/binarize/binarize.cpp | 4 ++-- src/core/core.cpp | 6 +++--- tests/core/core_tests.cpp | 20 ++++++++++---------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/binarize/binarize.cpp b/src/binarize/binarize.cpp index 760398a..c5786ad 100644 --- a/src/binarize/binarize.cpp +++ b/src/binarize/binarize.cpp @@ -22,8 +22,8 @@ static bool write_to_file(FILE& file, const void* data, size_t data_size) static bool read_from_file(FILE& file, void* data, size_t data_size) { - fread(data, 1, data_size, &file); - return !ferror(&file); + const size_t rsize = fread(data, 1, data_size, &file); + return !ferror(&file) && rsize == data_size; } static std::vector encode(const void* data, size_t data_size) diff --git a/src/core/core.cpp b/src/core/core.cpp index c375b34..9e72e2c 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -13,7 +13,7 @@ static bool write_to_file(FILE& file, const void* data, size_t data_size) static bool read_from_file(FILE& file, void* data, size_t data_size) { - size_t rsize = fread(data, 1, data_size, &file); + const size_t rsize = fread(data, 1, data_size, &file); return !ferror(&file) && rsize == data_size; } @@ -284,8 +284,8 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents // check magic number std::array magic; - size_t hsize = fread((void*)magic.data(), 1, magic.size(), &file); - if (ferror(&file) && hsize != magic.size()) + const size_t rsize = fread((void*)magic.data(), 1, magic.size(), &file); + if (ferror(&file) && rsize != magic.size()) return EResult::ReadError; else if (magic != MAGIC) { // restore file position diff --git a/tests/core/core_tests.cpp b/tests/core/core_tests.cpp index a1f78fb..5a4493b 100644 --- a/tests/core/core_tests.cpp +++ b/tests/core/core_tests.cpp @@ -130,8 +130,8 @@ TEST_CASE("Checksum max cache size", "[Core]") { const long curr_pos = ftell(file); uint16_t encoding; - fread(&encoding, 1, sizeof(encoding), file); - REQUIRE(ferror(file) == 0); + const size_t rsize = fread(&encoding, 1, sizeof(encoding), file); + REQUIRE((ferror(file) == 0 && rsize == sizeof(encoding))); fseek(file, curr_pos, SEEK_SET); std::cout << " - encoding: " << metadata_encoding_as_string((EMetadataEncodingType)encoding); break; @@ -140,8 +140,8 @@ TEST_CASE("Checksum max cache size", "[Core]") { const long curr_pos = ftell(file); uint16_t encoding; - fread(&encoding, 1, sizeof(encoding), file); - REQUIRE(ferror(file) == 0); + const size_t rsize = fread(&encoding, 1, sizeof(encoding), file); + REQUIRE((ferror(file) == 0 && rsize == sizeof(encoding))); fseek(file, curr_pos, SEEK_SET); std::cout << " - encoding: " << gcode_encoding_as_string((EGCodeEncodingType)encoding); break; @@ -150,14 +150,14 @@ TEST_CASE("Checksum max cache size", "[Core]") { const long curr_pos = ftell(file); uint16_t format; - fread(&format, 1, sizeof(format), file); - REQUIRE(ferror(file) == 0); + size_t rsize = fread(&format, 1, sizeof(format), file); + REQUIRE((ferror(file) == 0 && rsize == sizeof(format))); uint16_t width; - fread(&width, 1, sizeof(width), file); - REQUIRE(ferror(file) == 0); + rsize = fread(&width, 1, sizeof(width), file); + REQUIRE((ferror(file) == 0 && rsize == sizeof(width))); uint16_t height; - fread(&height, 1, sizeof(height), file); - REQUIRE(ferror(file) == 0); + rsize = fread(&height, 1, sizeof(height), file); + REQUIRE((ferror(file) == 0 && rsize == sizeof(height))); fseek(file, curr_pos, SEEK_SET); std::cout << " - format: " << thumbnail_format_as_string((EThumbnailFormat)format); std::cout << " (size: " << width << "x" << height << ")"; From 5948c51ed3582dd3f63bdf55b4904d512a3e9947 Mon Sep 17 00:00:00 2001 From: Lukas Lendvorsky Date: Fri, 4 Aug 2023 15:54:21 +0200 Subject: [PATCH 3/8] libgcode: remember position of block --- src/core/core.cpp | 4 +++- src/core/core.hpp | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index 9e72e2c..3a38cdc 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -201,8 +201,9 @@ void BlockHeader::update_checksum(Checksum& checksum) const checksum.append(encode((const void*)&compressed_size, sizeof(compressed_size))); } -EResult BlockHeader::write(FILE& file) const +EResult BlockHeader::write(FILE& file) { + position = ftell(&file); if (!write_to_file(file, (const void*)&type, sizeof(type))) return EResult::WriteError; if (!write_to_file(file, (const void*)&compression, sizeof(compression))) @@ -218,6 +219,7 @@ EResult BlockHeader::write(FILE& file) const EResult BlockHeader::read(FILE& file) { + position = ftell(&file); if (!read_from_file(file, (void*)&type, sizeof(type))) return EResult::ReadError; if (type >= block_types_count()) diff --git a/src/core/core.hpp b/src/core/core.hpp index 4d09c65..f902eed 100644 --- a/src/core/core.hpp +++ b/src/core/core.hpp @@ -96,7 +96,7 @@ class Checksum EChecksumType get_type() const; - // Appends the given data to the cache and performs a checksum update if + // Appends the given data to the cache and performs a checksum update if // the size of the cache exceeds the max checksum cache size. void append(const std::vector& data); // Returns true if the given checksum is equal to this one @@ -125,6 +125,7 @@ struct FileHeader struct BlockHeader { + long int position{ 0 }; uint16_t type{ 0 }; uint16_t compression{ 0 }; uint32_t uncompressed_size{ 0 }; @@ -133,7 +134,7 @@ struct BlockHeader // Updates the given checksum with the data of this BlockHeader void update_checksum(Checksum& checksum) const; - EResult write(FILE& file) const; + EResult write(FILE& file); EResult read(FILE& file); }; From d1446de27dda6a622994626d08d577e5d7478d08 Mon Sep 17 00:00:00 2001 From: enricoturri1966 Date: Tue, 8 Aug 2023 09:40:26 +0200 Subject: [PATCH 4/8] Follow-up of 51bb14b06097f09408a49cef874949f55fb0e102 - BlockHeader position set as private member --- src/binarize/binarize.cpp | 6 +++--- src/core/core.cpp | 18 +++++++++++++++--- src/core/core.hpp | 15 ++++++++++++--- 3 files changed, 30 insertions(+), 9 deletions(-) diff --git a/src/binarize/binarize.cpp b/src/binarize/binarize.cpp index c5786ad..2ba995a 100644 --- a/src/binarize/binarize.cpp +++ b/src/binarize/binarize.cpp @@ -378,7 +378,7 @@ EResult BaseMetadataBlock::write(FILE& file, EBlockType block_type, ECompression if (encoding_type > metadata_encoding_types_count()) return EResult::InvalidMetadataEncodingType; - BlockHeader block_header = { (uint16_t)block_type, (uint16_t)compression_type, (uint32_t)0 }; + BlockHeader block_header((uint16_t)block_type, (uint16_t)compression_type, (uint32_t)0); std::vector out_data; if (!raw_data.empty()) { // process payload encoding @@ -573,7 +573,7 @@ EResult ThumbnailBlock::write(FILE& file, EChecksumType checksum_type) const return EResult::InvalidThumbnailDataSize; // write block header - const BlockHeader block_header = { (uint16_t)EBlockType::Thumbnail, (uint16_t)ECompressionType::None, (uint32_t)data.size() }; + const BlockHeader block_header((uint16_t)EBlockType::Thumbnail, (uint16_t)ECompressionType::None, (uint32_t)data.size()); EResult res = block_header.write(file); if (res != EResult::Success) // propagate error @@ -651,7 +651,7 @@ EResult GCodeBlock::write(FILE& file, ECompressionType compression_type, EChecks if (encoding_type > gcode_encoding_types_count()) return EResult::InvalidGCodeEncodingType; - BlockHeader block_header = { (uint16_t)EBlockType::GCode, (uint16_t)compression_type, (uint32_t)0 }; + BlockHeader block_header((uint16_t)EBlockType::GCode, (uint16_t)compression_type, (uint32_t)0); std::vector out_data; if (!raw_data.empty()) { // process payload encoding diff --git a/src/core/core.cpp b/src/core/core.cpp index 3a38cdc..cfdaca6 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -192,6 +192,13 @@ EResult FileHeader::read(FILE& file, const uint32_t* const max_version) return EResult::Success; } +BlockHeader::BlockHeader(uint16_t type, uint16_t compression, uint32_t uncompressed_size, uint32_t compressed_size) + : type(type) + , compression(compression) + , uncompressed_size(uncompressed_size) + , compressed_size(compressed_size) +{} + void BlockHeader::update_checksum(Checksum& checksum) const { checksum.append(encode((const void*)&type, sizeof(type))); @@ -201,9 +208,14 @@ void BlockHeader::update_checksum(Checksum& checksum) const checksum.append(encode((const void*)&compressed_size, sizeof(compressed_size))); } -EResult BlockHeader::write(FILE& file) +long BlockHeader::get_position() const +{ + return m_position; +} + +EResult BlockHeader::write(FILE& file) const { - position = ftell(&file); + m_position = ftell(&file); if (!write_to_file(file, (const void*)&type, sizeof(type))) return EResult::WriteError; if (!write_to_file(file, (const void*)&compression, sizeof(compression))) @@ -219,7 +231,7 @@ EResult BlockHeader::write(FILE& file) EResult BlockHeader::read(FILE& file) { - position = ftell(&file); + m_position = ftell(&file); if (!read_from_file(file, (void*)&type, sizeof(type))) return EResult::ReadError; if (type >= block_types_count()) diff --git a/src/core/core.hpp b/src/core/core.hpp index f902eed..36ce844 100644 --- a/src/core/core.hpp +++ b/src/core/core.hpp @@ -125,17 +125,26 @@ struct FileHeader struct BlockHeader { - long int position{ 0 }; uint16_t type{ 0 }; uint16_t compression{ 0 }; uint32_t uncompressed_size{ 0 }; uint32_t compressed_size{ 0 }; - // Updates the given checksum with the data of this BlockHeader + BlockHeader() = default; + BlockHeader(uint16_t type, uint16_t compression, uint32_t uncompressed_size, uint32_t compressed_size = 0); + + // Updates the given checksum with the data of this BlockHeader. void update_checksum(Checksum& checksum) const; - EResult write(FILE& file); + // Returns the position of this block in the file. + // Position is set by calling write() and read() methods. + long get_position() const; + + EResult write(FILE& file) const; EResult read(FILE& file); + +private: + mutable long m_position{ 0 }; }; // Returns a string description of the given result From 34ca8dbecb33ca9887df70d8b4216d8eb400c544 Mon Sep 17 00:00:00 2001 From: Lukas Lendvorsky Date: Fri, 4 Aug 2023 17:24:42 +0200 Subject: [PATCH 5/8] libgcode: store position of blocks and use that to navigate gcode Fixed conflict after cherry-pick --- src/convert/convert.cpp | 4 ++-- src/core/core.cpp | 26 ++++++++++++-------------- src/core/core.hpp | 10 +++------- tests/core/core_tests.cpp | 4 ++-- 4 files changed, 19 insertions(+), 25 deletions(-) diff --git a/src/convert/convert.cpp b/src/convert/convert.cpp index 3e38b60..b908e01 100644 --- a/src/convert/convert.cpp +++ b/src/convert/convert.cpp @@ -7,7 +7,7 @@ #include #include -namespace bgcode { +namespace bgcode { using namespace core; using namespace binarize; namespace convert { @@ -661,7 +661,7 @@ BGCODE_CONVERT_EXPORT EResult from_binary_to_ascii(FILE& src_file, FILE& dst_fil if (!write_line("\n")) return EResult::WriteError; - res = skip_block_content(src_file, file_header, block_header); + res = skip_block(src_file, file_header, block_header); if (res != EResult::Success) // propagate error return res; diff --git a/src/core/core.cpp b/src/core/core.cpp index cfdaca6..a324e0f 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -252,6 +252,10 @@ EResult BlockHeader::read(FILE& file) return EResult::Success; } +size_t BlockHeader::get_size() const { + return sizeof(type) + sizeof(compression) + sizeof(uncompressed_size) + ((compression == (uint16_t)ECompressionType::None)? 0 : sizeof(compressed_size)); +} + BGCODE_CORE_EXPORT size_t get_checksum_max_cache_size() { return g_checksum_max_cache_size; } BGCODE_CORE_EXPORT void set_checksum_max_cache_size(size_t size) { g_checksum_max_cache_size = size; } @@ -338,7 +342,7 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents } // read printer metadata block header - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) { // restore file position fseek(&file, curr_pos, SEEK_SET); @@ -359,7 +363,7 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents } // read thumbnails block headers, if present - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) { // restore file position fseek(&file, curr_pos, SEEK_SET); @@ -374,7 +378,7 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents return res; } while ((EBlockType)block_header.type == EBlockType::Thumbnail) { - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) { // restore file position fseek(&file, curr_pos, SEEK_SET); @@ -398,7 +402,7 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents } // read slicer metadata block header - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) { // restore file position fseek(&file, curr_pos, SEEK_SET); @@ -420,7 +424,7 @@ BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents // read gcode block headers do { - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) { // restore file position fseek(&file, curr_pos, SEEK_SET); @@ -498,7 +502,7 @@ BGCODE_CORE_EXPORT EResult read_next_block_header(FILE& file, const FileHeader& } if (!feof(&file)) { - res = skip_block_content(file, file_header, block_header); + res = skip_block(file, file_header, block_header); if (res != EResult::Success) // propagate error return res; @@ -522,15 +526,9 @@ BGCODE_CORE_EXPORT size_t block_parameters_size(EBlockType type) return 0; } -BGCODE_CORE_EXPORT EResult skip_block_payload(FILE& file, const BlockHeader& block_header) -{ - fseek(&file, (long)block_payload_size(block_header), SEEK_CUR); - return ferror(&file) ? EResult::ReadError : EResult::Success; -} - -BGCODE_CORE_EXPORT EResult skip_block_content(FILE& file, const FileHeader& file_header, const BlockHeader& block_header) +BGCODE_CORE_EXPORT EResult skip_block(FILE& file, const FileHeader& file_header, const BlockHeader& block_header) { - fseek(&file, (long)block_content_size(file_header, block_header), SEEK_CUR); + fseek(&file, block_header.position + block_header.get_size() + block_content_size(file_header, block_header), SEEK_SET); return ferror(&file) ? EResult::ReadError : EResult::Success; } diff --git a/src/core/core.hpp b/src/core/core.hpp index 36ce844..c492358 100644 --- a/src/core/core.hpp +++ b/src/core/core.hpp @@ -143,6 +143,8 @@ struct BlockHeader EResult write(FILE& file) const; EResult read(FILE& file); + size_t get_size() const; + private: mutable long m_position{ 0 }; }; @@ -183,17 +185,11 @@ extern BGCODE_CORE_EXPORT EResult read_next_block_header(FILE& file, const FileH // - file position will keep the current value extern BGCODE_CORE_EXPORT EResult read_next_block_header(FILE& file, const FileHeader& file_header, BlockHeader& block_header, EBlockType type, bool verify_checksum); -// Skips the payload (parameters + data) of the block with the given block header. -// File position must be at the start of the block parameters. -// If return == EResult::Success: -// - file position will be set at the start of the block checksum, if present, or of next block header -extern BGCODE_CORE_EXPORT EResult skip_block_payload(FILE& file, const BlockHeader& block_header); - // Skips the content (parameters + data + checksum) of the block with the given block header. // File position must be at the start of the block parameters. // If return == EResult::Success: // - file position will be set at the start of the next block header -extern BGCODE_CORE_EXPORT EResult skip_block_content(FILE& file, const FileHeader& file_header, const BlockHeader& block_header); +extern BGCODE_CORE_EXPORT EResult skip_block(FILE& file, const FileHeader& file_header, const BlockHeader& block_header); // Returns the size of the parameters of the given block type, in bytes. extern BGCODE_CORE_EXPORT size_t block_parameters_size(EBlockType type); diff --git a/tests/core/core_tests.cpp b/tests/core/core_tests.cpp index 5a4493b..5e2b2c1 100644 --- a/tests/core/core_tests.cpp +++ b/tests/core/core_tests.cpp @@ -169,7 +169,7 @@ TEST_CASE("Checksum max cache size", "[Core]") std::cout << "\n"; // move to next block header - REQUIRE(skip_block_content(*file, file_header, block_header) == EResult::Success); + REQUIRE(skip_block(*file, file_header, block_header) == EResult::Success); if (ftell(file) == file_size) break; } while (true); @@ -206,7 +206,7 @@ TEST_CASE("Checksum max cache size", "[Core]") std::cout << "Block type: " << block_type_as_string((EBlockType)block_header.type) << "\n"; // move to next block header - REQUIRE(skip_block_content(*file, file_header, block_header) == EResult::Success); + REQUIRE(skip_block(*file, file_header, block_header) == EResult::Success); if (ftell(file) == file_size) break; } while (true); From ac5d757795428498a5953cfea40747f773a622a9 Mon Sep 17 00:00:00 2001 From: enricoturri1966 Date: Tue, 8 Aug 2023 09:52:15 +0200 Subject: [PATCH 6/8] Follow-up of 75764d9b43ef8069d736659cae91c4eff12b9e11 - Reintroduced function core::skip_block_content() --- src/core/core.cpp | 8 +++++++- src/core/core.hpp | 30 ++++++++++++++++++------------ 2 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/core/core.cpp b/src/core/core.cpp index a324e0f..1277381 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -526,9 +526,15 @@ BGCODE_CORE_EXPORT size_t block_parameters_size(EBlockType type) return 0; } +BGCODE_CORE_EXPORT EResult skip_block_content(FILE& file, const FileHeader& file_header, const BlockHeader& block_header) +{ + fseek(&file, (long)block_content_size(file_header, block_header), SEEK_CUR); + return ferror(&file) ? EResult::ReadError : EResult::Success; +} + BGCODE_CORE_EXPORT EResult skip_block(FILE& file, const FileHeader& file_header, const BlockHeader& block_header) { - fseek(&file, block_header.position + block_header.get_size() + block_content_size(file_header, block_header), SEEK_SET); + fseek(&file, block_header.get_position() + (long)block_header.get_size() + (long)block_content_size(file_header, block_header), SEEK_SET); return ferror(&file) ? EResult::ReadError : EResult::Success; } diff --git a/src/core/core.hpp b/src/core/core.hpp index c492358..5d14c28 100644 --- a/src/core/core.hpp +++ b/src/core/core.hpp @@ -149,12 +149,12 @@ struct BlockHeader mutable long m_position{ 0 }; }; -// Returns a string description of the given result +// Returns a string description of the given result. extern BGCODE_CORE_EXPORT std::string_view translate_result(EResult result); -// Get the max size of the cache used to calculate checksums, in bytes +// Get the max size of the cache used to calculate checksums, in bytes. extern BGCODE_CORE_EXPORT size_t get_checksum_max_cache_size(); -// Set the max size of the cache used to calculate checksums, in bytes +// Set the max size of the cache used to calculate checksums, in bytes. extern BGCODE_CORE_EXPORT void set_checksum_max_cache_size(size_t size); // Returns EResult::Success if the given file is a valid binary gcode @@ -163,32 +163,38 @@ extern BGCODE_CORE_EXPORT void set_checksum_max_cache_size(size_t size); extern BGCODE_CORE_EXPORT EResult is_valid_binary_gcode(FILE& file, bool check_contents = false); // Reads the file header. -// If max_version is not null, version is checked against the passed value +// If max_version is not null, version is checked against the passed value. // If return == EResult::Success: -// - header will contain the file header -// - file position will be set at the start of the 1st block header +// - header will contain the file header. +// - file position will be set at the start of the 1st block header. extern BGCODE_CORE_EXPORT EResult read_header(FILE& file, FileHeader& header, const uint32_t* const max_version); // Reads next block header from the current file position. // File position must be at the start of a block header. // If return == EResult::Success: -// - block_header will contain the header of the block -// - file position will be set at the start of the block parameters data +// - block_header will contain the header of the block. +// - file position will be set at the start of the block parameters data. extern BGCODE_CORE_EXPORT EResult read_next_block_header(FILE& file, const FileHeader& file_header, BlockHeader& block_header, bool verify_checksum); // Searches and reads next block header with the given type from the current file position. // File position must be at the start of a block header. // If return == EResult::Success: -// - block_header will contain the header of the block with the required type -// - file position will be set at the start of the block parameters data +// - block_header will contain the header of the block with the required type. +// - file position will be set at the start of the block parameters data. // otherwise: -// - file position will keep the current value +// - file position will keep the current value. extern BGCODE_CORE_EXPORT EResult read_next_block_header(FILE& file, const FileHeader& file_header, BlockHeader& block_header, EBlockType type, bool verify_checksum); // Skips the content (parameters + data + checksum) of the block with the given block header. // File position must be at the start of the block parameters. // If return == EResult::Success: -// - file position will be set at the start of the next block header +// - file position will be set at the start of the next block header. +extern BGCODE_CORE_EXPORT EResult skip_block_content(FILE& file, const FileHeader& file_header, const BlockHeader& block_header); + +// Skips the block with the given block header. +// File position must be set by a previous call to BlockHeader::write() or BlockHeader::read(). +// If return == EResult::Success: +// - file position will be set at the start of the next block header. extern BGCODE_CORE_EXPORT EResult skip_block(FILE& file, const FileHeader& file_header, const BlockHeader& block_header); // Returns the size of the parameters of the given block type, in bytes. From 4f2f07f11cbc6a354bf9ab437841e3b0008d172d Mon Sep 17 00:00:00 2001 From: Lukas Lendvorsky Date: Mon, 7 Aug 2023 13:46:14 +0200 Subject: [PATCH 7/8] libbgcode: move functions to read thumbnail header from base to core Fixed conflicts after cherry-pick --- src/binarize/binarize.cpp | 45 ++++++++++++++++++--------------------- src/binarize/binarize.hpp | 6 ++---- src/convert/convert.cpp | 10 ++++----- src/core/core.cpp | 20 +++++++++++++++++ src/core/core.hpp | 16 ++++++++++++-- tests/core/core_tests.cpp | 15 ++++--------- 6 files changed, 66 insertions(+), 46 deletions(-) diff --git a/src/binarize/binarize.cpp b/src/binarize/binarize.cpp index 2ba995a..8439d2d 100644 --- a/src/binarize/binarize.cpp +++ b/src/binarize/binarize.cpp @@ -561,13 +561,13 @@ EResult PrinterMetadataBlock::read_data(FILE& file, const FileHeader& file_heade return EResult::Success; } -EResult ThumbnailBlock::write(FILE& file, EChecksumType checksum_type) const +EResult ThumbnailBlock::write(FILE& file, EChecksumType checksum_type) { - if (format >= thumbnail_formats_count()) + if (params.format >= thumbnail_formats_count()) return EResult::InvalidThumbnailFormat; - if (width == 0) + if (params.width == 0) return EResult::InvalidThumbnailWidth; - if (height == 0) + if (params.height == 0) return EResult::InvalidThumbnailHeight; if (data.size() == 0) return EResult::InvalidThumbnailDataSize; @@ -579,13 +579,12 @@ EResult ThumbnailBlock::write(FILE& file, EChecksumType checksum_type) const // propagate error return res; - // write block payload - if (!write_to_file(file, (const void*)&format, sizeof(format))) - return EResult::WriteError; - if (!write_to_file(file, (const void*)&width, sizeof(width))) - return EResult::WriteError; - if (!write_to_file(file, (const void*)&height, sizeof(height))) - return EResult::WriteError; + res = params.write(file); + if (res != EResult::Success){ + // propagate error + return res; + } + if (!write_to_file(file, (const void*)data.data(), data.size())) return EResult::WriteError; @@ -607,17 +606,15 @@ EResult ThumbnailBlock::write(FILE& file, EChecksumType checksum_type) const EResult ThumbnailBlock::read_data(FILE& file, const FileHeader& file_header, const BlockHeader& block_header) { // read block payload - if (!read_from_file(file, (void*)&format, sizeof(format))) - return EResult::ReadError; - if (format >= thumbnail_formats_count()) + EResult res = params.read(file); + if (res != EResult::Success) + // propagate error + return res; + if (params.format >= thumbnail_formats_count()) return EResult::InvalidThumbnailFormat; - if (!read_from_file(file, (void*)&width, sizeof(width))) - return EResult::ReadError; - if (width == 0) + if (params.width == 0) return EResult::InvalidThumbnailWidth; - if (!read_from_file(file, (void*)&height, sizeof(height))) - return EResult::ReadError; - if (height == 0) + if (params.height == 0) return EResult::InvalidThumbnailHeight; if (block_header.uncompressed_size == 0) return EResult::InvalidThumbnailDataSize; @@ -640,9 +637,9 @@ EResult ThumbnailBlock::read_data(FILE& file, const FileHeader& file_header, con void ThumbnailBlock::update_checksum(Checksum& checksum) const { - checksum.append(encode((const void*)&format, sizeof(format))); - checksum.append(encode((const void*)&width, sizeof(width))); - checksum.append(encode((const void*)&height, sizeof(height))); + checksum.append(encode((const void*)¶ms.format, sizeof(params.format))); + checksum.append(encode((const void*)¶ms.width, sizeof(params.width))); + checksum.append(encode((const void*)¶ms.height, sizeof(params.height))); checksum.append(data); } @@ -813,7 +810,7 @@ EResult Binarizer::initialize(FILE& file, const BinarizerConfig& config) return res; // save thumbnail blocks - for (const ThumbnailBlock& block : m_binary_data.thumbnails) { + for (ThumbnailBlock& block : m_binary_data.thumbnails) { res = block.write(*m_file, m_config.checksum); if (res != EResult::Success) // propagate error diff --git a/src/binarize/binarize.hpp b/src/binarize/binarize.hpp index b0e6295..7d28dd3 100644 --- a/src/binarize/binarize.hpp +++ b/src/binarize/binarize.hpp @@ -45,13 +45,11 @@ struct PrinterMetadataBlock : public BaseMetadataBlock struct ThumbnailBlock { - uint16_t format{ 0 }; - uint16_t width{ 0 }; - uint16_t height{ 0 }; + bgcode::core::ThumbnailHeader header; std::vector data; // write block header and data - core::EResult write(FILE& file, core::EChecksumType checksum_type) const; + core::EResult write(FILE& file, core::EChecksumType checksum_type); // read block data core::EResult read_data(FILE& file, const core::FileHeader& file_header, const core::BlockHeader& block_header); diff --git a/src/convert/convert.cpp b/src/convert/convert.cpp index b908e01..f046944 100644 --- a/src/convert/convert.cpp +++ b/src/convert/convert.cpp @@ -361,7 +361,7 @@ BGCODE_CONVERT_EXPORT EResult from_ascii_to_binary(FILE& src_file, FILE& dst_fil } if (reading_thumbnail.has_value()) { ThumbnailBlock& thumbnail = binary_data.thumbnails.emplace_back(ThumbnailBlock()); - thumbnail.format = (uint16_t)*reading_thumbnail; + thumbnail.header.image_format = (bgcode::EThumbnailFormat)*reading_thumbnail; pos = sv_thumbnail_str.find(" "); if (pos == std::string_view::npos) { parse_res = EResult::InvalidAsciiGCodeFile; @@ -373,8 +373,8 @@ BGCODE_CONVERT_EXPORT EResult from_ascii_to_binary(FILE& src_file, FILE& dst_fil parse_res = EResult::InvalidAsciiGCodeFile; return; } - thumbnail.width = rect.first; - thumbnail.height = rect.second; + thumbnail.header.width = rect.first; + thumbnail.header.height = rect.second; const std::string_view sv_data_size_str = trim(sv_thumbnail_str.substr(pos + 1)); size_t data_size; to_int(sv_data_size_str, data_size); @@ -606,14 +606,14 @@ BGCODE_CONVERT_EXPORT EResult from_binary_to_ascii(FILE& src_file, FILE& dst_fil encoded.resize(boost::beast::detail::base64::encoded_size(thumbnail_block.data.size())); encoded.resize(boost::beast::detail::base64::encode((void*)encoded.data(), (const void*)thumbnail_block.data.data(), thumbnail_block.data.size())); std::string format; - switch ((EThumbnailFormat)thumbnail_block.format) + switch ((EThumbnailFormat)thumbnail_block.header.image_format) { default: case EThumbnailFormat::PNG: { format = "thumbnail"; break; } case EThumbnailFormat::JPG: { format = "thumbnail_JPG"; break; } case EThumbnailFormat::QOI: { format = "thumbnail_QOI"; break; } } - if (!write_line("\n;\n; " + format + " begin " + std::to_string(thumbnail_block.width) + "x" + std::to_string(thumbnail_block.height) + + if (!write_line("\n;\n; " + format + " begin " + std::to_string(thumbnail_block.header.width) + "x" + std::to_string(thumbnail_block.header.height) + " " + std::to_string(encoded.length()) + "\n")) return EResult::WriteError; while (encoded.size() > max_row_length) { diff --git a/src/core/core.cpp b/src/core/core.cpp index 1277381..fe157b2 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -256,6 +256,26 @@ size_t BlockHeader::get_size() const { return sizeof(type) + sizeof(compression) + sizeof(uncompressed_size) + ((compression == (uint16_t)ECompressionType::None)? 0 : sizeof(compressed_size)); } +EResult ThumbnailParams::write(FILE& file) const { + if (!write_to_file(file, (const void*)&format, sizeof(format))) + return EResult::WriteError; + if (!write_to_file(file, (const void*)&width, sizeof(width))) + return EResult::WriteError; + if (!write_to_file(file, (const void*)&height, sizeof(height))) + return EResult::WriteError; + return EResult::Success; +} + +EResult ThumbnailParams::read(FILE& file){ + if (!read_from_file(file, (void*)&format, sizeof(format))) + return EResult::ReadError; + if (!read_from_file(file, (void*)&width, sizeof(width))) + return EResult::ReadError; + if (!read_from_file(file, (void*)&height, sizeof(height))) + return EResult::ReadError; + return EResult::Success; +} + BGCODE_CORE_EXPORT size_t get_checksum_max_cache_size() { return g_checksum_max_cache_size; } BGCODE_CORE_EXPORT void set_checksum_max_cache_size(size_t size) { g_checksum_max_cache_size = size; } diff --git a/src/core/core.hpp b/src/core/core.hpp index 5d14c28..ea64bc7 100644 --- a/src/core/core.hpp +++ b/src/core/core.hpp @@ -133,7 +133,7 @@ struct BlockHeader BlockHeader() = default; BlockHeader(uint16_t type, uint16_t compression, uint32_t uncompressed_size, uint32_t compressed_size = 0); - // Updates the given checksum with the data of this BlockHeader. + // Updates the given checksum with the data of this BlockHeader void update_checksum(Checksum& checksum) const; // Returns the position of this block in the file. @@ -143,13 +143,25 @@ struct BlockHeader EResult write(FILE& file) const; EResult read(FILE& file); + // Returs the size of this BlockHeader, in bytes size_t get_size() const; private: mutable long m_position{ 0 }; }; -// Returns a string description of the given result. +struct ThumbnailParams +{ + uint16_t format; + uint16_t width; + uint16_t height; + + EResult write(FILE& file) const; + EResult read(FILE& file); +}; + + +// Returns a string description of the given result extern BGCODE_CORE_EXPORT std::string_view translate_result(EResult result); // Get the max size of the cache used to calculate checksums, in bytes. diff --git a/tests/core/core_tests.cpp b/tests/core/core_tests.cpp index 5e2b2c1..bde84a3 100644 --- a/tests/core/core_tests.cpp +++ b/tests/core/core_tests.cpp @@ -149,18 +149,11 @@ TEST_CASE("Checksum max cache size", "[Core]") case EBlockType::Thumbnail: { const long curr_pos = ftell(file); - uint16_t format; - size_t rsize = fread(&format, 1, sizeof(format), file); - REQUIRE((ferror(file) == 0 && rsize == sizeof(format))); - uint16_t width; - rsize = fread(&width, 1, sizeof(width), file); - REQUIRE((ferror(file) == 0 && rsize == sizeof(width))); - uint16_t height; - rsize = fread(&height, 1, sizeof(height), file); - REQUIRE((ferror(file) == 0 && rsize == sizeof(height))); + ThumbnailParams thumbnail_params; + REQUIRE(thumbnail_params.read(*file) == EResult::Success); fseek(file, curr_pos, SEEK_SET); - std::cout << " - format: " << thumbnail_format_as_string((EThumbnailFormat)format); - std::cout << " (size: " << width << "x" << height << ")"; + std::cout << " - format: " << thumbnail_format_as_string((EThumbnailFormat)thumbnail_params.format); + std::cout << " (size: " << thumbnail_params.width << "x" << thumbnail_params.height << ")"; break; } default: { break; } From c1401d664f7280edac27e1da0031b969d1c6c6f7 Mon Sep 17 00:00:00 2001 From: enricoturri1966 Date: Tue, 8 Aug 2023 10:51:50 +0200 Subject: [PATCH 8/8] Follow-up of 0841169233d3623ec98339d65d0b0d69042bb5c8 - Some refactoring --- src/binarize/binarize.hpp | 2 +- src/convert/convert.cpp | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/binarize/binarize.hpp b/src/binarize/binarize.hpp index 7d28dd3..ab8dac5 100644 --- a/src/binarize/binarize.hpp +++ b/src/binarize/binarize.hpp @@ -45,7 +45,7 @@ struct PrinterMetadataBlock : public BaseMetadataBlock struct ThumbnailBlock { - bgcode::core::ThumbnailHeader header; + core::ThumbnailParams params; std::vector data; // write block header and data diff --git a/src/convert/convert.cpp b/src/convert/convert.cpp index f046944..e30c893 100644 --- a/src/convert/convert.cpp +++ b/src/convert/convert.cpp @@ -361,7 +361,7 @@ BGCODE_CONVERT_EXPORT EResult from_ascii_to_binary(FILE& src_file, FILE& dst_fil } if (reading_thumbnail.has_value()) { ThumbnailBlock& thumbnail = binary_data.thumbnails.emplace_back(ThumbnailBlock()); - thumbnail.header.image_format = (bgcode::EThumbnailFormat)*reading_thumbnail; + thumbnail.params.format = (uint16_t)*reading_thumbnail; pos = sv_thumbnail_str.find(" "); if (pos == std::string_view::npos) { parse_res = EResult::InvalidAsciiGCodeFile; @@ -373,8 +373,8 @@ BGCODE_CONVERT_EXPORT EResult from_ascii_to_binary(FILE& src_file, FILE& dst_fil parse_res = EResult::InvalidAsciiGCodeFile; return; } - thumbnail.header.width = rect.first; - thumbnail.header.height = rect.second; + thumbnail.params.width = rect.first; + thumbnail.params.height = rect.second; const std::string_view sv_data_size_str = trim(sv_thumbnail_str.substr(pos + 1)); size_t data_size; to_int(sv_data_size_str, data_size); @@ -606,14 +606,14 @@ BGCODE_CONVERT_EXPORT EResult from_binary_to_ascii(FILE& src_file, FILE& dst_fil encoded.resize(boost::beast::detail::base64::encoded_size(thumbnail_block.data.size())); encoded.resize(boost::beast::detail::base64::encode((void*)encoded.data(), (const void*)thumbnail_block.data.data(), thumbnail_block.data.size())); std::string format; - switch ((EThumbnailFormat)thumbnail_block.header.image_format) + switch ((EThumbnailFormat)thumbnail_block.params.format) { default: case EThumbnailFormat::PNG: { format = "thumbnail"; break; } case EThumbnailFormat::JPG: { format = "thumbnail_JPG"; break; } case EThumbnailFormat::QOI: { format = "thumbnail_QOI"; break; } } - if (!write_line("\n;\n; " + format + " begin " + std::to_string(thumbnail_block.header.width) + "x" + std::to_string(thumbnail_block.header.height) + + if (!write_line("\n;\n; " + format + " begin " + std::to_string(thumbnail_block.params.width) + "x" + std::to_string(thumbnail_block.params.height) + " " + std::to_string(encoded.length()) + "\n")) return EResult::WriteError; while (encoded.size() > max_row_length) {