On Thu, Dec 08, 2016 at 11:01:30AM -0800, James Bottomley wrote: > On Tue, 2016-08-02 at 01:17 -0400, Martin K. Petersen wrote: > > > > > > > "Johannes" == Johannes Thumshirn <jthumshirn@xxxxxxx> > > > > > > > writes: > > > > Johannes> Check for the existence of piocb->vport before accessing > > it. > > > > Applied to 4.8/scsi-queue. > > OK, now that this has caused problems, could learn the lessons from it? > > Lines like this: > > + BUG_ON(!piocb || !piocb->vport); > > Should never appear in code. They only have the potential to cause > problems if the condition is inexact and they provide precisely no > information over what a NULL deref in the kernel is going to tell us > anyway ... this one even obscures information because you don't know if > pciob was null or pciob->vport when it triggers. > > The rule is never BUG_ON a NULL pointer unless you have an extremely > good reason why the kernel NULL deref handler isn't adequate (which > should be documented in the commit log). Yup I fully agree, but shouldn't we take as a 2nd lesson that BUG_ON()s generally aren't an extremely good idea? I personally think a lot of BUG_ON()s in driver code can be eliminated with proper error handling. That said, mea culpa. Byte, Johannes -- Johannes Thumshirn Storage jthumshirn@xxxxxxx +49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
