Fix a design error in liblzma API.

Originally the idea was that using LZMA_FULL_FLUSH
with Stream encoder would read the filter chain
from the same array that was used to intialize the
Stream encoder. Since most apps wouldn't use
LZMA_FULL_FLUSH, most apps wouldn't need to keep
the filter chain available after initializing the
Stream encoder. However, due to my mistake, it
actually required keeping the array always available.

Since setting the new filter chain via the array
used at initialization time is not a nice way to do
it for a couple of reasons, this commit ditches it
and introduces lzma_filters_update(). This new function
replaces also the "persistent" flag used by LZMA2
(and to-be-designed Subblock filter), which was also
an ugly thing to do.

Thanks to Alexey Tourbin for reminding me about the problem
that Stream encoder used to require keeping the filter
chain allocated.

8 files changed, 156 insertions, 71 deletions

diff --git a/src/liblzma/common/block_encoder.c b/src/liblzma/common/block_encoder.c
index 567889aa..ca515235 100644
--- a/src/liblzma/common/block_encoder.c
+++ b/src/liblzma/common/block_encoder.c
@@ -142,6 +142,19 @@ block_encoder_end(lzma_coder *coder, lzma_allocator *allocator)
 }
 
 
+static lzma_ret
+block_encoder_update(lzma_coder *coder, lzma_allocator *allocator,
+		const lzma_filter *filters lzma_attribute((unused)),
+		const lzma_filter *reversed_filters)
+{
+	if (coder->sequence != SEQ_CODE)
+		return LZMA_PROG_ERROR;
+
+	return lzma_next_filter_update(
+			&coder->next, allocator, reversed_filters);
+}
+
+
 extern lzma_ret
 lzma_block_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 		lzma_block *block)
@@ -167,6 +180,7 @@ lzma_block_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 
 		next->code = &block_encode;
 		next->end = &block_encoder_end;
+		next->update = &block_encoder_update;
 		next->coder->next = LZMA_NEXT_CODER_INIT;
 	}
 
diff --git a/src/liblzma/common/common.c b/src/liblzma/common/common.c
index 3bdf3252..edce90cd 100644
--- a/src/liblzma/common/common.c
+++ b/src/liblzma/common/common.c
@@ -92,12 +92,30 @@ lzma_next_filter_init(lzma_next_coder *next, lzma_allocator *allocator,
 		const lzma_filter_info *filters)
 {
 	lzma_next_coder_init(filters[0].init, next, allocator);
-
+	next->id = filters[0].id;
 	return filters[0].init == NULL
 			? LZMA_OK : filters[0].init(next, allocator, filters);
 }
 
 
+extern lzma_ret
+lzma_next_filter_update(lzma_next_coder *next, lzma_allocator *allocator,
+		const lzma_filter *reversed_filters)
+{
+	// Check that the application isn't trying to change the Filter ID.
+	// End of filters is indicated with LZMA_VLI_UNKNOWN in both
+	// reversed_filters[0].id and next->id.
+	if (reversed_filters[0].id != next->id)
+		return LZMA_PROG_ERROR;
+
+	if (reversed_filters[0].id == LZMA_VLI_UNKNOWN)
+		return LZMA_OK;
+
+	assert(next->update != NULL);
+	return next->update(next->coder, allocator, NULL, reversed_filters);
+}
+
+
 extern void
 lzma_next_end(lzma_next_coder *next, lzma_allocator *allocator)
 {
diff --git a/src/liblzma/common/common.h b/src/liblzma/common/common.h
index 81f51421..6551e39f 100644
--- a/src/liblzma/common/common.h
+++ b/src/liblzma/common/common.h
@@ -109,6 +109,10 @@ typedef void (*lzma_end_function)(
 /// an array of lzma_filter_info structures. This array is used with
 /// lzma_next_filter_init to initialize the filter chain.
 struct lzma_filter_info_s {
+	/// Filter ID. This is used only by the encoder
+	/// with lzma_filters_update().
+	lzma_vli id;
+
 	/// Pointer to function used to initialize the filter.
 	/// This is NULL to indicate end of array.
 	lzma_init_function init;
@@ -123,6 +127,10 @@ struct lzma_next_coder_s {
 	/// Pointer to coder-specific data
 	lzma_coder *coder;
 
+	/// Filter ID. This is LZMA_VLI_UNKNOWN when this structure doesn't
+	/// point to a filter coder.
+	lzma_vli id;
+
 	/// "Pointer" to init function. This is never called here.
 	/// We need only to detect if we are initializing a coder
 	/// that was allocated earlier. See lzma_next_coder_init and
@@ -145,6 +153,12 @@ struct lzma_next_coder_s {
 	/// If new_memlimit == 0, the limit is not changed.
 	lzma_ret (*memconfig)(lzma_coder *coder, uint64_t *memusage,
 			uint64_t *old_memlimit, uint64_t new_memlimit);
+
+	/// Update the filter-specific options or the whole filter chain
+	/// in the encoder.
+	lzma_ret (*update)(lzma_coder *coder, lzma_allocator *allocator,
+			const lzma_filter *filters,
+			const lzma_filter *reversed_filters);
 };
 
 
@@ -153,10 +167,12 @@ struct lzma_next_coder_s {
 	(lzma_next_coder){ \
 		.coder = NULL, \
 		.init = (uintptr_t)(NULL), \
+		.id = LZMA_VLI_UNKNOWN, \
 		.code = NULL, \
 		.end = NULL, \
 		.get_check = NULL, \
 		.memconfig = NULL, \
+		.update = NULL, \
 	}
 
 
@@ -212,6 +228,12 @@ extern lzma_ret lzma_strm_init(lzma_stream *strm);
 extern lzma_ret lzma_next_filter_init(lzma_next_coder *next,
 		lzma_allocator *allocator, const lzma_filter_info *filters);
 
+/// Update the next filter in the chain, if any. This checks that
+/// the application is not trying to change the Filter IDs.
+extern lzma_ret lzma_next_filter_update(
+		lzma_next_coder *next, lzma_allocator *allocator,
+		const lzma_filter *reversed_filters);
+
 /// Frees the memory allocated for next->coder either using next->end or,
 /// if next->end is NULL, using lzma_free.
 extern void lzma_next_end(lzma_next_coder *next, lzma_allocator *allocator);
diff --git a/src/liblzma/common/easy_encoder.c b/src/liblzma/common/easy_encoder.c
index 5e2641c9..d13ccd73 100644
--- a/src/liblzma/common/easy_encoder.c
+++ b/src/liblzma/common/easy_encoder.c
@@ -14,67 +14,12 @@
 #include "stream_encoder.h"
 
 
-struct lzma_coder_s {
-	lzma_next_coder stream_encoder;
-	lzma_options_easy opt_easy;
-};
-
-
-static lzma_ret
-easy_encode(lzma_coder *coder, lzma_allocator *allocator,
-		const uint8_t *restrict in, size_t *restrict in_pos,
-		size_t in_size, uint8_t *restrict out,
-		size_t *restrict out_pos, size_t out_size, lzma_action action)
-{
-	return coder->stream_encoder.code(
-			coder->stream_encoder.coder, allocator,
-			in, in_pos, in_size, out, out_pos, out_size, action);
-}
-
-
-static void
-easy_encoder_end(lzma_coder *coder, lzma_allocator *allocator)
-{
-	lzma_next_end(&coder->stream_encoder, allocator);
-	lzma_free(coder, allocator);
-	return;
-}
-
-
-static lzma_ret
-easy_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
-		uint32_t preset, lzma_check check)
-{
-	lzma_next_coder_init(&easy_encoder_init, next, allocator);
-
-	if (next->coder == NULL) {
-		next->coder = lzma_alloc(sizeof(lzma_coder), allocator);
-		if (next->coder == NULL)
-			return LZMA_MEM_ERROR;
-
-		next->code = &easy_encode;
-		next->end = &easy_encoder_end;
-
-		next->coder->stream_encoder = LZMA_NEXT_CODER_INIT;
-	}
-
-	if (lzma_easy_preset(&next->coder->opt_easy, preset))
-		return LZMA_OPTIONS_ERROR;
-
-	return lzma_stream_encoder_init(&next->coder->stream_encoder,
-			allocator, next->coder->opt_easy.filters, check);
-}
-
-
 extern LZMA_API(lzma_ret)
 lzma_easy_encoder(lzma_stream *strm, uint32_t preset, lzma_check check)
 {
-	lzma_next_strm_init(easy_encoder_init, strm, preset, check);
-
-	strm->internal->supported_actions[LZMA_RUN] = true;
-	strm->internal->supported_actions[LZMA_SYNC_FLUSH] = true;
-	strm->internal->supported_actions[LZMA_FULL_FLUSH] = true;
-	strm->internal->supported_actions[LZMA_FINISH] = true;
+	lzma_options_easy opt_easy;
+	if (lzma_easy_preset(&opt_easy, preset))
+		return LZMA_OPTIONS_ERROR;
 
-	return LZMA_OK;
+	return lzma_stream_encoder(strm, opt_easy.filters, check);
 }
diff --git a/src/liblzma/common/filter_common.c b/src/liblzma/common/filter_common.c
index 055093f7..2322d7de 100644
--- a/src/liblzma/common/filter_common.c
+++ b/src/liblzma/common/filter_common.c
@@ -270,6 +270,7 @@ lzma_raw_coder_init(lzma_next_coder *next, lzma_allocator *allocator,
 			if (fc == NULL || fc->init == NULL)
 				return LZMA_OPTIONS_ERROR;
 
+			filters[j].id = options[i].id;
 			filters[j].init = fc->init;
 			filters[j].options = options[i].options;
 		}
@@ -280,12 +281,14 @@ lzma_raw_coder_init(lzma_next_coder *next, lzma_allocator *allocator,
 			if (fc == NULL || fc->init == NULL)
 				return LZMA_OPTIONS_ERROR;
 
+			filters[i].id = options[i].id;
 			filters[i].init = fc->init;
 			filters[i].options = options[i].options;
 		}
 	}
 
 	// Terminate the array.
+	filters[count].id = LZMA_VLI_UNKNOWN;
 	filters[count].init = NULL;
 
 	// Initialize the filters.
diff --git a/src/liblzma/common/filter_encoder.c b/src/liblzma/common/filter_encoder.c
index d6fb82e5..3b0493fe 100644
--- a/src/liblzma/common/filter_encoder.c
+++ b/src/liblzma/common/filter_encoder.c
@@ -180,6 +180,33 @@ lzma_filter_encoder_is_supported(lzma_vli id)
 }
 
 
+extern LZMA_API(lzma_ret)
+lzma_filters_update(lzma_stream *strm, const lzma_filter *filters)
+{
+	if (strm->internal->next.update == NULL)
+		return LZMA_PROG_ERROR;
+
+	// Validate the filter chain.
+	if (lzma_raw_encoder_memusage(filters) == UINT64_MAX)
+		return LZMA_OPTIONS_ERROR;
+
+	// The actual filter chain in the encoder is reversed. Some things
+	// still want the normal order chain, so we provide both.
+	size_t count = 1;
+	while (filters[count].id != LZMA_VLI_UNKNOWN)
+		++count;
+
+	lzma_filter reversed_filters[LZMA_FILTERS_MAX + 1];
+	for (size_t i = 0; i < count; ++i)
+		reversed_filters[count - i - 1] = filters[i];
+
+	reversed_filters[count].id = LZMA_VLI_UNKNOWN;
+
+	return strm->internal->next.update(strm->internal->next.coder,
+			strm->allocator, filters, reversed_filters);
+}
+
+
 extern lzma_ret
 lzma_raw_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 		const lzma_filter *options)
diff --git a/src/liblzma/common/filter_encoder.h b/src/liblzma/common/filter_encoder.h
index 5b65cd30..a978932d 100644
--- a/src/liblzma/common/filter_encoder.h
+++ b/src/liblzma/common/filter_encoder.h
@@ -22,6 +22,6 @@ extern lzma_vli lzma_chunk_size(const lzma_filter *filters);
 
 extern lzma_ret lzma_raw_encoder_init(
 		lzma_next_coder *next, lzma_allocator *allocator,
-		const lzma_filter *options);
+		const lzma_filter *filters);
 
 #endif
diff --git a/src/liblzma/common/stream_encoder.c b/src/liblzma/common/stream_encoder.c
index 292efc82..705ec0eb 100644
--- a/src/liblzma/common/stream_encoder.c
+++ b/src/liblzma/common/stream_encoder.c
@@ -25,12 +25,20 @@ struct lzma_coder_s {
 		SEQ_STREAM_FOOTER,
 	} sequence;
 
+	/// True if Block encoder has been initialized by
+	/// lzma_stream_encoder_init() or stream_encoder_update()
+	/// and thus doesn't need to be initialized in stream_encode().
+	bool block_encoder_is_initialized;
+
 	/// Block
 	lzma_next_coder block_encoder;
 
 	/// Options for the Block encoder
 	lzma_block block_options;
 
+	/// The filter chain currently in use
+	lzma_filter filters[LZMA_FILTERS_MAX + 1];
+
 	/// Index encoder. This is separate from Block encoder, because this
 	/// doesn't take much memory, and when encoding multiple Streams
 	/// with the same encoding options we avoid reallocating memory.
@@ -117,12 +125,16 @@ stream_encode(lzma_coder *coder, lzma_allocator *allocator,
 			break;
 		}
 
-		// Initialize the Block encoder except if this is the first
-		// Block, because stream_encoder_init() has already
-		// initialized it.
-		if (lzma_index_count(coder->index) != 0)
+		// Initialize the Block encoder unless it was already
+		// initialized by lzma_stream_encoder_init() or
+		// stream_encoder_update().
+		if (!coder->block_encoder_is_initialized)
 			return_if_error(block_encoder_init(coder, allocator));
 
+		// Make it false so that we don't skip the initialization
+		// with the next Block.
+		coder->block_encoder_is_initialized = false;
+
 		// Encode the Block Header. This shouldn't fail since we have
 		// already initialized the Block encoder.
 		if (lzma_block_header_encode(&coder->block_options,
@@ -202,11 +214,54 @@ stream_encoder_end(lzma_coder *coder, lzma_allocator *allocator)
 	lzma_next_end(&coder->block_encoder, allocator);
 	lzma_next_end(&coder->index_encoder, allocator);
 	lzma_index_end(coder->index, allocator);
+
+	for (size_t i = 0; coder->filters[i].id != LZMA_VLI_UNKNOWN; ++i)
+		lzma_free(coder->filters[i].options, allocator);
+
 	lzma_free(coder, allocator);
 	return;
 }
 
 
+static lzma_ret
+stream_encoder_update(lzma_coder *coder, lzma_allocator *allocator,
+		const lzma_filter *filters,
+		const lzma_filter *reversed_filters)
+{
+	if (coder->sequence <= SEQ_BLOCK_INIT) {
+		// There is no incomplete Block waiting to be finished,
+		// thus we can change the whole filter chain. Start by
+		// trying to initialize the Block encoder with the new
+		// chain. This way we detect if the chain is valid.
+		coder->block_encoder_is_initialized = false;
+		coder->block_options.filters = (lzma_filter *)(filters);
+		const lzma_ret ret = block_encoder_init(coder, allocator);
+		coder->block_options.filters = coder->filters;
+		if (ret != LZMA_OK)
+			return ret;
+
+		coder->block_encoder_is_initialized = true;
+
+	} else if (coder->sequence <= SEQ_BLOCK_ENCODE) {
+		// We are in the middle of a Block. Try to update only
+		// the filter-specific options.
+		return_if_error(coder->block_encoder.update(
+				coder->block_encoder.coder, allocator,
+				filters, reversed_filters));
+	} else {
+		// Trying to update the filter chain when we are already
+		// encoding Index or Stream Footer.
+		return LZMA_PROG_ERROR;
+	}
+
+	// Free the copy of the old chain and make a copy of the new chain.
+	for (size_t i = 0; coder->filters[i].id != LZMA_VLI_UNKNOWN; ++i)
+		lzma_free(coder->filters[i].options, allocator);
+
+	return lzma_filters_copy(filters, coder->filters, allocator);
+}
+
+
 extern lzma_ret
 lzma_stream_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 		const lzma_filter *filters, lzma_check check)
@@ -223,6 +278,7 @@ lzma_stream_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 
 		next->code = &stream_encode;
 		next->end = &stream_encoder_end;
+		next->update = &stream_encoder_update;
 
 		next->coder->block_encoder = LZMA_NEXT_CODER_INIT;
 		next->coder->index_encoder = LZMA_NEXT_CODER_INIT;
@@ -233,7 +289,7 @@ lzma_stream_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 	next->coder->sequence = SEQ_STREAM_HEADER;
 	next->coder->block_options.version = 0;
 	next->coder->block_options.check = check;
-	next->coder->block_options.filters = (lzma_filter *)(filters);
+	next->coder->filters[0].id = LZMA_VLI_UNKNOWN;
 
 	// Initialize the Index
 	next->coder->index = lzma_index_init(next->coder->index, allocator);
@@ -251,11 +307,11 @@ lzma_stream_encoder_init(lzma_next_coder *next, lzma_allocator *allocator,
 	next->coder->buffer_pos = 0;
 	next->coder->buffer_size = LZMA_STREAM_HEADER_SIZE;
 
-	// Initialize the Block encoder. This way we detect if the given
-	// filters are supported by the current liblzma build, and the
-	// application doesn't need to keep the filters structure available
-	// unless it is going to use LZMA_FULL_FLUSH.
-	return block_encoder_init(next->coder, allocator);
+	// Initialize the Block encoder. This way we detect unsupported
+	// filter chains when initializing the Stream encoder instead of
+	// giving an error after Stream Header has already written out.
+	return stream_encoder_update(
+			next->coder, allocator, filters, NULL);
 }