On Sat, Aug 10, 2019 at 09:27:26PM +0200, Robert Jarzmik wrote: > Greg Kroah-Hartman <[email protected]> writes: > > Hi Greg, > > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Also, because there is no need to save the file dentry, remove the > > variable that was saving it as it was never even being used once set. > > > > Cc: Daniel Mack <[email protected]> > > Cc: Haojian Zhuang <[email protected]> > > Cc: Robert Jarzmik <[email protected]> > > Cc: Vinod Koul <[email protected]> > > Cc: Dan Williams <[email protected]> > > Cc: [email protected] > > Cc: [email protected] > > Cc: [email protected] > > Signed-off-by: Greg Kroah-Hartman <[email protected]> > > --- > > drivers/dma/pxa_dma.c | 56 +++++++++---------------------------------- > > 1 file changed, 11 insertions(+), 45 deletions(-) > > > > diff --git a/drivers/dma/pxa_dma.c b/drivers/dma/pxa_dma.c > > index b429642f3e7a..0f698f49ee26 100644 > > --- a/drivers/dma/pxa_dma.c > > +++ b/drivers/dma/pxa_dma.c > > @@ -132,7 +132,6 @@ struct pxad_device { > > spinlock_t phy_lock; /* Phy association */ > > #ifdef CONFIG_DEBUG_FS > > struct dentry *dbgfs_root; > > - struct dentry *dbgfs_state; > > struct dentry **dbgfs_chan; > > #endif > > }; > > @@ -326,31 +325,18 @@ static struct dentry *pxad_dbg_alloc_chan(struct > > pxad_device *pdev, > > int ch, struct dentry *chandir) > > { > > char chan_name[11]; > > - struct dentry *chan, *chan_state = NULL, *chan_descr = NULL; > > - struct dentry *chan_reqs = NULL; > > + struct dentry *chan; > > void *dt; > > > > scnprintf(chan_name, sizeof(chan_name), "%d", ch); > > chan = debugfs_create_dir(chan_name, chandir); > > dt = (void *)&pdev->phys[ch]; > > > > - if (chan) > > - chan_state = debugfs_create_file("state", 0400, chan, dt, > > - &chan_state_fops); > > - if (chan_state) > > - chan_descr = debugfs_create_file("descriptors", 0400, chan, dt, > > - &descriptors_fops); > > - if (chan_descr) > > - chan_reqs = debugfs_create_file("requesters", 0400, chan, dt, > > - &requester_chan_fops); > > - if (!chan_reqs) > > - goto err_state; > > + debugfs_create_file("state", 0400, chan, dt, &chan_state_fops); > > + debugfs_create_file("descriptors", 0400, chan, dt, &descriptors_fops); > > + debugfs_create_file("requesters", 0400, chan, dt, &requester_chan_fops); > > This is not strictly equivalent. > Imagine that the debugfs_create_dir() fails and returns NULL :
How can that happen? > - in the former case, neither "state", "descriptors" nor "requesters" would > be > created > - in the new code, "state", "descriptors" nor "requesters" will be created in > the debugfs root directory I agree, but debugfs_create_dir() does not return a NULL on an error since many kernel releases. Neither can debugfs_create_file() so really this test is not working at all as-is :) thanks, greg k-h
