Re: [PATCH] mm: hugetlb: checking for IS_ERR() instead of NULL

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

On Tue, Jun 01, 2021 at 11:50:06PM +0300, Dan Carpenter wrote:
> On Tue, Jun 01, 2021 at 10:51:23PM +0300, Dan Carpenter wrote:
> > The other thing which might be interesting is if you pass a NULL
> > to IS_ERR() and then dereference the NULL then print a warning about
> > that.  This has a lot of overlaps with some of my existing checks, but
> > it's still a new idea so it belongs in a separate check.  It's fine and
> > good even if one bug triggers a lot of different warnings.  I'll write
> > that, hang on, brb.
> 
> 100% untested.  :)  I'll test it tonight.
> 

This test is decent, but I ended up making a few changes:

1)  My devel version of Smatch had a new bug in it which caused some
    false positives.  Fixed now, hopefully.

2)  The test:

	if (get_state_expr(my_id, expr) != &null)
		return;

    check was not strict enough.  I realized that I knew that from
    square one but I was lazy.  So now I have introduced a global helper
    function and updated the code:

bool expr_has_possible_state(int owner, struct expression *expr, struct smatch_state *state)
{
        struct sm_state *sm;

        sm = get_sm_state_expr(owner, expr);
        if (!sm)
                return false;

        return slist_has_state(sm->possible, state);
}

    I replaced the test with:

	if (!expr_has_possible_state(my_id, expr, &null))

3)  The warning message was too vague and too similar to other warning
    messages.  It should be something unique to the test.  It's now:

	sm_error("potential NULL/IS_ERR bug '%s'", name);

I'll post the results tomorrow.

regards,
dan carpenter