Re: Rust kernel policy

[Dan Carpenter <dan.carpenter@linaro.org>]

On Fri, Feb 21, 2025 at 07:31:11PM +0100, Martin Uecker wrote:
> > This is non-negotiable. Anybody who thinks that a compiler is valid
> > warning about
> > 
> >          if (x < 0 || x >= 10) {
> > 
> > just because 'x' may in some cases be an unsigned entity is not worth
> > even discussing with.
> 
> Do you think the warning is useless in macros, or in general?

This is a fair question.  In smatch I often will turn off a static
checker warnings if they're inside a macro.  For example, macros will
have NULL checks which aren't required in every place where they're
used so it doesn't make sense to warn about inconsistent NULL checking
if the NULL check is done in a macro.

In this unsigned less than zero example, we can easily see that it works
to clamp 0-9 and the compiler could silence the warning based on that.
I mentioned that Justin filtered out idiomatic integer overflows like
if (a < a + b) { and we could do the same here.  That would silence most
of the false positives.  It's a culture debate not a technical problem.

Silencing the checks inside macros would silence quite a few of the
remaining false positives.  In Smatch, I've silenced a few false
positives that way for specific macros but I haven't felt the need to
go all the way and turning the check off inside all macros.

There are also a handful of defines which can be zero depending on the
circumstances like DPMCP_MIN_VER_MINOR:

	if (dpmcp_dev->obj_desc.ver_minor < DPMCP_MIN_VER_MINOR)
		return -ENOTSUPP;

Or another example is in set_iter_tags()

        /* This never happens if RADIX_TREE_TAG_LONGS == 1 */
        if (tag_long < RADIX_TREE_TAG_LONGS - 1) {

The other thing is that in Smatch, I don't try to silence every false
positives.  Or any false positives.  :P  So long as I can handle the work
load of reviewing new warnings it's fine. I look at a warning once and
then I'm done.

regards,
dan carpenter