Re: [PATCH] lib/lz4: smatch warning in LZ4_decompress_generic()

[Vasily Averin]

Dear Yann,
could you please consult us about your lz4 pacth
https://github.com/lz4/lz4/commit/1a191b3f8d26b50a7c1d41590b529ec308d768cd

Please see details below.

Thank you,
	Vasily Averin

On 6/6/20 5:36 PM, Gao Xiang wrote:
> On Sat, Jun 06, 2020 at 04:28:02PM +0300, Vasily Averin wrote:
>> Found by smatch:
>> lib/lz4/lz4_decompress.c:150 LZ4_decompress_generic() warn: maybe use && instead of &
>> It was realy incorrectly copied from
>> https://github.com/lz4/lz4/commit/45f8603aae389d34c689d3ff7427b314071ccd2c
>> line 1431
> 
> Simply no.
> 
>>
>> Fixes: 2209fda323e2 ("lib/lz4: update LZ4 decompressor module")
>> Signed-off-by: Vasily Averin <vvs@xxxxxxxxxxxxx>
>> ---
>>  lib/lz4/lz4_decompress.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>commit 1a191b3f8d26b50a7c1d41590b529ec308d768cd
Author: Yann Collet <cyan@xxxxxx>
Date:   Wed May 2 10:33:12 2018 -0700

    simplify shortcut

diff --git a/lib/lz4.c b/lib/lz4.c
index c6f0426..b46910f 100644
--- a/lib/lz4.c
+++ b/lib/lz4.c
@@ -1429,62 +1429,29 @@ LZ4_FORCE_INLINE int LZ4_decompress_generic(
          */
         if ((endOnInput ? length != RUN_MASK : length <= 8) &&
             /* strictly "less than" on input, to re-enter the loop with at least one byte */
-            likely((endOnInput ? ip < shortiend : 1) && (op <= shortoend)))
+            likely((endOnInput ? ip < shortiend : 1) & (op <= shortoend)))
>
>> diff --git a/lib/lz4/lz4_decompress.c b/lib/lz4/lz4_decompress.c
>> index 0c9d3ad..f7f7dca 100644
>> --- a/lib/lz4/lz4_decompress.c
>> +++ b/lib/lz4/lz4_decompress.c
>> @@ -147,7 +147,7 @@ static FORCE_INLINE int LZ4_decompress_generic(
>>  		    * strictly "less than" on input, to re-enter
>>  		    * the loop with at least one byte
>>  		    */
>> -		   && likely((endOnInput ? ip < shortiend : 1) &
>> +		   && likely((endOnInput ? ip < shortiend : 1) &&
> 
> I'd like to say, this is not my mistake (even not an issue).
> If you notice the latest LZ4 upstream
> https://github.com/lz4/lz4/blob/dev/lib/lz4.c#L1865
> 
> Or some related change, the lz4 author Cyan did it on purpose.
> https://github.com/lz4/lz4/commit/1a191b3f8d26b50a7c1d41590b529ec308d768cd

For me it looks like typo in patch,
lets ask author of this commit.

> I think we could follow the latest LZ4 upstream in order to
> avoid further maintainence overhead. That's my own thought
> anyway.
> 
> Thanks,
> Gao Xiang
>