From 203b008eb220208981902e0db541c02d1c1c9f5e Mon Sep 17 00:00:00 2001 From: Jia Tan Date: Wed, 17 Aug 2022 17:59:51 +0800 Subject: [PATCH 01/13] liblzma: Replaced hardcoded 0x0 index indicator byte with macro --- src/liblzma/common/index.h | 3 +++ src/liblzma/common/index_decoder.c | 2 +- src/liblzma/common/index_encoder.c | 2 +- src/liblzma/common/index_hash.c | 2 +- src/liblzma/common/stream_decoder.c | 3 ++- src/liblzma/common/stream_decoder_mt.c | 2 +- 6 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/liblzma/common/index.h b/src/liblzma/common/index.h index 64e97247..ea396714 100644 --- a/src/liblzma/common/index.h +++ b/src/liblzma/common/index.h @@ -22,6 +22,9 @@ /// Maximum Unpadded Size #define UNPADDED_SIZE_MAX (LZMA_VLI_MAX & ~LZMA_VLI_C(3)) +/// Index Indicator based on xz specification +#define INDEX_INDICATOR 0 + /// Get the size of the Index Padding field. This is needed by Index encoder /// and decoder, but applications should have no use for this. diff --git a/src/liblzma/common/index_decoder.c b/src/liblzma/common/index_decoder.c index b2689885..8622b2f0 100644 --- a/src/liblzma/common/index_decoder.c +++ b/src/liblzma/common/index_decoder.c @@ -80,7 +80,7 @@ index_decode(void *coder_ptr, const lzma_allocator *allocator, // format". One could argue that the application should // verify the Index Indicator before trying to decode the // Index, but well, I suppose it is simpler this way. - if (in[(*in_pos)++] != 0x00) + if (in[(*in_pos)++] != INDEX_INDICATOR) return LZMA_DATA_ERROR; coder->sequence = SEQ_COUNT; diff --git a/src/liblzma/common/index_encoder.c b/src/liblzma/common/index_encoder.c index ac97d0ce..c7cafb72 100644 --- a/src/liblzma/common/index_encoder.c +++ b/src/liblzma/common/index_encoder.c @@ -65,7 +65,7 @@ index_encode(void *coder_ptr, while (*out_pos < out_size) switch (coder->sequence) { case SEQ_INDICATOR: - out[*out_pos] = 0x00; + out[*out_pos] = INDEX_INDICATOR; ++*out_pos; coder->sequence = SEQ_COUNT; break; diff --git a/src/liblzma/common/index_hash.c b/src/liblzma/common/index_hash.c index 34df85d7..c3c56674 100644 --- a/src/liblzma/common/index_hash.c +++ b/src/liblzma/common/index_hash.c @@ -190,7 +190,7 @@ lzma_index_hash_decode(lzma_index_hash *index_hash, const uint8_t *in, switch (index_hash->sequence) { case SEQ_BLOCK: // Check the Index Indicator is present. - if (in[(*in_pos)++] != 0x00) + if (in[(*in_pos)++] != INDEX_INDICATOR) return LZMA_DATA_ERROR; index_hash->sequence = SEQ_COUNT; diff --git a/src/liblzma/common/stream_decoder.c b/src/liblzma/common/stream_decoder.c index dcf7c149..64283812 100644 --- a/src/liblzma/common/stream_decoder.c +++ b/src/liblzma/common/stream_decoder.c @@ -12,6 +12,7 @@ #include "stream_decoder.h" #include "block_decoder.h" +#include "index.h" typedef struct { @@ -164,7 +165,7 @@ stream_decode(void *coder_ptr, const lzma_allocator *allocator, if (coder->pos == 0) { // Detect if it's Index. - if (in[*in_pos] == 0x00) { + if (in[*in_pos] == INDEX_INDICATOR) { coder->sequence = SEQ_INDEX; break; } diff --git a/src/liblzma/common/stream_decoder_mt.c b/src/liblzma/common/stream_decoder_mt.c index 5733c764..fd5cd7fd 100644 --- a/src/liblzma/common/stream_decoder_mt.c +++ b/src/liblzma/common/stream_decoder_mt.c @@ -887,7 +887,7 @@ decode_block_header(struct lzma_stream_coder *coder, if (coder->pos == 0) { // Detect if it's Index. - if (in[*in_pos] == 0x00) + if (in[*in_pos] == INDEX_INDICATOR) return LZMA_INDEX_DETECTED; // Calculate the size of the Block Header. Note that From f16e12d5e755d371247202fcccbcccd1ec16b2cf Mon Sep 17 00:00:00 2001 From: Jia Tan Date: Wed, 17 Aug 2022 20:20:16 +0800 Subject: [PATCH 02/13] liblzma: Add NULL check to lzma_index_hash_append. This is for consistency with lzma_index_append. --- src/liblzma/common/index_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/liblzma/common/index_hash.c b/src/liblzma/common/index_hash.c index c3c56674..f55f7bc8 100644 --- a/src/liblzma/common/index_hash.c +++ b/src/liblzma/common/index_hash.c @@ -145,7 +145,7 @@ lzma_index_hash_append(lzma_index_hash *index_hash, lzma_vli unpadded_size, lzma_vli uncompressed_size) { // Validate the arguments. - if (index_hash->sequence != SEQ_BLOCK + if (index_hash == NULL || index_hash->sequence != SEQ_BLOCK || unpadded_size < UNPADDED_SIZE_MIN || unpadded_size > UNPADDED_SIZE_MAX || uncompressed_size > LZMA_VLI_MAX) From 3959162baec074511d83ba0fec1284c3ed724799 Mon Sep 17 00:00:00 2001 From: Jia Tan Date: Thu, 29 Dec 2022 00:25:18 +0800 Subject: [PATCH 03/13] Tests: Creates test_index_hash.c Tests all API functions exported from index_hash.h. Does not have a dedicated test for lzma_index_hash_end. --- CMakeLists.txt | 2 + tests/Makefile.am | 3 + tests/test_index_hash.c | 379 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 384 insertions(+) create mode 100644 tests/test_index_hash.c diff --git a/CMakeLists.txt b/CMakeLists.txt index c4f10594..3a58d740 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -801,6 +801,7 @@ if(BUILD_TESTING) test_filter_flags test_hardware test_index + test_index_hash test_memlimit test_stream_flags test_vli @@ -812,6 +813,7 @@ if(BUILD_TESTING) target_include_directories("${TEST}" PRIVATE src/common src/liblzma/api + src/liblzma lib ) diff --git a/tests/Makefile.am b/tests/Makefile.am index f84c1749..b1a2378a 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -26,6 +26,7 @@ EXTRA_DIST = \ AM_CPPFLAGS = \ -I$(top_srcdir)/src/common \ -I$(top_srcdir)/src/liblzma/api \ + -I$(top_srcdir)/src/liblzma \ -I$(top_builddir)/lib LDADD = $(top_builddir)/src/liblzma/liblzma.la @@ -44,6 +45,7 @@ check_PROGRAMS = \ test_filter_flags \ test_block_header \ test_index \ + test_index_hash \ test_bcj_exact_size \ test_memlimit \ test_lzip_decoder \ @@ -56,6 +58,7 @@ TESTS = \ test_filter_flags \ test_block_header \ test_index \ + test_index_hash \ test_bcj_exact_size \ test_memlimit \ test_lzip_decoder \ diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c new file mode 100644 index 00000000..a3c021b9 --- /dev/null +++ b/tests/test_index_hash.c @@ -0,0 +1,379 @@ +/////////////////////////////////////////////////////////////////////////////// +// +/// \file test_index_hash.c +/// \brief Tests src/liblzma/common/index_hash.c API functions +/// +/// \note No test included for lzma_index_hash_end since it +/// would be trivial unless tested for memory leaks +/// with something like valgrind +// +// Author: Jia Tan +// +// This file has been put into the public domain. +// You can do whatever you want with this file. +// +/////////////////////////////////////////////////////////////////////////////// + +#include "tests.h" +// Needed for UNPADDED_SIZE_MIN and UNPADDED_SIZE_MAX macro definitions +// and index_size and vli_ceil4 helper functions +#include "common/index.h" + + +static void +test_lzma_index_hash_init(void) +{ +#ifndef HAVE_DECODERS + assert_skip("Decoder support disabled"); +#else + // First test with NULL index hash + // This should create a fresh index hash + lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); + assert_true(index_hash != NULL); + + // Next test with non-NULL index hash + lzma_index_hash *second_hash = lzma_index_hash_init(index_hash, NULL); + + // Should not create a new index_hash pointer + // Instead must just re-init the first index hash + assert_true(index_hash == second_hash); + + lzma_index_hash_end(index_hash, NULL); +#endif +} + + +static void +test_lzma_index_hash_append(void) +{ +#ifndef HAVE_DECODERS + assert_skip("Decoder support disabled"); +#else + // Test all invalid parameters + assert_lzma_ret(lzma_index_hash_append(NULL, 0, 0), + LZMA_PROG_ERROR); + + // Test NULL index_hash + assert_lzma_ret(lzma_index_hash_append(NULL, UNPADDED_SIZE_MIN, + LZMA_VLI_MAX), LZMA_PROG_ERROR); + + // Test with invalid unpadded size + lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); + assert_true(index_hash); + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN - 1, LZMA_VLI_MAX), + LZMA_PROG_ERROR); + + // Test with invalid uncompressed size + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN, LZMA_VLI_MAX + 1), + LZMA_PROG_ERROR); + + // Append first a small "block" to the index, which should succeed + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN, 1), LZMA_OK); + + // Append another small "block" + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN, 1), LZMA_OK); + + // Append a block that would cause the compressed size to grow + // too big + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MAX, 1), LZMA_DATA_ERROR); + + lzma_index_hash_end(index_hash, NULL); +#endif +} + +#ifdef HAVE_DECODERS +// Fill an index_hash with unpadded and uncompressed VLIs +// by calling lzma_index_hash_append +static void +fill_index_hash(lzma_index_hash *index_hash, const lzma_vli *unpadded_sizes, + const lzma_vli *uncomp_sizes, uint32_t block_count) +{ + for(uint32_t i = 0; i < block_count; i++) + assert_lzma_ret(lzma_index_hash_append(index_hash, + unpadded_sizes[i], uncomp_sizes[i]), LZMA_OK); +} + + +#ifdef HAVE_ENCODERS +// Set the index parameter to the expected index based on the +// xz specification. Needs the unpadded and uncompressed VLIs +// to correctly create the index +static void +generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, + const lzma_vli *uncomp_sizes, uint32_t block_count, + size_t index_max_size) +{ + size_t in_pos = 0; + size_t out_pos = 0; + // First set index indicator + index[out_pos++] = 0; + + // Next write out Number of Records + assert_lzma_ret(lzma_vli_encode(block_count, &in_pos, index, + &out_pos, index_max_size), LZMA_STREAM_END); + + // Next write out each record + // A record consists of unpadded size and uncompressed size + // written next to each other as VLIs + for (uint32_t i = 0; i < block_count; i++) { + in_pos = 0; + assert_lzma_ret(lzma_vli_encode(unpadded_sizes[i], &in_pos, + index, &out_pos, index_max_size), LZMA_STREAM_END); + in_pos = 0; + assert_lzma_ret(lzma_vli_encode(uncomp_sizes[i], &in_pos, + index, &out_pos, index_max_size), LZMA_STREAM_END); + } + + // Add index padding + lzma_vli rounded_out_pos = vli_ceil4(out_pos); + memzero(index + out_pos, rounded_out_pos - out_pos); + out_pos = rounded_out_pos; + + // Add the CRC32 + write32le(index + out_pos, lzma_crc32(index, out_pos, 0)); +} +#endif +#endif + + +static void +test_lzma_index_hash_decode(void) +{ +#if !defined(HAVE_ENCODERS) || !defined(HAVE_DECODERS) + assert_skip("Encoder or decoder support disabled"); +#else + lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); + assert_true(index_hash); + + size_t in_pos = 0; + + // Six valid sizes for unpadded data sizes + const lzma_vli unpadded_sizes[6] = { + UNPADDED_SIZE_MIN, + 1000, + 4000, + 8000, + 16000, + 32000 + }; + + // Six valid sizes for uncompressed data sizes + const lzma_vli uncomp_sizes[6] = { + 1, + 500, + 8000, + 20, + 1, + 500 + }; + + // Add two entries to a index hash + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); + + const lzma_vli size_two_entries = lzma_index_hash_size(index_hash); + assert_uint(size_two_entries, >, 0); + uint8_t *index_two_entries = tuktest_malloc(size_two_entries); + + generate_index(index_two_entries, unpadded_sizes, uncomp_sizes, 2, + size_two_entries); + + // First test for basic buffer size error + in_pos = size_two_entries + 1; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_two_entries, &in_pos, + size_two_entries), LZMA_BUF_ERROR); + + // Next test for invalid index indicator + in_pos = 0; + index_two_entries[0] ^= 1; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_two_entries, &in_pos, + size_two_entries), LZMA_DATA_ERROR); + index_two_entries[0] ^= 1; + + // Next verify the index_hash as expected + in_pos = 0; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_two_entries, &in_pos, + size_two_entries), LZMA_STREAM_END); + + // Next test a three entry index hash + index_hash = lzma_index_hash_init(index_hash, NULL); + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 3); + + const lzma_vli size_three_entries = lzma_index_hash_size( + index_hash); + assert_uint(size_three_entries, >, 0); + uint8_t *index_three_entries = tuktest_malloc(size_three_entries); + + generate_index(index_three_entries, unpadded_sizes, uncomp_sizes, + 3, size_three_entries); + + in_pos = 0; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_three_entries, &in_pos, + size_three_entries), LZMA_STREAM_END); + + // Next test a five entry index hash + index_hash = lzma_index_hash_init(index_hash, NULL); + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 5); + + const lzma_vli size_five_entries = lzma_index_hash_size( + index_hash); + assert_uint(size_five_entries, >, 0); + uint8_t *index_five_entries = tuktest_malloc(size_five_entries); + + generate_index(index_five_entries, unpadded_sizes, uncomp_sizes, 5, + size_five_entries); + + // Instead of testing all input at once, give input + // one byte at a time + in_pos = 0; + for (lzma_vli i = 0; i < size_five_entries - 1; i++) { + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_five_entries, &in_pos, in_pos + 1), + LZMA_OK); + } + + // Last byte should return LZMA_STREAM_END + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_five_entries, &in_pos, + in_pos + 1), LZMA_STREAM_END); + + // Next test if the index hash is given an incorrect unpadded + // size. Should detect and report LZMA_DATA_ERROR + index_hash = lzma_index_hash_init(index_hash, NULL); + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 5); + // The sixth entry will have invalid unpadded size + assert_lzma_ret(lzma_index_hash_append(index_hash, + unpadded_sizes[5] + 1, + uncomp_sizes[5]), LZMA_OK); + + const lzma_vli size_six_entries = lzma_index_hash_size( + index_hash); + + assert_uint(size_six_entries, >, 0); + uint8_t *index_six_entries = tuktest_malloc(size_six_entries); + + generate_index(index_six_entries, unpadded_sizes, uncomp_sizes, 6, + size_six_entries); + in_pos = 0; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_six_entries, &in_pos, + size_six_entries), LZMA_DATA_ERROR); + + // Next test if the index is corrupt (invalid CRC) + // Should detect and report LZMA_DATA_ERROR + index_hash = lzma_index_hash_init(index_hash, NULL); + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); + + index_two_entries[size_two_entries - 1] ^= 1; + + in_pos = 0; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_two_entries, &in_pos, + size_two_entries), LZMA_DATA_ERROR); + + // Next test with index and index_hash struct not matching + // an entry + index_hash = lzma_index_hash_init(index_hash, NULL); + fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); + // Recalculate index with invalid unpadded size + const lzma_vli unpadded_sizes_invalid[2] = { + unpadded_sizes[0], + unpadded_sizes[1] + 1 + }; + + generate_index(index_two_entries, unpadded_sizes_invalid, + uncomp_sizes, 2, size_two_entries); + + in_pos = 0; + assert_lzma_ret(lzma_index_hash_decode(index_hash, + index_two_entries, &in_pos, + size_two_entries), LZMA_DATA_ERROR); + + lzma_index_hash_end(index_hash, NULL); +#endif +} + + +static void +test_lzma_index_hash_size(void) +{ +#ifndef HAVE_DECODERS + assert_skip("Decoder support disabled"); +#else + lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); + assert_true(index_hash); + + // First test empty index hash + // Expected size should be: + // Index Indicator - 1 byte + // Number of Records - 1 byte + // List of Records - 0 bytes + // Index Padding - 2 bytes + // CRC32 - 4 bytes + // Total - 8 bytes + assert_uint_eq(lzma_index_hash_size(index_hash), 8); + + // Append a small block to the index hash + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN, 1), LZMA_OK); + + // Expected size should be: + // Index Indicator - 1 byte + // Number of Records - 1 byte + // List of Records - 2 bytes + // Index Padding - 0 bytes + // CRC32 - 4 bytes + // Total - 8 bytes + lzma_vli expected_size = 8; + assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); + + // Append additional small block + assert_lzma_ret(lzma_index_hash_append(index_hash, + UNPADDED_SIZE_MIN, 1), LZMA_OK); + + // Expected size should be: + // Index Indicator - 1 byte + // Number of Records - 1 byte + // List of Records - 4 bytes + // Index Padding - 2 bytes + // CRC32 - 4 bytes + // Total - 12 bytes + expected_size = 12; + assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); + + // Append a larger block to the index hash (3 bytes for each vli) + const lzma_vli three_byte_vli = 0x10000; + assert_lzma_ret(lzma_index_hash_append(index_hash, + three_byte_vli, three_byte_vli), LZMA_OK); + + // Expected size should be: + // Index Indicator - 1 byte + // Number of Records - 1 byte + // List of Records - 10 bytes + // Index Padding - 0 bytes + // CRC32 - 4 bytes + // Total - 16 bytes + expected_size = 16; + assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); +#endif +} + + +extern int +main(int argc, char **argv) +{ + tuktest_start(argc, argv); + tuktest_run(test_lzma_index_hash_init); + tuktest_run(test_lzma_index_hash_append); + tuktest_run(test_lzma_index_hash_decode); + tuktest_run(test_lzma_index_hash_size); + return tuktest_end(); +} From 064cd385a716abc78d93a3612411a82d69ceb221 Mon Sep 17 00:00:00 2001 From: Jia Tan Date: Thu, 29 Dec 2022 00:30:52 +0800 Subject: [PATCH 04/13] Adds test_index_hash to .gitignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 44f269cc..7f2dec23 100644 --- a/.gitignore +++ b/.gitignore @@ -62,6 +62,7 @@ coverage /tests/test_filter_flags /tests/test_hardware /tests/test_index +/tests/test_index_hash /test/test_lzip_decoder /tests/test_memlimit /tests/test_stream_flags From 84f9687cbae972c2c342e10bf69f8ec8f70ae111 Mon Sep 17 00:00:00 2001 From: Jia Tan Date: Thu, 5 Jan 2023 20:57:25 +0800 Subject: [PATCH 05/13] liblzma: Remove common.h include from common/index.h. common/index.h is needed by liblzma internally and tests. common.h will include and define many things that are not needed by the tests. Also, this prevents include order problems because common.h will redefine LZMA_API resulting in a warning. --- src/liblzma/common/index.c | 1 + src/liblzma/common/index.h | 9 +++++++-- src/liblzma/common/index_decoder.h | 1 + src/liblzma/common/stream_buffer_encoder.c | 1 + 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/liblzma/common/index.c b/src/liblzma/common/index.c index 24ec3c10..97cc9f95 100644 --- a/src/liblzma/common/index.c +++ b/src/liblzma/common/index.c @@ -10,6 +10,7 @@ // /////////////////////////////////////////////////////////////////////////////// +#include "common.h" #include "index.h" #include "stream_flags_common.h" diff --git a/src/liblzma/common/index.h b/src/liblzma/common/index.h index ea396714..031efcc7 100644 --- a/src/liblzma/common/index.h +++ b/src/liblzma/common/index.h @@ -2,6 +2,13 @@ // /// \file index.h /// \brief Handling of Index +/// \note This header file does not include common.h or lzma.h because +/// this file is needed by both liblzma internally and by the +/// tests. Including common.h will include and define many things +/// the tests do not need and prevents issues with header file +/// include order. This way, if lzma.h or common.h are not +/// included before this file it will break on every OS instead +/// of causing more subtle errors. // // Author: Lasse Collin // @@ -13,8 +20,6 @@ #ifndef LZMA_INDEX_H #define LZMA_INDEX_H -#include "common.h" - /// Minimum Unpadded Size #define UNPADDED_SIZE_MIN LZMA_VLI_C(5) diff --git a/src/liblzma/common/index_decoder.h b/src/liblzma/common/index_decoder.h index 1af433b5..3fec4833 100644 --- a/src/liblzma/common/index_decoder.h +++ b/src/liblzma/common/index_decoder.h @@ -13,6 +13,7 @@ #ifndef LZMA_INDEX_DECODER_H #define LZMA_INDEX_DECODER_H +#include "common.h" #include "index.h" diff --git a/src/liblzma/common/stream_buffer_encoder.c b/src/liblzma/common/stream_buffer_encoder.c index af49554a..73157590 100644 --- a/src/liblzma/common/stream_buffer_encoder.c +++ b/src/liblzma/common/stream_buffer_encoder.c @@ -10,6 +10,7 @@ // /////////////////////////////////////////////////////////////////////////////// +#include "common.h" #include "index.h" From c48b24fc06d98569adb72f13c2e8e5ff30bb8036 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:17:37 +0200 Subject: [PATCH 06/13] Tests: test_index_hash: Use INDEX_INDICATOR constant instead of 0. --- tests/test_index_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index a3c021b9..bc0cef50 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -111,7 +111,7 @@ generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, size_t in_pos = 0; size_t out_pos = 0; // First set index indicator - index[out_pos++] = 0; + index[out_pos++] = INDEX_INDICATOR; // Next write out Number of Records assert_lzma_ret(lzma_vli_encode(block_count, &in_pos, index, From b93f7c5cbb02b42024ac866fc0af541de3d816e2 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:35:05 +0200 Subject: [PATCH 07/13] Tests: test_index_hash: Tweak comments and style. The words defined in the .xz file format specification begin with capital letter to emphasize that they have a specific meaning. --- tests/test_index_hash.c | 62 ++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 29 deletions(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index bc0cef50..cc7e2c63 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -15,6 +15,7 @@ /////////////////////////////////////////////////////////////////////////////// #include "tests.h" + // Needed for UNPADDED_SIZE_MIN and UNPADDED_SIZE_MAX macro definitions // and index_size and vli_ceil4 helper functions #include "common/index.h" @@ -26,16 +27,16 @@ test_lzma_index_hash_init(void) #ifndef HAVE_DECODERS assert_skip("Decoder support disabled"); #else - // First test with NULL index hash - // This should create a fresh index hash + // First test with NULL index_hash. + // This should create a fresh index_hash. lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); assert_true(index_hash != NULL); - // Next test with non-NULL index hash + // Next test with non-NULL index_hash. lzma_index_hash *second_hash = lzma_index_hash_init(index_hash, NULL); - // Should not create a new index_hash pointer - // Instead must just re-init the first index hash + // It should not create a new index_hash pointer. + // Instead it must just re-init the first index_hash. assert_true(index_hash == second_hash); lzma_index_hash_end(index_hash, NULL); @@ -57,27 +58,28 @@ test_lzma_index_hash_append(void) assert_lzma_ret(lzma_index_hash_append(NULL, UNPADDED_SIZE_MIN, LZMA_VLI_MAX), LZMA_PROG_ERROR); - // Test with invalid unpadded size + // Test with invalid Unpadded Size lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); assert_true(index_hash); assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN - 1, LZMA_VLI_MAX), LZMA_PROG_ERROR); - // Test with invalid uncompressed size + // Test with invalid Uncompressed Size assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN, LZMA_VLI_MAX + 1), LZMA_PROG_ERROR); - // Append first a small "block" to the index, which should succeed + // First append a Record describing a small Block. + // This should succeed. assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN, 1), LZMA_OK); - // Append another small "block" + // Append another small Record. assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN, 1), LZMA_OK); - // Append a block that would cause the compressed size to grow + // Append a Record that would cause the compressed size to grow // too big assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MAX, 1), LZMA_DATA_ERROR); @@ -86,6 +88,7 @@ test_lzma_index_hash_append(void) #endif } + #ifdef HAVE_DECODERS // Fill an index_hash with unpadded and uncompressed VLIs // by calling lzma_index_hash_append @@ -93,7 +96,7 @@ static void fill_index_hash(lzma_index_hash *index_hash, const lzma_vli *unpadded_sizes, const lzma_vli *uncomp_sizes, uint32_t block_count) { - for(uint32_t i = 0; i < block_count; i++) + for (uint32_t i = 0; i < block_count; ++i) assert_lzma_ret(lzma_index_hash_append(index_hash, unpadded_sizes[i], uncomp_sizes[i]), LZMA_OK); } @@ -101,8 +104,8 @@ fill_index_hash(lzma_index_hash *index_hash, const lzma_vli *unpadded_sizes, #ifdef HAVE_ENCODERS // Set the index parameter to the expected index based on the -// xz specification. Needs the unpadded and uncompressed VLIs -// to correctly create the index +// .xz specification. This needs the unpadded and uncompressed VLIs +// to correctly create the Index. static void generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, const lzma_vli *uncomp_sizes, uint32_t block_count, @@ -110,17 +113,18 @@ generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, { size_t in_pos = 0; size_t out_pos = 0; - // First set index indicator + + // First set Index Indicator index[out_pos++] = INDEX_INDICATOR; // Next write out Number of Records assert_lzma_ret(lzma_vli_encode(block_count, &in_pos, index, &out_pos, index_max_size), LZMA_STREAM_END); - // Next write out each record - // A record consists of unpadded size and uncompressed size - // written next to each other as VLIs - for (uint32_t i = 0; i < block_count; i++) { + // Next write out each Record. + // A Record consists of Unpadded Size and Uncompressed Size + // written next to each other as VLIs. + for (uint32_t i = 0; i < block_count; ++i) { in_pos = 0; assert_lzma_ret(lzma_vli_encode(unpadded_sizes[i], &in_pos, index, &out_pos, index_max_size), LZMA_STREAM_END); @@ -129,7 +133,7 @@ generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, index, &out_pos, index_max_size), LZMA_STREAM_END); } - // Add index padding + // Add Index Padding lzma_vli rounded_out_pos = vli_ceil4(out_pos); memzero(index + out_pos, rounded_out_pos - out_pos); out_pos = rounded_out_pos; @@ -152,7 +156,7 @@ test_lzma_index_hash_decode(void) size_t in_pos = 0; - // Six valid sizes for unpadded data sizes + // Six valid values for the Unpadded Size fields in an Index const lzma_vli unpadded_sizes[6] = { UNPADDED_SIZE_MIN, 1000, @@ -162,7 +166,7 @@ test_lzma_index_hash_decode(void) 32000 }; - // Six valid sizes for uncompressed data sizes + // Six valid values for the Uncompressed Size fields in an Index const lzma_vli uncomp_sizes[6] = { 1, 500, @@ -188,7 +192,7 @@ test_lzma_index_hash_decode(void) index_two_entries, &in_pos, size_two_entries), LZMA_BUF_ERROR); - // Next test for invalid index indicator + // Next test for invalid Index Indicator in_pos = 0; index_two_entries[0] ^= 1; assert_lzma_ret(lzma_index_hash_decode(index_hash, @@ -234,7 +238,7 @@ test_lzma_index_hash_decode(void) // Instead of testing all input at once, give input // one byte at a time in_pos = 0; - for (lzma_vli i = 0; i < size_five_entries - 1; i++) { + for (lzma_vli i = 0; i < size_five_entries - 1; ++i) { assert_lzma_ret(lzma_index_hash_decode(index_hash, index_five_entries, &in_pos, in_pos + 1), LZMA_OK); @@ -267,7 +271,7 @@ test_lzma_index_hash_decode(void) index_six_entries, &in_pos, size_six_entries), LZMA_DATA_ERROR); - // Next test if the index is corrupt (invalid CRC) + // Next test if the Index is corrupt (invalid CRC32). // Should detect and report LZMA_DATA_ERROR index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); @@ -283,7 +287,7 @@ test_lzma_index_hash_decode(void) // an entry index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); - // Recalculate index with invalid unpadded size + // Recalculate Index with invalid Unpadded Size const lzma_vli unpadded_sizes_invalid[2] = { unpadded_sizes[0], unpadded_sizes[1] + 1 @@ -311,7 +315,7 @@ test_lzma_index_hash_size(void) lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); assert_true(index_hash); - // First test empty index hash + // First test empty index_hash // Expected size should be: // Index Indicator - 1 byte // Number of Records - 1 byte @@ -321,7 +325,7 @@ test_lzma_index_hash_size(void) // Total - 8 bytes assert_uint_eq(lzma_index_hash_size(index_hash), 8); - // Append a small block to the index hash + // Append a Record describing a small Block to the index_hash assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN, 1), LZMA_OK); @@ -335,7 +339,7 @@ test_lzma_index_hash_size(void) lzma_vli expected_size = 8; assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); - // Append additional small block + // Append additional small Record assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN, 1), LZMA_OK); @@ -349,7 +353,7 @@ test_lzma_index_hash_size(void) expected_size = 12; assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); - // Append a larger block to the index hash (3 bytes for each vli) + // Append a larger Record to the index_hash (3 bytes for each VLI) const lzma_vli three_byte_vli = 0x10000; assert_lzma_ret(lzma_index_hash_append(index_hash, three_byte_vli, three_byte_vli), LZMA_OK); From d1f24c35874eeba8432d75aa77b06c50375ed937 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:35:50 +0200 Subject: [PATCH 08/13] Tests: test_index_hash: Use the word "Record" instead of "entry". --- tests/test_index_hash.c | 102 ++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index cc7e2c63..cb817a0e 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -176,115 +176,115 @@ test_lzma_index_hash_decode(void) 500 }; - // Add two entries to a index hash + // Add two Records to a index_hash fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); - const lzma_vli size_two_entries = lzma_index_hash_size(index_hash); - assert_uint(size_two_entries, >, 0); - uint8_t *index_two_entries = tuktest_malloc(size_two_entries); + const lzma_vli size_two_records = lzma_index_hash_size(index_hash); + assert_uint(size_two_records, >, 0); + uint8_t *index_two_records = tuktest_malloc(size_two_records); - generate_index(index_two_entries, unpadded_sizes, uncomp_sizes, 2, - size_two_entries); + generate_index(index_two_records, unpadded_sizes, uncomp_sizes, 2, + size_two_records); // First test for basic buffer size error - in_pos = size_two_entries + 1; + in_pos = size_two_records + 1; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_two_entries, &in_pos, - size_two_entries), LZMA_BUF_ERROR); + index_two_records, &in_pos, + size_two_records), LZMA_BUF_ERROR); // Next test for invalid Index Indicator in_pos = 0; - index_two_entries[0] ^= 1; + index_two_records[0] ^= 1; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_two_entries, &in_pos, - size_two_entries), LZMA_DATA_ERROR); - index_two_entries[0] ^= 1; + index_two_records, &in_pos, + size_two_records), LZMA_DATA_ERROR); + index_two_records[0] ^= 1; // Next verify the index_hash as expected in_pos = 0; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_two_entries, &in_pos, - size_two_entries), LZMA_STREAM_END); + index_two_records, &in_pos, + size_two_records), LZMA_STREAM_END); - // Next test a three entry index hash + // Next test an index_hash with three Records index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 3); - const lzma_vli size_three_entries = lzma_index_hash_size( + const lzma_vli size_three_records = lzma_index_hash_size( index_hash); - assert_uint(size_three_entries, >, 0); - uint8_t *index_three_entries = tuktest_malloc(size_three_entries); + assert_uint(size_three_records, >, 0); + uint8_t *index_three_records = tuktest_malloc(size_three_records); - generate_index(index_three_entries, unpadded_sizes, uncomp_sizes, - 3, size_three_entries); + generate_index(index_three_records, unpadded_sizes, uncomp_sizes, + 3, size_three_records); in_pos = 0; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_three_entries, &in_pos, - size_three_entries), LZMA_STREAM_END); + index_three_records, &in_pos, + size_three_records), LZMA_STREAM_END); - // Next test a five entry index hash + // Next test an index_hash with five Records index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 5); - const lzma_vli size_five_entries = lzma_index_hash_size( + const lzma_vli size_five_records = lzma_index_hash_size( index_hash); - assert_uint(size_five_entries, >, 0); - uint8_t *index_five_entries = tuktest_malloc(size_five_entries); + assert_uint(size_five_records, >, 0); + uint8_t *index_five_records = tuktest_malloc(size_five_records); - generate_index(index_five_entries, unpadded_sizes, uncomp_sizes, 5, - size_five_entries); + generate_index(index_five_records, unpadded_sizes, uncomp_sizes, 5, + size_five_records); // Instead of testing all input at once, give input // one byte at a time in_pos = 0; - for (lzma_vli i = 0; i < size_five_entries - 1; ++i) { + for (lzma_vli i = 0; i < size_five_records - 1; ++i) { assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_five_entries, &in_pos, in_pos + 1), + index_five_records, &in_pos, in_pos + 1), LZMA_OK); } // Last byte should return LZMA_STREAM_END assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_five_entries, &in_pos, + index_five_records, &in_pos, in_pos + 1), LZMA_STREAM_END); - // Next test if the index hash is given an incorrect unpadded - // size. Should detect and report LZMA_DATA_ERROR + // Next test if the index_hash is given an incorrect Unpadded + // Size. Should detect and report LZMA_DATA_ERROR index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 5); - // The sixth entry will have invalid unpadded size + // The sixth Record will have an invalid Unpadded Size assert_lzma_ret(lzma_index_hash_append(index_hash, unpadded_sizes[5] + 1, uncomp_sizes[5]), LZMA_OK); - const lzma_vli size_six_entries = lzma_index_hash_size( + const lzma_vli size_six_records = lzma_index_hash_size( index_hash); - assert_uint(size_six_entries, >, 0); - uint8_t *index_six_entries = tuktest_malloc(size_six_entries); + assert_uint(size_six_records, >, 0); + uint8_t *index_six_records = tuktest_malloc(size_six_records); - generate_index(index_six_entries, unpadded_sizes, uncomp_sizes, 6, - size_six_entries); + generate_index(index_six_records, unpadded_sizes, uncomp_sizes, 6, + size_six_records); in_pos = 0; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_six_entries, &in_pos, - size_six_entries), LZMA_DATA_ERROR); + index_six_records, &in_pos, + size_six_records), LZMA_DATA_ERROR); // Next test if the Index is corrupt (invalid CRC32). // Should detect and report LZMA_DATA_ERROR index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); - index_two_entries[size_two_entries - 1] ^= 1; + index_two_records[size_two_records - 1] ^= 1; in_pos = 0; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_two_entries, &in_pos, - size_two_entries), LZMA_DATA_ERROR); + index_two_records, &in_pos, + size_two_records), LZMA_DATA_ERROR); - // Next test with index and index_hash struct not matching - // an entry + // Next test with Index and index_hash struct not matching + // a Record index_hash = lzma_index_hash_init(index_hash, NULL); fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); // Recalculate Index with invalid Unpadded Size @@ -293,13 +293,13 @@ test_lzma_index_hash_decode(void) unpadded_sizes[1] + 1 }; - generate_index(index_two_entries, unpadded_sizes_invalid, - uncomp_sizes, 2, size_two_entries); + generate_index(index_two_records, unpadded_sizes_invalid, + uncomp_sizes, 2, size_two_records); in_pos = 0; assert_lzma_ret(lzma_index_hash_decode(index_hash, - index_two_entries, &in_pos, - size_two_entries), LZMA_DATA_ERROR); + index_two_records, &in_pos, + size_two_records), LZMA_DATA_ERROR); lzma_index_hash_end(index_hash, NULL); #endif From 873e684028ba9738f071c5236db7d452ed797b4c Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:44:29 +0200 Subject: [PATCH 09/13] Tests: test_index_hash: Avoid the variable name "index". It can trigger warnings from -Wshadow on some systems. --- tests/test_index_hash.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index cb817a0e..a957891f 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -103,11 +103,11 @@ fill_index_hash(lzma_index_hash *index_hash, const lzma_vli *unpadded_sizes, #ifdef HAVE_ENCODERS -// Set the index parameter to the expected index based on the +// Set the contents of buf to the expected Index based on the // .xz specification. This needs the unpadded and uncompressed VLIs // to correctly create the Index. static void -generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, +generate_index(uint8_t *buf, const lzma_vli *unpadded_sizes, const lzma_vli *uncomp_sizes, uint32_t block_count, size_t index_max_size) { @@ -115,10 +115,10 @@ generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, size_t out_pos = 0; // First set Index Indicator - index[out_pos++] = INDEX_INDICATOR; + buf[out_pos++] = INDEX_INDICATOR; // Next write out Number of Records - assert_lzma_ret(lzma_vli_encode(block_count, &in_pos, index, + assert_lzma_ret(lzma_vli_encode(block_count, &in_pos, buf, &out_pos, index_max_size), LZMA_STREAM_END); // Next write out each Record. @@ -127,19 +127,19 @@ generate_index(uint8_t *index, const lzma_vli *unpadded_sizes, for (uint32_t i = 0; i < block_count; ++i) { in_pos = 0; assert_lzma_ret(lzma_vli_encode(unpadded_sizes[i], &in_pos, - index, &out_pos, index_max_size), LZMA_STREAM_END); + buf, &out_pos, index_max_size), LZMA_STREAM_END); in_pos = 0; assert_lzma_ret(lzma_vli_encode(uncomp_sizes[i], &in_pos, - index, &out_pos, index_max_size), LZMA_STREAM_END); + buf, &out_pos, index_max_size), LZMA_STREAM_END); } // Add Index Padding lzma_vli rounded_out_pos = vli_ceil4(out_pos); - memzero(index + out_pos, rounded_out_pos - out_pos); + memzero(buf + out_pos, rounded_out_pos - out_pos); out_pos = rounded_out_pos; // Add the CRC32 - write32le(index + out_pos, lzma_crc32(index, out_pos, 0)); + write32le(buf + out_pos, lzma_crc32(buf, out_pos, 0)); } #endif #endif From 056766c8601a3808bea1761f6cc833197a35a3e0 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:51:41 +0200 Subject: [PATCH 10/13] Tests: test_index_hash: Fix a typo in a comment. --- tests/test_index_hash.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index a957891f..63188dcb 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -176,7 +176,7 @@ test_lzma_index_hash_decode(void) 500 }; - // Add two Records to a index_hash + // Add two Records to an index_hash fill_index_hash(index_hash, unpadded_sizes, uncomp_sizes, 2); const lzma_vli size_two_records = lzma_index_hash_size(index_hash); From 02608f74ea1f2d2d56585711ff241c34b4ad0937 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:53:03 +0200 Subject: [PATCH 11/13] Tests: test_index_hash: Don't treat pointers as booleans. --- tests/test_index_hash.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index 63188dcb..9b7caade 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -60,7 +60,7 @@ test_lzma_index_hash_append(void) // Test with invalid Unpadded Size lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); - assert_true(index_hash); + assert_true(index_hash != NULL); assert_lzma_ret(lzma_index_hash_append(index_hash, UNPADDED_SIZE_MIN - 1, LZMA_VLI_MAX), LZMA_PROG_ERROR); @@ -152,7 +152,7 @@ test_lzma_index_hash_decode(void) assert_skip("Encoder or decoder support disabled"); #else lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); - assert_true(index_hash); + assert_true(index_hash != NULL); size_t in_pos = 0; @@ -313,7 +313,7 @@ test_lzma_index_hash_size(void) assert_skip("Decoder support disabled"); #else lzma_index_hash *index_hash = lzma_index_hash_init(NULL, NULL); - assert_true(index_hash); + assert_true(index_hash != NULL); // First test empty index_hash // Expected size should be: From d550304f5343b3a082da265107cd820e0d81dc71 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:55:06 +0200 Subject: [PATCH 12/13] Tests: test_index_hash: Fix a memory leak. --- tests/test_index_hash.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index 9b7caade..063ec0f2 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -367,6 +367,8 @@ test_lzma_index_hash_size(void) // Total - 16 bytes expected_size = 16; assert_uint_eq(lzma_index_hash_size(index_hash), expected_size); + + lzma_index_hash_end(index_hash, NULL); #endif } From fc0c788469159f634f09ff23c8cef6925c91da57 Mon Sep 17 00:00:00 2001 From: Lasse Collin Date: Fri, 6 Jan 2023 17:58:48 +0200 Subject: [PATCH 13/13] Tests: test_index_hash: Add an assert_uint_eq(). --- tests/test_index_hash.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_index_hash.c b/tests/test_index_hash.c index 063ec0f2..2cf91138 100644 --- a/tests/test_index_hash.c +++ b/tests/test_index_hash.c @@ -140,6 +140,9 @@ generate_index(uint8_t *buf, const lzma_vli *unpadded_sizes, // Add the CRC32 write32le(buf + out_pos, lzma_crc32(buf, out_pos, 0)); + out_pos += 4; + + assert_uint_eq(out_pos, index_max_size); } #endif #endif