On Tue, Feb 19, 2019 at 07:20:34PM +0200, Leon Romanovsky wrote: > On Tue, Feb 19, 2019 at 05:47:41PM +0200, Moni Shoua wrote: > > > This kind of change makes the code confusing to human readers. Have you > > > considered to add a BUG_ON(!qp) or WARN_ON(!qp) with a comment that > > > refers to sparse instead? > > > > > > Thanks, > > > > > > Bart. > > > > > > > > > This kind of change makes the code confusing to human readers. Have you > > > considered to add a BUG_ON(!qp) or WARN_ON(!qp) with a comment that > > > refers to sparse instead? > > > > > > Thanks, > > > > > I'm not sure that this kind of fix will satisfy the static checker. It > > depends how it understand BUG_ON(). > > I didn't ever think to use BUG_ON(), because we are trying to get rid of them. > Regarding WARN_ON(), I have the same comment as Moni, it is unclear how > static analysis tools will handle WARN_ON(). > Smatch understands BUG_ON(). It ignores WARN_ON(). WARN_ON() is used in various ways by different developers and it's not useful for static analysis. There is not really any point to a WARN_ON() before a NULL dereference because the Oops information already has the stack trace. The other option is to just leave the code as-is. I don't mind if we do that. You're right that it would be really hard to make the static checker understand this code... regards, dan carpenter
