On Wed, Jun 02, 2021 at 05:47:52PM +0300, Dan Carpenter wrote:
> 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.
Many thanks! Gonna check today.
>
> 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.
This is great, will be a nice bug to check for beginners like me.
>
> regards,
> dan carpenter
>
