On Wed, Apr 05, 2017 at 11:59:31AM +0300, Dan Carpenter wrote: > On Tue, Apr 04, 2017 at 09:59:30PM +1000, Tobin C. Harding wrote: > > On Tue, Apr 04, 2017 at 01:21:33PM +0300, Dan Carpenter wrote: > > > On Sun, Apr 02, 2017 at 10:18:09AM +1000, Tobin C. Harding wrote: > > > > Constant is used to allocate memory for a buffer, then buffer is > > > > filled upto 'size' which is passed as a parameter. If 'size' is bigger > > > > than the constant then the buffer will overflow. Function has internal > > > > linkage so this can only happen due to programmer error. BUG_ON() is > > > > designed for catching these cases. Currently there is only one call > > > > site and it is correct, adding BUG_ON() will potentially save > > > > developer time if later changes to the code are incorrect. > > > > > > > > Use BUG_ON() to guard buffer write size in function with internal linkage. > > > > > > Nah. Adding these BUG_ON() calls everywhere is ugly. There are tons > > > and tons of assumptions like this in the kernel and we'd have to add > > > thousands of BUG_ON()s to cover everything. > > > > So do you mean we should we not make the assumptions, or that we should > > check input arguments and return an error if assumptions are violated? > > I read that BUG_ON() is not liked in many cases, is there any cases > > where BUG_ON() is good to use? I tried to read up on it and from what > > I could gather this was the use case. Did I miss-read? > > > > The most important valid excuse for using BUG_ON() is for place > where continuing in an invalid probably means we're going to corrupt the > file system. It's easier to talk about bad uses of BUG_ON(). > > BUG_ON(p == NULL); > > This is wrong for several reasons, because first you should just audit > all the code so that p is never NULL. Second if p is NULL and we > dereference it then the kernel will Oops and generate a stack trace so > the BUG_ON() doesn't add any value. Also when the kernel Oopses you > still might be able to type, save a dmesg and debug the problem. If > it does a hard stop then you have to take a photo on your phone and > type in the data again which is obviously a pain. Finally, sprinkling > extra BUG_ON() droppings here and there makes the code messier and hurts > readability. > > Say it's a core kernel function and you can't audit all the callers > because many of them will be written in the future, then probably it's > better to just add proper error handling and perhaps a WARN_ON() for > debugging. > > Device drivers normally can't corrupt the filesystem so probably they > shouldn't be adding a bunch of BUG_ON() calls. > > regards, > dan carpenter Got it. thanks, Tobin. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
