Tests: test_filter_flags: Clean up minor issues.

Here are the list of the most significant issues addressed:
- Avoid using internal common.h header. It's not good to copy the
constants like this but common.h cannot be included for use outside
of liblzma. This is the quickest thing to do that could be fixed later.

- Omit the INIT_FILTER macro. Initialization should be done with just
regular designated initializers.

- Use start_offset = 257 for BCJ tests. It demonstrates that Filter
Flags encoder and decoder don't validate the options thoroughly.
257 is valid only for the x86 filter. This is a bit silly but
not a significant problem in practice because the encoder and
decoder initialization functions will catch bad alignment still.
Perhaps this should be fixed but it's not urgent and doesn't need
to be in 5.4.x.

- Various tweaks to comments such as filter id -> Filter ID
This commit is contained in:
Lasse Collin 2023-01-06 22:53:38 +02:00 committed by Jia Tan
parent 5c9fdd3bf5
commit 0c210ca7f4
1 changed files with 78 additions and 75 deletions

View File

@ -12,46 +12,41 @@
///////////////////////////////////////////////////////////////////////////////
#include "tests.h"
// Including the internal header file for access to the
// LZMA_FILTER_RESERVED_START macro
#include "common/common.h"
// Used to create filters and easily to set id and options
#define INIT_FILTER(_id, _options) {\
.id = _id, \
.options = _options \
}
// FIXME: This is from src/liblzma/common/common.h but it cannot be
// included here. This constant is needed in only a few files, perhaps
// move it to some other internal header or create a new one?
#define LZMA_FILTER_RESERVED_START (LZMA_VLI_C(1) << 62)
#if defined(HAVE_ENCODERS)
// No tests are run without encoders, so init the global filters
// only when the encoders are enabled.
static lzma_filter lzma1_filter = INIT_FILTER(LZMA_FILTER_LZMA1, NULL);
static lzma_filter lzma2_filter = INIT_FILTER(LZMA_FILTER_LZMA2, NULL);
static lzma_filter delta_filter = INIT_FILTER(LZMA_FILTER_DELTA, NULL);
static lzma_filter lzma1_filter = { LZMA_FILTER_LZMA1, NULL };
static lzma_filter lzma2_filter = { LZMA_FILTER_LZMA2, NULL };
static lzma_filter delta_filter = { LZMA_FILTER_DELTA, NULL };
static lzma_filter bcj_filters_encoders[] = {
#ifdef HAVE_ENCODER_X86
INIT_FILTER(LZMA_FILTER_X86, NULL),
{ LZMA_FILTER_X86, NULL },
#endif
#ifdef HAVE_ENCODER_POWERPC
INIT_FILTER(LZMA_FILTER_POWERPC, NULL),
{ LZMA_FILTER_POWERPC, NULL },
#endif
#ifdef HAVE_ENCODER_IA64
INIT_FILTER(LZMA_FILTER_IA64, NULL),
{ LZMA_FILTER_IA64, NULL },
#endif
#ifdef HAVE_ENCODER_ARM
INIT_FILTER(LZMA_FILTER_ARM, NULL),
{ LZMA_FILTER_ARM, NULL },
#endif
#ifdef HAVE_ENCODER_ARM64
INIT_FILTER(LZMA_FILTER_ARM64, NULL),
{ LZMA_FILTER_ARM64, NULL },
#endif
#ifdef HAVE_ENCODER_ARMTHUMB
INIT_FILTER(LZMA_FILTER_ARMTHUMB, NULL),
{ LZMA_FILTER_ARMTHUMB, NULL },
#endif
#ifdef HAVE_ENCODER_SPARC
INIT_FILTER(LZMA_FILTER_SPARC, NULL),
{ LZMA_FILTER_SPARC, NULL },
#endif
};
@ -62,25 +57,25 @@ static lzma_filter bcj_filters_encoders[] = {
#ifdef HAVE_DECODERS
static lzma_filter bcj_filters_decoders[] = {
#ifdef HAVE_DECODER_X86
INIT_FILTER(LZMA_FILTER_X86, NULL),
{ LZMA_FILTER_X86, NULL },
#endif
#ifdef HAVE_DECODER_POWERPC
INIT_FILTER(LZMA_FILTER_POWERPC, NULL),
{ LZMA_FILTER_POWERPC, NULL },
#endif
#ifdef HAVE_DECODER_IA64
INIT_FILTER(LZMA_FILTER_IA64, NULL),
{ LZMA_FILTER_IA64, NULL },
#endif
#ifdef HAVE_DECODER_ARM
INIT_FILTER(LZMA_FILTER_ARM, NULL),
{ LZMA_FILTER_ARM, NULL },
#endif
#ifdef HAVE_DECODER_ARM64
INIT_FILTER(LZMA_FILTER_ARM64, NULL),
{ LZMA_FILTER_ARM64, NULL },
#endif
#ifdef HAVE_DECODER_ARMTHUMB
INIT_FILTER(LZMA_FILTER_ARMTHUMB, NULL),
{ LZMA_FILTER_ARMTHUMB, NULL },
#endif
#ifdef HAVE_DECODER_SPARC
INIT_FILTER(LZMA_FILTER_SPARC, NULL),
{ LZMA_FILTER_SPARC, NULL },
#endif
};
#endif
@ -121,8 +116,8 @@ test_lzma_filter_flags_size(void)
assert_true(size != 0 && size < LZMA_BLOCK_HEADER_SIZE_MAX);
}
// Test invalid filter ids
lzma_filter bad_filter = INIT_FILTER(2, NULL);
// Test invalid Filter IDs
lzma_filter bad_filter = { 2, NULL };
assert_lzma_ret(lzma_filter_flags_size(&size, &bad_filter),
LZMA_OPTIONS_ERROR);
@ -142,15 +137,16 @@ test_lzma_filter_flags_size(void)
// Avoid data -> encode -> decode -> compare to data.
// Instead create expected encoding and compare to result from
// lzma_filter_flags_encode.
// Filter flags for xz are encoded as:
// Filter Flags in .xz are encoded as:
// |Filter ID (VLI)|Size of Properties (VLI)|Filter Properties|
#if defined(HAVE_ENCODERS) && defined(HAVE_DECODERS)
static void
verify_filter_flags_encode(lzma_filter *filter, bool should_encode)
{
uint32_t size = 0;
// First calculate the size of filter flags to know how much
// memory to allocate to hold the filter flags encoded
// First calculate the size of Filter Flags to know how much
// memory to allocate to hold the encoded Filter Flags
assert_lzma_ret(lzma_filter_flags_size(&size, filter), LZMA_OK);
uint8_t *encoded_out = tuktest_malloc(size * sizeof(uint8_t));
size_t out_pos = 0;
@ -160,20 +156,21 @@ verify_filter_flags_encode(lzma_filter *filter, bool should_encode)
return;
}
// Next encode the filter flags for the provided filter
// Next encode the Filter Flags for the provided filter
assert_lzma_ret(lzma_filter_flags_encode(filter, encoded_out,
&out_pos, size), LZMA_OK);
assert_uint_eq(size, out_pos);
// Next decode the vli for the filter ID and verify it matches
// the expected filter id
// Next decode the VLI for the Filter ID and verify it matches
// the expected Filter ID
size_t filter_id_vli_size = 0;
lzma_vli filter_id = 0;
assert_lzma_ret(lzma_vli_decode(&filter_id, NULL, encoded_out,
&filter_id_vli_size, size), LZMA_OK);
assert_uint_eq(filter->id, filter_id);
// Next decode the size of properites and ensure it equals
// the expected size
// Next decode the Size of Properites and ensure it equals
// the expected size.
// Expected size should be:
// total filter flag length - size of filter id VLI + size of
// property size VLI
@ -196,67 +193,72 @@ test_lzma_filter_flags_encode(void)
#if !defined(HAVE_ENCODERS) || !defined(HAVE_DECODERS)
assert_skip("Encoder or decoder support disabled");
#else
// No test for LZMA1 since the xz format does not support LZMA1
// No test for LZMA1 since the .xz format does not support LZMA1
// and so the flags cannot be encoded for that filter
if (lzma_filter_encoder_is_supported(LZMA_FILTER_LZMA2)) {
// Test with NULL options that should fail
lzma_options_lzma *options = lzma2_filter.options;
lzma2_filter.options = NULL;
verify_filter_flags_encode(&lzma2_filter, false);
// Place options back in the filter, and test should pass
lzma2_filter.options = options;
verify_filter_flags_encode(&lzma2_filter, true);
}
// NOTE: Many BCJ filters require that start_offset is a multiple
// of some power of two. The Filter Flags encoder and decoder don't
// completely validate the options and thus 257 passes the tests
// with all BCJ filters. It would be caught when initializing
// a filter chain encoder or decoder.
lzma_options_bcj bcj_options = {
.start_offset = 200
.start_offset = 257
};
for (uint32_t i = 0; i < ARRAY_SIZE(bcj_filters_encoders); i++) {
// NULL options should pass for bcj filters
verify_filter_flags_encode(&bcj_filters_encoders[i], true);
lzma_filter bcj_with_options = INIT_FILTER(
bcj_filters_encoders[i].id, &bcj_options);
lzma_filter bcj_with_options = {
bcj_filters_encoders[i].id, &bcj_options };
verify_filter_flags_encode(&bcj_with_options, true);
}
if (lzma_filter_encoder_is_supported(LZMA_FILTER_DELTA)) {
lzma_options_delta delta_ops_below_min = {
lzma_options_delta delta_opts_below_min = {
.type = LZMA_DELTA_TYPE_BYTE,
.dist = LZMA_DELTA_DIST_MIN - 1
};
lzma_options_delta delta_ops_above_max = {
lzma_options_delta delta_opts_above_max = {
.type = LZMA_DELTA_TYPE_BYTE,
.dist = LZMA_DELTA_DIST_MAX + 1
};
verify_filter_flags_encode(&delta_filter, true);
lzma_filter delta_filter_bad_options = INIT_FILTER(
LZMA_FILTER_DELTA, &delta_ops_below_min);
lzma_filter delta_filter_bad_options = {
LZMA_FILTER_DELTA, &delta_opts_below_min };
// Next test error case using minimum - 1 delta distance
verify_filter_flags_encode(&delta_filter_bad_options, false);
// Next test error case using maximum + 1 delta distance
delta_filter_bad_options.options = &delta_ops_above_max;
delta_filter_bad_options.options = &delta_opts_above_max;
verify_filter_flags_encode(&delta_filter_bad_options, false);
// Next test null case
// Next test NULL case
delta_filter_bad_options.options = NULL;
verify_filter_flags_encode(&delta_filter_bad_options, false);
}
// Test expected failing cases
lzma_filter bad_filter = INIT_FILTER(LZMA_FILTER_RESERVED_START,
NULL);
lzma_filter bad_filter = { LZMA_FILTER_RESERVED_START, NULL };
size_t out_pos = 0;
size_t out_size = LZMA_BLOCK_HEADER_SIZE_MAX;
uint8_t out[LZMA_BLOCK_HEADER_SIZE_MAX];
// Filter id outside of valid range
// Filter ID outside of valid range
assert_lzma_ret(lzma_filter_flags_encode(&bad_filter, out, &out_pos,
out_size), LZMA_PROG_ERROR);
out_pos = 0;
@ -265,7 +267,7 @@ test_lzma_filter_flags_encode(void)
out_size), LZMA_PROG_ERROR);
out_pos = 0;
// Invalid filter id
// Invalid Filter ID
bad_filter.id = 2;
assert_lzma_ret(lzma_filter_flags_encode(&bad_filter, out, &out_pos,
out_size), LZMA_OPTIONS_ERROR);
@ -294,6 +296,7 @@ test_lzma_filter_flags_encode(void)
// Invalid options
if (lzma_filter_encoder_is_supported(LZMA_FILTER_DELTA)) {
bad_filter.id = LZMA_FILTER_DELTA;
// First test with NULL options
assert_lzma_ret(lzma_filter_flags_encode(&bad_filter, out,
&out_pos, out_size), LZMA_PROG_ERROR);
@ -325,10 +328,12 @@ verify_filter_flags_decode(lzma_filter *filter_in, lzma_filter *filter_out)
assert_lzma_ret(lzma_filter_flags_size(&total_size, filter_in),
LZMA_OK);
assert_uint(total_size, >, 0);
uint8_t *filter_flag_buffer = tuktest_malloc(total_size);
uint32_t properties_size = 0;
size_t out_pos = 0, in_pos = 0;
size_t out_pos = 0;
size_t in_pos = 0;
assert_lzma_ret(lzma_properties_size(&properties_size, filter_in),
LZMA_OK);
assert_lzma_ret(lzma_vli_encode(filter_in->id, NULL,
@ -353,12 +358,11 @@ test_lzma_filter_flags_decode(void)
assert_skip("Encoder or decoder support disabled");
#else
// For each filter, only run the decoder test if both the encoder
// and decoder is enabled. This is because verify_filter_flags_decode
// uses lzma_filter_flags_size, which requires the encoder.
// and decoder are enabled. This is because verify_filter_flags_decode
// uses lzma_filter_flags_size which requires the encoder.
if (lzma_filter_decoder_is_supported(LZMA_FILTER_LZMA2) &&
lzma_filter_encoder_is_supported(LZMA_FILTER_LZMA2)) {
lzma_filter lzma2_decoded = INIT_FILTER(LZMA_FILTER_LZMA2,
NULL);
lzma_filter lzma2_decoded = { LZMA_FILTER_LZMA2, NULL };
verify_filter_flags_decode(&lzma2_filter, &lzma2_decoded);
@ -376,11 +380,11 @@ test_lzma_filter_flags_decode(void)
for (uint32_t i = 0; i < ARRAY_SIZE(bcj_filters_decoders); i++) {
if (lzma_filter_encoder_is_supported(
bcj_filters_decoders[i].id)) {
lzma_filter bcj_decoded = INIT_FILTER(
bcj_filters_decoders[i].id, NULL);
lzma_filter bcj_decoded = {
bcj_filters_decoders[i].id, NULL };
lzma_filter bcj_encoded = INIT_FILTER(
bcj_filters_decoders[i].id, NULL);
lzma_filter bcj_encoded = {
bcj_filters_decoders[i].id, NULL };
// First test without options
verify_filter_flags_decode(&bcj_encoded,
@ -389,24 +393,22 @@ test_lzma_filter_flags_decode(void)
// Next test with offset
lzma_options_bcj options = {
.start_offset = 200
.start_offset = 257
};
bcj_encoded.options = &options;
verify_filter_flags_decode(&bcj_encoded,
&bcj_decoded);
lzma_options_bcj *decoded_ops = bcj_decoded.options;
assert_uint_eq(decoded_ops->start_offset,
lzma_options_bcj *decoded_opts = bcj_decoded.options;
assert_uint_eq(decoded_opts->start_offset,
options.start_offset);
free(decoded_ops);
free(decoded_opts);
}
}
if (lzma_filter_decoder_is_supported(LZMA_FILTER_DELTA) &&
lzma_filter_encoder_is_supported(LZMA_FILTER_DELTA)) {
lzma_filter delta_decoded = INIT_FILTER(LZMA_FILTER_DELTA,
NULL);
lzma_filter delta_decoded = { LZMA_FILTER_DELTA, NULL };
verify_filter_flags_decode(&delta_filter, &delta_decoded);
lzma_options_delta *expected = delta_filter.options;
@ -421,7 +423,7 @@ test_lzma_filter_flags_decode(void)
uint8_t bad_encoded_filter[LZMA_BLOCK_HEADER_SIZE_MAX];
lzma_filter bad_filter;
// Filter outside of valid range
// Filter ID outside of valid range
lzma_vli bad_filter_id = LZMA_FILTER_RESERVED_START;
size_t bad_encoded_out_pos = 0;
size_t in_pos = 0;
@ -437,7 +439,7 @@ test_lzma_filter_flags_decode(void)
bad_encoded_out_pos = 0;
in_pos = 0;
// Invalid filter Id
// Invalid Filter ID
bad_filter_id = 2;
bad_encoded_out_pos = 0;
in_pos = 0;
@ -446,11 +448,12 @@ test_lzma_filter_flags_decode(void)
bad_encoded_filter, &bad_encoded_out_pos,
LZMA_BLOCK_HEADER_SIZE_MAX), LZMA_OK);
// Next encode propery size of 0
// Next encode Size of Properties with the value of 0
assert_lzma_ret(lzma_vli_encode(0, NULL,
bad_encoded_filter, &bad_encoded_out_pos,
LZMA_BLOCK_HEADER_SIZE_MAX), LZMA_OK);
// Decode should fail on bad filter id
// Decode should fail on bad Filter ID
assert_lzma_ret(lzma_filter_flags_decode(&bad_filter, NULL,
bad_encoded_filter, &in_pos,
LZMA_BLOCK_HEADER_SIZE_MAX), LZMA_OPTIONS_ERROR);
@ -458,7 +461,7 @@ test_lzma_filter_flags_decode(void)
in_pos = 0;
// Outsize too small
// Encode the lzma2 filter normally, but then set
// Encode the LZMA2 filter normally, but then set
// the out size when decoding as too small
if (lzma_filter_encoder_is_supported(LZMA_FILTER_LZMA2) &&
lzma_filter_decoder_is_supported(LZMA_FILTER_LZMA2)) {