Re: [PATCH] lib/lzo/lzo1x_compress.c: replace ternary operator with min() and min_t()

[Dave Rodgman]

> From: Jonathan Corbet <corbet@xxxxxxx>
> Sent: 14 July 2022 14:39
> To: Jiangshan Yi
> Cc: linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; Jiangshan Yi; Andrew Morton
> Subject: Re: [PATCH] lib/lzo/lzo1x_compress.c: replace ternary operator with min() and min_t()
>
> Jiangshan Yi <13667453960@xxxxxxx> writes:
>
> > From: Jiangshan Yi <yijiangshan@xxxxxxxxxx>
> >
> > Fix the following coccicheck warning:
> >
> > lib/lzo/lzo1x_compress.c:54: WARNING opportunity for min().
> > lib/lzo/lzo1x_compress.c:329: WARNING opportunity for min().
> >
> > min() and min_t() macro is defined in include/linux/minmax.h. It avoids
> > multiple evaluations of the arguments when non-constant and performs
> > strict type-checking.
> >
> > Signed-off-by: Jiangshan Yi <yijiangshan@xxxxxxxxxx>
> > ---
> >  lib/lzo/lzo1x_compress.c | 6 ++----
> >  1 file changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/lzo/lzo1x_compress.c b/lib/lzo/lzo1x_compress.c
> > index 76758e9296ba..9d31e7126606 100644
> > --- a/lib/lzo/lzo1x_compress.c
> > +++ b/lib/lzo/lzo1x_compress.c
> > @@ -50,9 +50,7 @@ lzo1x_1_do_compress(const unsigned char *in, size_t in_len,
> >
> >               if (dv == 0 && bitstream_version) {
> >                       const unsigned char *ir = ip + 4;
> > -                     const unsigned char *limit = ip_end
> > -                             < (ip + MAX_ZERO_RUN_LENGTH + 1)
> > -                             ? ip_end : ip + MAX_ZERO_RUN_LENGTH + 1;
> > +                     const unsigned char *limit = min(ip_end, ip + MAX_ZERO_RUN_LENGTH + 1);
> >  #if defined(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) && >       defined(LZO_FAST_64BIT_MEMORY_ACCESS)
> >                       u64 dv64;
> > @@ -326,7 +324,7 @@ static int lzogeneric1x_1_compress(const unsigned char *in, size_t in_len,
> >       data_start = op;
> >
> >       while (l > 20) {
> > -             size_t ll = l <= (m4_max_offset + 1) ? l : (m4_max_offset + 1);
> > +             size_t ll = min_t(size_t, l, m4_max_offset + 1);
> >               uintptr_t ll_end = (uintptr_t) ip + ll;
> >               if ((ll_end + ((t + ll) >> 5)) <= ll_end)
> >                       break;
>
> So these look like good changes to me, but I am not the maintainer of
> this code.  Maybe Andrew (copied) would have a spot for this patch?
>
> Thanks,
>
> jon

This looks good to me. I've given it a quick test and it appears to function properly.

Dave
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.