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
在 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
>
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
* David Binderman <[email protected]> [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.