linux-6.3-rc6/lib/maple_tree.c: Two style issues

linux-6.3-rc6/lib/maple_tree.c: Two style issues

[David Binderman]

Hello there,

Static analyser cppcheck says:

1.

linux-6.3-rc6/lib/maple_tree.c:1951:21: style: Array index 'split' is used before limits check. [arrayIndexThenCheck]

Source code is 

        while (((bn->pivot[split] - min) < slot_count - 1) &&
               (split < slot_count - 1) && (b_end - split > slot_min))

Suggest move limits check to before use.

2.

linux-6.3-rc6/lib/maple_tree.c:3289:11: warning: Size of pointer 'pivs' used instead of size of its data. [pointerSize]

Source code is

           memset(pivs + tmp, 0,
                   sizeof(unsigned long *) * (max_p - tmp));

but

    unsigned long *l_pivs, *pivs, gap;

Pointers and long don't have to be the same size. Suggest code rework.

Regards

David Binderman

Re: linux-6.3-rc6/lib/maple_tree.c: Two style issues

[Peng Zhang]

在 2023/4/10 15:05, David Binderman 写道:
> Hello there,
>
> Static analyser cppcheck says:
>
> 1.
>
> linux-6.3-rc6/lib/maple_tree.c:1951:21: style: Array index 'split' is used before limits check. [arrayIndexThenCheck]
>
> Source code is
>
>          while (((bn->pivot[split] - min) < slot_count - 1) &&
>                 (split < slot_count - 1) && (b_end - split > slot_min))
>
> Suggest move limits check to before use.

Hi,

It should be fine here. The upper bound of split is b_end.
The initial state (split = b_end / 2) must not cross the boundary,
and (b_end - split > slot_min) ensures that it will not cross the 
boundary in the future.

>
> 2.
>
> linux-6.3-rc6/lib/maple_tree.c:3289:11: warning: Size of pointer 'pivs' used instead of size of its data. [pointerSize]
>
> Source code is
>
>             memset(pivs + tmp, 0,
>                     sizeof(unsigned long *) * (max_p - tmp));

It's not good here, I can fix it.

Thanks.

>
> but
>
>      unsigned long *l_pivs, *pivs, gap;
>
> Pointers and long don't have to be the same size. Suggest code rework.
>
> Regards
>
> David Binderman
>

Re: linux-6.3-rc6/lib/maple_tree.c: Two style issues

[David Binderman]

Hello there,

>It should be fine here. 

Perhaps I have been slightly less than clear. The discussion is about the *style*
of the code, *not* whether it works or not. My apologies for the lack of clarity. 

Expression is

    while (A &&
        B && C)

The static analyser notices it is poor style to have B do a limit check, but have A use it.
Sure its working code, but suggest new code

    while (B && A && C)

It won't make much difference to the code, it will merely be better style.

2.

>> Source code is
>>
>>             memset(pivs + tmp, 0,
>>                     sizeof(unsigned long *) * (max_p - tmp));

>It's not good here, I can fix it.

sizeof( pivs[ 0]) is a better thing to say than sizeof( unsigned long). 
There is reduced future maintenance burden, when the type of *pivs changes.

Regards

David Binderman

Re: linux-6.3-rc6/lib/maple_tree.c: Two style issues

[Liam R. Howlett]

* David Binderman <dcb314@hotmail.com> [230417 10:23]:
> Hello there,
> 
> >It should be fine here. 
> 
> Perhaps I have been slightly less than clear. The discussion is about the *style*
> of the code, *not* whether it works or not. My apologies for the lack of clarity. 
> 
> Expression is
> 
>     while (A &&
>         B && C)
> 
> The static analyser notices it is poor style to have B do a limit check, but have A use it.
> Sure its working code, but suggest new code
> 
>     while (B && A && C)
> 
> It won't make much difference to the code, it will merely be better style.
> 
> 2.
> 
> >> Source code is
> >>
> >>             memset(pivs + tmp, 0,
> >>                     sizeof(unsigned long *) * (max_p - tmp));
> 
> >It's not good here, I can fix it.
> 
> sizeof( pivs[ 0]) is a better thing to say than sizeof( unsigned long). 
> There is reduced future maintenance burden, when the type of *pivs changes.
> 

If you want to make the code better, then send a patch.  It is very
frustrating to try and decode a compilers output over email and be told
it wasn't decoded to your liking.

This is far more of a maintenance burden than the code you are trying to change.