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 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
