liblzma: Threaded decoder: Improve setting of pending_error.

It doesn't need to be done conditionally. The comments try
to explain it.

1 files changed, 35 insertions, 16 deletions

diff --git a/src/liblzma/common/stream_decoder_mt.c b/src/liblzma/common/stream_decoder_mt.c
index 32e0b892..e8939254 100644
--- a/src/liblzma/common/stream_decoder_mt.c
+++ b/src/liblzma/common/stream_decoder_mt.c
@@ -711,16 +711,29 @@ read_output_and_wait(struct lzma_stream_coder *coder,
 
 			// Check if any thread has indicated an error.
 			if (coder->thread_error != LZMA_OK) {
-				if (coder->pending_error == LZMA_OK)
-					coder->pending_error
-							= coder->thread_error;
-
 				// If LZMA_FAIL_FAST was used, report errors
 				// from worker threads immediately.
 				if (coder->fail_fast) {
 					ret = coder->thread_error;
 					break;
 				}
+
+				// Otherwise set pending_error. The value we
+				// set here will not actually get used other
+				// than working as a flag that an error has
+				// occurred. This is because in SEQ_ERROR
+				// all output before the error will be read
+				// first by calling this function, and once we
+				// reach the location of the (first) error the
+				// error code from the above lzma_outq_read()
+				// will be returned to the application.
+				//
+				// Use LZMA_PROG_ERROR since the value should
+				// never leak to the application. It's
+				// possible that pending_error has already
+				// been set but that doesn't matter: if we get
+				// here, pending_error only works as a flag.
+				coder->pending_error = LZMA_PROG_ERROR;
 			}
 
 			// Check if decoding of the next Block can be started.
@@ -1150,9 +1163,18 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
 
 		// See if an error occurred.
 		if (ret != LZMA_STREAM_END) {
-			if (coder->pending_error == LZMA_OK)
-				coder->pending_error = ret;
-
+			// NOTE: Here and in all other places where
+			// pending_error is set, it may overwrite the value
+			// (LZMA_PROG_ERROR) set by read_output_and_wait().
+			// That function might overwrite value set here too.
+			// These are fine because when read_output_and_wait()
+			// sets pending_error, it actually works as a flag
+			// variable only ("some error has occurred") and the
+			// actual value of pending_error is not used in
+			// SEQ_ERROR. In such cases SEQ_ERROR will eventually
+			// get the correct error code from the return value of
+			// a later read_output_and_wait() call.
+			coder->pending_error = ret;
 			coder->sequence = SEQ_ERROR;
 			break;
 		}
@@ -1163,9 +1185,7 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
 
 		if (coder->mem_next_filters == UINT64_MAX) {
 			// One or more unknown Filter IDs.
-			if (coder->pending_error == LZMA_OK)
-				coder->pending_error = LZMA_OPTIONS_ERROR;
-
+			coder->pending_error = LZMA_OPTIONS_ERROR;
 			coder->sequence = SEQ_ERROR;
 			break;
 		}
@@ -1254,9 +1274,7 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
 					&coder->block_options),
 				coder->block_options.uncompressed_size);
 		if (ret != LZMA_OK) {
-			if (coder->pending_error == LZMA_OK)
-				coder->pending_error = ret;
-
+			coder->pending_error = ret;
 			coder->sequence = SEQ_ERROR;
 			break;
 		}
@@ -1425,9 +1443,7 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
 		// Check if memory usage calculation and Block encoder
 		// initialization succeeded.
 		if (ret != LZMA_OK) {
-			if (coder->pending_error == LZMA_OK)
-				coder->pending_error = ret;
-
+			coder->pending_error = ret;
 			coder->sequence = SEQ_ERROR;
 			break;
 		}
@@ -1712,6 +1728,9 @@ stream_decode_mt(void *coder_ptr, const lzma_allocator *allocator,
 				return LZMA_OK;
 		}
 
+		// We only get here if no errors were detected by the worker
+		// threads. Errors from worker threads would have already been
+		// returned by the call to read_output_and_wait() above.
 		return coder->pending_error;
 
 	default: