[PATCH 1/2] lib/xz: Use size_t instead of uint32_t in a few places

[PATCH 1/2] lib/xz: Use size_t instead of uint32_t in a few places

[Lasse Collin]

Reduce the number of uint32_t <-> size_t conversions a little. Eliminating
such conversions entirely would require changing almost all uint32_t to
size_t, which would look confusing and increase the sizes of the structs
even more. Going the other way, converting everything to uint32_t, isn't
possible because the input and output buffers use size_t in struct xz_buf.

Now both arguments to min() have the same type. This is required to for
compatibility with PowerPC boot code[1] whose min() is strict like
include/linux/minmax.h was before the commit d03eba99f5bf ("minmax: allow
min()/max()/clamp() if the arguments have the same signedness.").

Swap the order of the "state" and "len" in struct lzma_dec to avoid
padding in the middle of the struct when size_t is 64 bits. The
reordering doesn't change the size of the struct; the padding just
appears at the end instead.

dict_flush() used to truncate size_t to uint32_t when returning.
This wasn't a bug; the value is always small enough.

Reported-by: Nathan Chancellor <nathan@kernel.org>
Closes: https://lore.kernel.org/lkml/20260610232323.GA1071374@ax162/ [1]
Cc: Thorsten Blum <thorsten.blum@linux.dev>
Cc: David Laight <david.laight.linux@gmail.com>
Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>
---
 lib/xz/xz_dec_lzma2.c | 40 ++++++++++++++++++++--------------------
 lib/xz/xz_lzma2.h     |  2 +-
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
index 9d80342b9c6b..68ae9c33b6a8 100644
--- a/lib/xz/xz_dec_lzma2.c
+++ b/lib/xz/xz_dec_lzma2.c
@@ -135,14 +135,16 @@ struct lzma_dec {
 	uint32_t rep2;
 	uint32_t rep3;
 
-	/* Types of the most recently seen LZMA symbols */
-	enum lzma_state state;
-
 	/*
 	 * Length of a match. This is updated so that dict_repeat can
-	 * be called again to finish repeating the whole match.
+	 * be called again to finish repeating the whole match. This is
+	 * size_t because a pointer to this is passed to dict_repeat,
+	 * and there it's nicer to have size_t instead of uint32_t.
 	 */
-	uint32_t len;
+	size_t len;
+
+	/* Types of the most recently seen LZMA symbols */
+	enum lzma_state state;
 
 	/*
 	 * LZMA properties or related bit masks (number of literal
@@ -228,13 +230,13 @@ struct lzma2_dec {
 	enum lzma2_seq next_sequence;
 
 	/* Uncompressed size of LZMA chunk (2 MiB at maximum) */
-	uint32_t uncompressed;
+	size_t uncompressed;
 
 	/*
 	 * Compressed size of LZMA chunk or compressed/uncompressed
 	 * size of uncompressed chunk (64 KiB at maximum)
 	 */
-	uint32_t compressed;
+	size_t compressed;
 
 	/*
 	 * True if dictionary reset is needed. This is false before
@@ -273,7 +275,7 @@ struct xz_dec_lzma2 {
 	 * decoder calls. See lzma2_lzma() for details.
 	 */
 	struct {
-		uint32_t size;
+		size_t size;
 		uint8_t buf[3 * LZMA_IN_REQUIRED];
 	} temp;
 };
@@ -320,7 +322,7 @@ static inline bool dict_has_space(const struct dictionary *dict)
  * still empty. This special case is needed for single-call decoding to
  * avoid writing a '\0' to the end of the destination buffer.
  */
-static inline uint32_t dict_get(const struct dictionary *dict, uint32_t dist)
+static inline uint32_t dict_get(const struct dictionary *dict, size_t dist)
 {
 	size_t offset = dict->pos - dist - 1;
 
@@ -346,10 +348,10 @@ static inline void dict_put(struct dictionary *dict, uint8_t byte)
  * invalid, false is returned. On success, true is returned and *len is
  * updated to indicate how many bytes were left to be repeated.
  */
-static bool dict_repeat(struct dictionary *dict, uint32_t *len, uint32_t dist)
+static bool dict_repeat(struct dictionary *dict, size_t *len, size_t dist)
 {
 	size_t back;
-	uint32_t left;
+	size_t left;
 
 	if (dist >= dict->full || dist >= dict->size)
 		return false;
@@ -375,7 +377,7 @@ static bool dict_repeat(struct dictionary *dict, uint32_t *len, uint32_t dist)
 
 /* Copy uncompressed data as is from input to dictionary and output buffers. */
 static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b,
-			      uint32_t *left)
+			      size_t *left)
 {
 	size_t copy_size;
 
@@ -433,7 +435,7 @@ static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b,
  * enough space in b->out. This is guaranteed because caller uses dict_limit()
  * before decoding data into the dictionary.
  */
-static uint32_t dict_flush(struct dictionary *dict, struct xz_buf *b)
+static size_t dict_flush(struct dictionary *dict, struct xz_buf *b)
 {
 	size_t copy_size = dict->pos - dict->start;
 
@@ -878,7 +880,7 @@ static bool lzma_props(struct xz_dec_lzma2 *s, uint8_t props)
 static bool lzma2_lzma(struct xz_dec_lzma2 *s, struct xz_buf *b)
 {
 	size_t in_avail;
-	uint32_t tmp;
+	size_t tmp;
 
 	in_avail = b->in_size - b->in_pos;
 	if (s->temp.size > 0 || s->lzma2.compressed == 0) {
@@ -1046,25 +1048,23 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
 
 		case SEQ_UNCOMPRESSED_1:
 			s->lzma2.uncompressed
-					+= (uint32_t)b->in[b->in_pos++] << 8;
+					+= (size_t)b->in[b->in_pos++] << 8;
 			s->lzma2.sequence = SEQ_UNCOMPRESSED_2;
 			break;
 
 		case SEQ_UNCOMPRESSED_2:
 			s->lzma2.uncompressed
-					+= (uint32_t)b->in[b->in_pos++] + 1;
+					+= (size_t)b->in[b->in_pos++] + 1;
 			s->lzma2.sequence = SEQ_COMPRESSED_0;
 			break;
 
 		case SEQ_COMPRESSED_0:
-			s->lzma2.compressed
-					= (uint32_t)b->in[b->in_pos++] << 8;
+			s->lzma2.compressed = (size_t)b->in[b->in_pos++] << 8;
 			s->lzma2.sequence = SEQ_COMPRESSED_1;
 			break;
 
 		case SEQ_COMPRESSED_1:
-			s->lzma2.compressed
-					+= (uint32_t)b->in[b->in_pos++] + 1;
+			s->lzma2.compressed += (size_t)b->in[b->in_pos++] + 1;
 			s->lzma2.sequence = s->lzma2.next_sequence;
 			break;
 
diff --git a/lib/xz/xz_lzma2.h b/lib/xz/xz_lzma2.h
index d2632b7dfb9c..a612ce4fd450 100644
--- a/lib/xz/xz_lzma2.h
+++ b/lib/xz/xz_lzma2.h
@@ -143,7 +143,7 @@ static inline bool lzma_state_is_literal(enum lzma_state state)
  * Get the index of the appropriate probability array for decoding
  * the distance slot.
  */
-static inline uint32_t lzma_get_dist_state(uint32_t len)
+static inline size_t lzma_get_dist_state(size_t len)
 {
 	return len < DIST_STATES + MATCH_LEN_MIN
 			? len - MATCH_LEN_MIN : DIST_STATES - 1;
-- 
2.54.0

[PATCH 2/2] lib/xz: Fix comments

[Lasse Collin]

Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>
---
 lib/xz/xz_dec_lzma2.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
index 68ae9c33b6a8..46c2df6ad6f5 100644
--- a/lib/xz/xz_dec_lzma2.c
+++ b/lib/xz/xz_dec_lzma2.c
@@ -271,7 +271,7 @@ struct xz_dec_lzma2 {
 	struct lzma_dec lzma;
 
 	/*
-	 * Temporary buffer which holds small number of input bytes between
+	 * Temporary buffer which holds a small number of input bytes between
 	 * decoder calls. See lzma2_lzma() for details.
 	 */
 	struct {
@@ -757,8 +757,8 @@ static bool lzma_main(struct xz_dec_lzma2 *s)
 	uint32_t pos_state;
 
 	/*
-	 * If the dictionary was reached during the previous call, try to
-	 * finish the possibly pending repeat in the dictionary.
+	 * If the dictionary write limit was reached during the previous call,
+	 * try to finish the possibly pending repeat in the dictionary.
 	 */
 	if (dict_has_space(&s->dict) && s->lzma.len > 0)
 		dict_repeat(&s->dict, &s->lzma.len, s->lzma.rep0);
@@ -978,7 +978,7 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
 			 *          an uncompressed chunk
 			 *   0x02   Uncompressed chunk (no dictionary reset)
 			 *
-			 * Highest three bits (s->control & 0xE0):
+			 * Highest three bits (tmp & 0xE0):
 			 *   0xE0   Dictionary reset, new properties and state
 			 *          reset, followed by LZMA compressed chunk
 			 *   0xC0   New properties and state reset, followed
@@ -990,7 +990,7 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
 			 *   0x80   LZMA chunk (no dictionary or state reset)
 			 *
 			 * For LZMA compressed chunks, the lowest five bits
-			 * (s->control & 1F) are the highest bits of the
+			 * (tmp & 1F) are the highest bits of the
 			 * uncompressed size (bits 16-20).
 			 *
 			 * A new LZMA2 stream must begin with a dictionary
@@ -1091,7 +1091,7 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
 		case SEQ_LZMA_RUN:
 			/*
 			 * Set dictionary limit to indicate how much we want
-			 * to be encoded at maximum. Decode new data into the
+			 * to be decoded at maximum. Decode new data into the
 			 * dictionary. Flush the new data from dictionary to
 			 * b->out. Check if we finished decoding this chunk.
 			 * In case the dictionary got full but we didn't fill
-- 
2.54.0

Re: [PATCH 1/2] lib/xz: Use size_t instead of uint32_t in a few places

[David Laight]

On Sun, 14 Jun 2026 19:05:17 +0300
Lasse Collin <lasse.collin@tukaani.org> wrote:

> Reduce the number of uint32_t <-> size_t conversions a little. Eliminating
> such conversions entirely would require changing almost all uint32_t to
> size_t, which would look confusing and increase the sizes of the structs
> even more. Going the other way, converting everything to uint32_t, isn't
> possible because the input and output buffers use size_t in struct xz_buf.
> 
> Now both arguments to min() have the same type. This is required to for
> compatibility with PowerPC boot code[1] whose min() is strict like
> include/linux/minmax.h was before the commit d03eba99f5bf ("minmax: allow
> min()/max()/clamp() if the arguments have the same signedness.").
> 
> Swap the order of the "state" and "len" in struct lzma_dec to avoid
> padding in the middle of the struct when size_t is 64 bits. The
> reordering doesn't change the size of the struct; the padding just
> appears at the end instead.
> 
> dict_flush() used to truncate size_t to uint32_t when returning.
> This wasn't a bug; the value is always small enough.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/lkml/20260610232323.GA1071374@ax162/ [1]
> Cc: Thorsten Blum <thorsten.blum@linux.dev>
> Cc: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>
> ---
>  lib/xz/xz_dec_lzma2.c | 40 ++++++++++++++++++++--------------------
>  lib/xz/xz_lzma2.h     |  2 +-
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
> index 9d80342b9c6b..68ae9c33b6a8 100644
> --- a/lib/xz/xz_dec_lzma2.c
> +++ b/lib/xz/xz_dec_lzma2.c
> @@ -135,14 +135,16 @@ struct lzma_dec {
>  	uint32_t rep2;
>  	uint32_t rep3;
>  
> -	/* Types of the most recently seen LZMA symbols */
> -	enum lzma_state state;
> -
>  	/*
>  	 * Length of a match. This is updated so that dict_repeat can
> -	 * be called again to finish repeating the whole match.
> +	 * be called again to finish repeating the whole match. This is
> +	 * size_t because a pointer to this is passed to dict_repeat,
> +	 * and there it's nicer to have size_t instead of uint32_t.
>  	 */
> -	uint32_t len;
> +	size_t len;
> +
> +	/* Types of the most recently seen LZMA symbols */
> +	enum lzma_state state;
>  
>  	/*
>  	 * LZMA properties or related bit masks (number of literal
> @@ -228,13 +230,13 @@ struct lzma2_dec {
>  	enum lzma2_seq next_sequence;
>  
>  	/* Uncompressed size of LZMA chunk (2 MiB at maximum) */
> -	uint32_t uncompressed;
> +	size_t uncompressed;
>  
>  	/*
>  	 * Compressed size of LZMA chunk or compressed/uncompressed
>  	 * size of uncompressed chunk (64 KiB at maximum)
>  	 */
> -	uint32_t compressed;
> +	size_t compressed;
>  
>  	/*
>  	 * True if dictionary reset is needed. This is false before
> @@ -273,7 +275,7 @@ struct xz_dec_lzma2 {
>  	 * decoder calls. See lzma2_lzma() for details.
>  	 */
>  	struct {
> -		uint32_t size;
> +		size_t size;
>  		uint8_t buf[3 * LZMA_IN_REQUIRED];
>  	} temp;
>  };
> @@ -320,7 +322,7 @@ static inline bool dict_has_space(const struct dictionary *dict)
>   * still empty. This special case is needed for single-call decoding to
>   * avoid writing a '\0' to the end of the destination buffer.
>   */
> -static inline uint32_t dict_get(const struct dictionary *dict, uint32_t dist)
> +static inline uint32_t dict_get(const struct dictionary *dict, size_t dist)
>  {
>  	size_t offset = dict->pos - dist - 1;
>  
> @@ -346,10 +348,10 @@ static inline void dict_put(struct dictionary *dict, uint8_t byte)
>   * invalid, false is returned. On success, true is returned and *len is
>   * updated to indicate how many bytes were left to be repeated.
>   */
> -static bool dict_repeat(struct dictionary *dict, uint32_t *len, uint32_t dist)
> +static bool dict_repeat(struct dictionary *dict, size_t *len, size_t dist)
>  {
>  	size_t back;
> -	uint32_t left;
> +	size_t left;
>  
>  	if (dist >= dict->full || dist >= dict->size)
>  		return false;
> @@ -375,7 +377,7 @@ static bool dict_repeat(struct dictionary *dict, uint32_t *len, uint32_t dist)
>  
>  /* Copy uncompressed data as is from input to dictionary and output buffers. */
>  static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b,
> -			      uint32_t *left)
> +			      size_t *left)
>  {
>  	size_t copy_size;
>  
> @@ -433,7 +435,7 @@ static void dict_uncompressed(struct dictionary *dict, struct xz_buf *b,
>   * enough space in b->out. This is guaranteed because caller uses dict_limit()
>   * before decoding data into the dictionary.
>   */
> -static uint32_t dict_flush(struct dictionary *dict, struct xz_buf *b)
> +static size_t dict_flush(struct dictionary *dict, struct xz_buf *b)
>  {
>  	size_t copy_size = dict->pos - dict->start;
>  
> @@ -878,7 +880,7 @@ static bool lzma_props(struct xz_dec_lzma2 *s, uint8_t props)
>  static bool lzma2_lzma(struct xz_dec_lzma2 *s, struct xz_buf *b)
>  {
>  	size_t in_avail;
> -	uint32_t tmp;
> +	size_t tmp;
>  
>  	in_avail = b->in_size - b->in_pos;
>  	if (s->temp.size > 0 || s->lzma2.compressed == 0) {
> @@ -1046,25 +1048,23 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
>  
>  		case SEQ_UNCOMPRESSED_1:
>  			s->lzma2.uncompressed
> -					+= (uint32_t)b->in[b->in_pos++] << 8;
> +					+= (size_t)b->in[b->in_pos++] << 8;

The type of that casts can't matter.
Indeed all these casts can be deleted.
(The value read from the uint8_t variable is promoted to int before
it is used.)

	David

>  			s->lzma2.sequence = SEQ_UNCOMPRESSED_2;
>  			break;
>  
>  		case SEQ_UNCOMPRESSED_2:
>  			s->lzma2.uncompressed
> -					+= (uint32_t)b->in[b->in_pos++] + 1;
> +					+= (size_t)b->in[b->in_pos++] + 1;
>  			s->lzma2.sequence = SEQ_COMPRESSED_0;
>  			break;
>  
>  		case SEQ_COMPRESSED_0:
> -			s->lzma2.compressed
> -					= (uint32_t)b->in[b->in_pos++] << 8;
> +			s->lzma2.compressed = (size_t)b->in[b->in_pos++] << 8;
>  			s->lzma2.sequence = SEQ_COMPRESSED_1;
>  			break;
>  
>  		case SEQ_COMPRESSED_1:
> -			s->lzma2.compressed
> -					+= (uint32_t)b->in[b->in_pos++] + 1;
> +			s->lzma2.compressed += (size_t)b->in[b->in_pos++] + 1;
>  			s->lzma2.sequence = s->lzma2.next_sequence;
>  			break;
>  
> diff --git a/lib/xz/xz_lzma2.h b/lib/xz/xz_lzma2.h
> index d2632b7dfb9c..a612ce4fd450 100644
> --- a/lib/xz/xz_lzma2.h
> +++ b/lib/xz/xz_lzma2.h
> @@ -143,7 +143,7 @@ static inline bool lzma_state_is_literal(enum lzma_state state)
>   * Get the index of the appropriate probability array for decoding
>   * the distance slot.
>   */
> -static inline uint32_t lzma_get_dist_state(uint32_t len)
> +static inline size_t lzma_get_dist_state(size_t len)
>  {
>  	return len < DIST_STATES + MATCH_LEN_MIN
>  			? len - MATCH_LEN_MIN : DIST_STATES - 1;

Re: [PATCH 1/2] lib/xz: Use size_t instead of uint32_t in a few places

[Thorsten Blum]

On Sun, Jun 14, 2026 at 07:05:17PM +0300, Lasse Collin wrote:
> Reduce the number of uint32_t <-> size_t conversions a little. Eliminating
> such conversions entirely would require changing almost all uint32_t to
> size_t, which would look confusing and increase the sizes of the structs
> even more. Going the other way, converting everything to uint32_t, isn't
> possible because the input and output buffers use size_t in struct xz_buf.
> 
> Now both arguments to min() have the same type. This is required to for
> compatibility with PowerPC boot code[1] whose min() is strict like
> include/linux/minmax.h was before the commit d03eba99f5bf ("minmax: allow
> min()/max()/clamp() if the arguments have the same signedness.").
> 
> Swap the order of the "state" and "len" in struct lzma_dec to avoid
> padding in the middle of the struct when size_t is 64 bits. The
> reordering doesn't change the size of the struct; the padding just
> appears at the end instead.
> 
> dict_flush() used to truncate size_t to uint32_t when returning.
> This wasn't a bug; the value is always small enough.
> 
> Reported-by: Nathan Chancellor <nathan@kernel.org>
> Closes: https://lore.kernel.org/lkml/20260610232323.GA1071374@ax162/ [1]
> Cc: Thorsten Blum <thorsten.blum@linux.dev>
> Cc: David Laight <david.laight.linux@gmail.com>
> Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>

LGTM, thanks!

Reviewed-by: Thorsten Blum <thorsten.blum@linux.dev>

Re: [PATCH 1/2] lib/xz: Use size_t instead of uint32_t in a few places

[Lasse Collin]

On 2026-06-15 David Laight wrote:
> On Sun, 14 Jun 2026 19:05:17 +0300
> Lasse Collin <lasse.collin@tukaani.org> wrote:
> > @@ -1046,25 +1048,23 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
> >  
> >  		case SEQ_UNCOMPRESSED_1:
> >  			s->lzma2.uncompressed
> > -					+= (uint32_t)b->in[b->in_pos++] << 8;
> > +					+= (size_t)b->in[b->in_pos++] << 8;
> 
> The type of that casts can't matter.
> Indeed all these casts can be deleted.
> (The value read from the uint8_t variable is promoted to int before
> it is used.)

It is true that the casts aren't mandatory here. There are a few more
similar casts in lib/xz.

I dislike that narrow unsigned types are promoted to signed int in C; I
would prefer unsigned staying unsigned. If the result doesn't fit into
the original unsigned type, it feels clearest to cast to the unsigned
target type, skipping the intermediate signed type and the need to
verify that the signed type indeed works correctly in the particular
expression. This results in a few seemingly useless casts, but it's
less mental load to think about two types instead of three.

    uint8_t b = 0x80;
    uint32_t x = b << 8;  // (1)
    uint32_t y = b << 24; // (2)
    size_t   z = b << 24; // (3)

(1) is fine, but I tend to add a cast anyway.

(2) is undefined behavior due to shifting into the sign bit of a signed
int, except with GCC it's well defined[*] but -fsanitize=undefined will
still complain.

(3) Even if the shift into the sign bit was OK, the conversion of a
negative signed int to size_t is unlikely to produce the intended
result.

Maybe many people find the extra casts weird, but at least you know my
rationale behind them now.

[*] https://gcc.gnu.org/onlinedocs/gcc-16.1.0/gcc/Integers-implementation.html

-- 
Lasse Collin