* Re: [PATCH] lib/xz: replace min_t with min
2026-06-10 23:48 ` Andrew Morton
@ 2026-06-11 9:54 ` David Laight
2026-06-12 19:41 ` Lasse Collin
1 sibling, 0 replies; 6+ messages in thread
From: David Laight @ 2026-06-11 9:54 UTC (permalink / raw)
To: Andrew Morton
Cc: Nathan Chancellor, Lasse Collin, Thorsten Blum, linux-kernel
On Wed, 10 Jun 2026 16:48:00 -0700
Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 10 Jun 2026 16:23:23 -0700 Nathan Chancellor <nathan@kernel.org> wrote:
>
> > On Tue, Jun 09, 2026 at 06:00:28PM +0300, Lasse Collin wrote:
> > > From: Thorsten Blum <thorsten.blum@linux.dev>
> > >
> > > Use the simpler min() macro since the values are unsigned and
> > > compatible.
> > >
> > > Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
> > > Reviewed-by: Lasse Collin <lasse.collin@tukaani.org>
> > > Signed-off-by: Lasse Collin <lasse.collin@tukaani.org>
> > ...
> > > diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
> > > index 4b783ac94e71..9d80342b9c6b 100644
> > > --- a/lib/xz/xz_dec_lzma2.c
> > > +++ b/lib/xz/xz_dec_lzma2.c
> > > @@ -354,7 +354,7 @@ static bool dict_repeat(struct dictionary *dict, uint32_t *len, uint32_t dist)
> > > if (dist >= dict->full || dist >= dict->size)
> > > return false;
> > >
> > > - left = min_t(size_t, dict->limit - dict->pos, *len);
> > > + left = min(dict->limit - dict->pos, *len);
> > > *len -= left;
> > >
> > > back = dict->pos - dist - 1;
> > > @@ -1098,9 +1098,8 @@ enum xz_ret xz_dec_lzma2_run(struct xz_dec_lzma2 *s, struct xz_buf *b)
> > > * the output buffer yet, we may run this loop
> > > * multiple times without changing s->lzma2.sequence.
> > > */
> > > - dict_limit(&s->dict, min_t(size_t,
> > > - b->out_size - b->out_pos,
> > > - s->lzma2.uncompressed));
> > > + dict_limit(&s->dict, min(b->out_size - b->out_pos,
> > > + s->lzma2.uncompressed));
> > > if (!lzma2_lzma(s, b))
> > > return XZ_DATA_ERROR;
> > >
> >
> > These two hunks from this change in -next as 1003161e12ac ("lib/xz:
> > replace min_t with min") cause warnings in the arch/powerpc/boot code,
> > as it uses an old, simple version of min() and max() that does not have
> > the improvements done in d03eba99f5bf ("minmax: allow
> > min()/max()/clamp() if the arguments have the same signedness."):
>
> Well that's annoying.
>
> A pleasing solution would be to make the xz code be more consistent in its
> type usage. struct lzma_dec has a liking for uintXX_t whereas struct
> dictionary likes size_t. Perhaps a fundamental reexamination of what
> these fields are representing is in order?
>
>
Or change the ppc boot definitions to simple ones like:
#define min(a, b) ({ \
auto _a = a; \
auto _b = b; \
_a < _b ? _a : _b; \
})
That remove the type check (fairly pointless!).
(There isn't even a commit message that says someone got bitten
by a negative value becoming a very large one.)
-- David
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] lib/xz: replace min_t with min
2026-06-10 23:48 ` Andrew Morton
2026-06-11 9:54 ` David Laight
@ 2026-06-12 19:41 ` Lasse Collin
1 sibling, 0 replies; 6+ messages in thread
From: Lasse Collin @ 2026-06-12 19:41 UTC (permalink / raw)
To: Andrew Morton
Cc: Nathan Chancellor, Thorsten Blum, linux-kernel, David Laight
[-- Attachment #1: Type: text/plain, Size: 2086 bytes --]
On 2026-06-10 Andrew Morton wrote:
> On Wed, 10 Jun 2026 16:23:23 -0700 Nathan Chancellor
> <nathan@kernel.org> wrote:
> > These two hunks from this change in -next as 1003161e12ac ("lib/xz:
> > replace min_t with min") cause warnings in the arch/powerpc/boot
> > code, as it uses an old, simple version of min() and max() that
> > does not have the improvements done in d03eba99f5bf ("minmax: allow
> > min()/max()/clamp() if the arguments have the same signedness."):
>
> Well that's annoying.
>
> A pleasing solution would be to make the xz code be more consistent
> in its type usage. struct lzma_dec has a liking for uintXX_t whereas
> struct dictionary likes size_t. Perhaps a fundamental reexamination
> of what these fields are representing is in order?
I tried to rethink it.
Callers use struct xz_buf to pass input and output buffers. That struct
uses size_t. I assume it should stay that way because the existing
callers assign size_t values to struct xz_buf members.
The input buffer members from struct xz_buf are assigned to members of
struct rc_dec, which is why size_t is needed in struct rc_dec.
In single-call mode (XZ_SINGLE), the decoder writes directly to the
caller-specified output buffer. In this case the output buffer members
from struct xz_buf are assigned to struct dictionary, and thus multiple
members in struct dictionary have to be size_t. In multi-call mode
(XZ_PREALLOC or XZ_DYNALLOC) the dictionary buffer is private to the
decoder and then uint32_t would suffice in struct dictionary.
Otherwise xz_dec_lzma2.c would be fine without size_t.
A few uint32_t uses could be changed to size_t to avoid the min()
issue, and slightly cleanup uint32_t vs. size_t usage otherwise too.
The first attached draft patch does this.
Even all uint32_t uses could be changed to size_t like in the second
patch, but it looks confusing in situations when a value isn't a size
or offset. It also increases the sizes of structs more.
I don't like the second patch, but I will submit the first one in a day
or two if I still think it's OK.
--
Lasse Collin
[-- Attachment #2: 0001-DRAFT-lib-xz-Use-size_t-instead-of-uint32_t-in-a-few.patch --]
[-- Type: text/x-patch, Size: 5707 bytes --]
From 2826da02c269cd846d18741dfee083c4058defe7 Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.collin@tukaani.org>
Date: Fri, 12 Jun 2026 21:37:49 +0300
Subject: [PATCH 1/2] DRAFT *** lib/xz: Use size_t instead of uint32_t in a few
places
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 commit d03eba99f5bf ("minmax: allow
min()/max()/clamp() if the arguments have the same signedness.").
Since "uncompressed" was changed to size_t, change also "compressed"
and its uses for consistency.
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 size of
the struct doesn't change; the padding just appears at the end instead.
Link: https://lore.kernel.org/lkml/20260610232323.GA1071374@ax162/ [1]
---
lib/xz/xz_dec_lzma2.c | 36 +++++++++++++++++-------------------
lib/xz/xz_lzma2.h | 2 +-
2 files changed, 18 insertions(+), 20 deletions(-)
diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
index 9d80342b9c6b..087385e5d9a7 100644
--- a/lib/xz/xz_dec_lzma2.c
+++ b/lib/xz/xz_dec_lzma2.c
@@ -135,14 +135,14 @@ 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.
*/
- 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 +228,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 +273,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 +320,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 +346,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 +375,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 +433,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 +878,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 +1046,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
[-- Attachment #3: 0002-DRAFT-lib-xz-Convert-even-more-uses-of-uint32_t-to-s.patch --]
[-- Type: text/x-patch, Size: 5535 bytes --]
From b7234c4d5251cd751dfe87db5f23d33ba868806f Mon Sep 17 00:00:00 2001
From: Lasse Collin <lasse.collin@tukaani.org>
Date: Fri, 12 Jun 2026 22:09:41 +0300
Subject: [PATCH 2/2] DRAFT *** lib/xz: Convert even more uses of uint32_t to
size_t
---
lib/xz/xz_dec_lzma2.c | 62 +++++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/lib/xz/xz_dec_lzma2.c b/lib/xz/xz_dec_lzma2.c
index 087385e5d9a7..685c0efa4328 100644
--- a/lib/xz/xz_dec_lzma2.c
+++ b/lib/xz/xz_dec_lzma2.c
@@ -130,10 +130,10 @@ struct lzma_len_dec {
struct lzma_dec {
/* Distances of latest four matches */
- uint32_t rep0;
- uint32_t rep1;
- uint32_t rep2;
- uint32_t rep3;
+ size_t rep0;
+ size_t rep1;
+ size_t rep2;
+ size_t rep3;
/*
* Length of a match. This is updated so that dict_repeat can
@@ -150,9 +150,9 @@ struct lzma_dec {
* position bits, and a mask derived from the number
* position bits)
*/
- uint32_t lc;
- uint32_t literal_pos_mask; /* (1 << lp) - 1 */
- uint32_t pos_mask; /* (1 << pb) - 1 */
+ size_t lc;
+ size_t literal_pos_mask; /* (1 << lp) - 1 */
+ size_t pos_mask; /* (1 << pb) - 1 */
/* If 1, it's a match. Otherwise it's a single 8-bit literal. */
uint16_t is_match[STATES][POS_STATES_MAX];
@@ -320,7 +320,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, size_t dist)
+static inline size_t dict_get(const struct dictionary *dict, size_t dist)
{
size_t offset = dict->pos - dist - 1;
@@ -547,10 +547,10 @@ static __always_inline int rc_bit(struct rc_dec *rc, uint16_t *prob)
}
/* Decode a bittree starting from the most significant bit. */
-static __always_inline uint32_t rc_bittree(struct rc_dec *rc,
- uint16_t *probs, uint32_t limit)
+static __always_inline size_t rc_bittree(struct rc_dec *rc,
+ uint16_t *probs, size_t limit)
{
- uint32_t symbol = 1;
+ size_t symbol = 1;
do {
if (rc_bit(rc, &probs[symbol]))
@@ -565,10 +565,10 @@ static __always_inline uint32_t rc_bittree(struct rc_dec *rc,
/* Decode a bittree starting from the least significant bit. */
static __always_inline void rc_bittree_reverse(struct rc_dec *rc,
uint16_t *probs,
- uint32_t *dest, uint32_t limit)
+ size_t *dest, size_t limit)
{
- uint32_t symbol = 1;
- uint32_t i = 0;
+ size_t symbol = 1;
+ size_t i = 0;
do {
if (rc_bit(rc, &probs[symbol])) {
@@ -581,7 +581,7 @@ static __always_inline void rc_bittree_reverse(struct rc_dec *rc,
}
/* Decode direct bits (fixed fifty-fifty probability) */
-static inline void rc_direct(struct rc_dec *rc, uint32_t *dest, uint32_t limit)
+static inline void rc_direct(struct rc_dec *rc, size_t *dest, size_t limit)
{
uint32_t mask;
@@ -602,9 +602,9 @@ static inline void rc_direct(struct rc_dec *rc, uint32_t *dest, uint32_t limit)
/* Get pointer to literal coder probability array. */
static uint16_t *lzma_literal_probs(struct xz_dec_lzma2 *s)
{
- uint32_t prev_byte = dict_get(&s->dict, 0);
- uint32_t low = prev_byte >> (8 - s->lzma.lc);
- uint32_t high = (s->dict.pos & s->lzma.literal_pos_mask) << s->lzma.lc;
+ size_t prev_byte = dict_get(&s->dict, 0);
+ size_t low = prev_byte >> (8 - s->lzma.lc);
+ size_t high = (s->dict.pos & s->lzma.literal_pos_mask) << s->lzma.lc;
return s->lzma.literal[low + high];
}
@@ -612,11 +612,11 @@ static uint16_t *lzma_literal_probs(struct xz_dec_lzma2 *s)
static void lzma_literal(struct xz_dec_lzma2 *s)
{
uint16_t *probs;
- uint32_t symbol;
- uint32_t match_byte;
- uint32_t match_bit;
- uint32_t offset;
- uint32_t i;
+ size_t symbol;
+ size_t match_byte;
+ size_t match_bit;
+ size_t offset;
+ size_t i;
probs = lzma_literal_probs(s);
@@ -648,10 +648,10 @@ static void lzma_literal(struct xz_dec_lzma2 *s)
/* Decode the length of the match into s->lzma.len. */
static void lzma_len(struct xz_dec_lzma2 *s, struct lzma_len_dec *l,
- uint32_t pos_state)
+ size_t pos_state)
{
uint16_t *probs;
- uint32_t limit;
+ size_t limit;
if (!rc_bit(&s->rc, &l->choice)) {
probs = l->low[pos_state];
@@ -674,11 +674,11 @@ static void lzma_len(struct xz_dec_lzma2 *s, struct lzma_len_dec *l,
}
/* Decode a match. The distance will be stored in s->lzma.rep0. */
-static void lzma_match(struct xz_dec_lzma2 *s, uint32_t pos_state)
+static void lzma_match(struct xz_dec_lzma2 *s, size_t pos_state)
{
uint16_t *probs;
- uint32_t dist_slot;
- uint32_t limit;
+ size_t dist_slot;
+ size_t limit;
lzma_state_match(&s->lzma.state);
@@ -716,9 +716,9 @@ static void lzma_match(struct xz_dec_lzma2 *s, uint32_t pos_state)
* Decode a repeated match. The distance is one of the four most recently
* seen matches. The distance will be stored in s->lzma.rep0.
*/
-static void lzma_rep_match(struct xz_dec_lzma2 *s, uint32_t pos_state)
+static void lzma_rep_match(struct xz_dec_lzma2 *s, size_t pos_state)
{
- uint32_t tmp;
+ size_t tmp;
if (!rc_bit(&s->rc, &s->lzma.is_rep0[s->lzma.state])) {
if (!rc_bit(&s->rc, &s->lzma.is_rep0_long[
@@ -752,7 +752,7 @@ static void lzma_rep_match(struct xz_dec_lzma2 *s, uint32_t pos_state)
/* LZMA decoder core */
static bool lzma_main(struct xz_dec_lzma2 *s)
{
- uint32_t pos_state;
+ size_t pos_state;
/*
* If the dictionary was reached during the previous call, try to
--
2.54.0
^ permalink raw reply [flat|nested] 6+ messages in thread