On 30 May 2018 at 23:41, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > On Wed, May 30, 2018 at 08:13:27AM -0700, Matthew Wilcox wrote: >> On Wed, May 30, 2018 at 08:14:09PM +0800, Baolin Wang wrote: >> > On 30 May 2018 at 20:01, Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: >> > > On Wed, May 30, 2018 at 07:28:43PM +0800, Baolin Wang wrote: >> > >> It will be crash if we pass one NULL name when creating one dma pool, >> > >> so we should check the passing name when copy it to dma pool. >> > > >> > > NAK. Crashing is the appropriate thing to do. Fix the caller to not >> > > pass NULL. >> > > >> > > If you permit NULL to be passed then you're inviting crashes or just >> > > bad reporting later when pool->name is printed. >> > >> > I think it just prints one NULL pool name. Sometimes the device >> > doesn't care the dma pool names, so I think we can make code more >> > solid to valid the passing parameters like other code does. >> > Or can we add check to return NULL when the passing name is NULL >> > instead of crashing the kernel? Thanks. >> >> No. Fix your driver. > > Let me elaborate on this. Kernel code is supposed to be "reasonable". > That means we don't check every argument to every function for sanity, > unless it's going to cause trouble later. Crashing immediately with > a bogus argument is fine; you can see the problem and fix it immediately. > Returning NULL with a bad argument is actually worse; you won't know why > the function returned NULL (maybe we're out of memory?) and you'll have > a more complex debugging experience. > > Sometimes it makes sense to accept a NULL pointer and do something > reasonable, like kfree(). In this case, we can eliminate checks in all > the callers. But we don't, in general, put sanity checks on arguments > without a good reason. > > Your reasons aren't good. "The driver doesn't care" -- well, just pass > the driver's name, then. Thanks for your explanation. OK, force the driver to pass a pool name. Sorry for noises. -- Baolin.wang Best Regards
