On Tue, Feb 19, 2019 at 09:14:58PM +0300, Dan Carpenter wrote: > 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. I prefer to see this proposed patch applied, it is easier for me to see less warnings. > > You're right that it would be really hard to make the static checker > understand this code... > > regards, > dan carpenter
Attachment:
signature.asc
Description: PGP signature
