aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLasse Collin <lasse.collin@tukaani.org>2022-04-06 23:11:59 +0300
committerLasse Collin <lasse.collin@tukaani.org>2022-04-06 23:11:59 +0300
commitfe87b4cd5364f5bbb6a75a0299f1500c852d7c9a (patch)
treef5565ca296cf0132254a5f6324961ee55907d614
parentliblzma: Add a new flag LZMA_FAIL_FAST for threaded decoder. (diff)
downloadxz-fe87b4cd5364f5bbb6a75a0299f1500c852d7c9a.tar.xz
liblzma: Threaded decoder: Improve setting of pending_error.
It doesn't need to be done conditionally. The comments try to explain it.
-rw-r--r--src/liblzma/common/stream_decoder_mt.c51
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: