Message ID | 1479251395-3100-1-git-send-email-sudipm.mukherjee@gmail.com |
---|---|
State | Superseded |
Headers | show |
On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: > debugfs_create_dir() and debugfs_create_file() returns NULL on error or > a pointer on success. They do not return the error value with ERR_PTR. > So we should not check the return with IS_ERR_OR_NULL, instead we > should just check for NULL. > > Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> > --- > drivers/mtd/nand/nandsim.c | 9 +++------ > 1 file changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c > index c76287a..9b0d79a 100644 > --- a/drivers/mtd/nand/nandsim.c > +++ b/drivers/mtd/nand/nandsim.c > @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct nandsim *dev) > { > struct nandsim_debug_info *dbg = &dev->dbg; > struct dentry *dent; > - int err; > + int err = -ENODEV; Why don't you just nuke the err altogether and just return -ENODEV ? > if (!IS_ENABLED(CONFIG_DEBUG_FS)) > return 0; > > dent = debugfs_create_dir("nandsim", NULL); > - if (IS_ERR_OR_NULL(dent)) { > - int err = dent ? -ENODEV : PTR_ERR(dent); > - > + if (!dent) { > NS_ERR("cannot create \"nandsim\" debugfs directory, err %d\n", > err); > return err; > @@ -542,7 +540,7 @@ static int nandsim_debugfs_create(struct nandsim *dev) > > dent = debugfs_create_file("wear_report", S_IRUSR, > dbg->dfs_root, dev, &dfs_fops); > - if (IS_ERR_OR_NULL(dent)) > + if (!dent) > goto out_remove; > dbg->dfs_wear_report = dent; > > @@ -550,7 +548,6 @@ static int nandsim_debugfs_create(struct nandsim *dev) > > out_remove: > debugfs_remove_recursive(dbg->dfs_root); > - err = dent ? PTR_ERR(dent) : -ENODEV; > return err; > } > >
On Tuesday 15 November 2016 11:42 PM, Marek Vasut wrote: > On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: >> debugfs_create_dir() and debugfs_create_file() returns NULL on error or >> a pointer on success. They do not return the error value with ERR_PTR. >> So we should not check the return with IS_ERR_OR_NULL, instead we >> should just check for NULL. >> >> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >> --- >> drivers/mtd/nand/nandsim.c | 9 +++------ >> 1 file changed, 3 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >> index c76287a..9b0d79a 100644 >> --- a/drivers/mtd/nand/nandsim.c >> +++ b/drivers/mtd/nand/nandsim.c >> @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct nandsim *dev) >> { >> struct nandsim_debug_info *dbg = &dev->dbg; >> struct dentry *dent; >> - int err; >> + int err = -ENODEV; > > Why don't you just nuke the err altogether and just return -ENODEV ? That was the first version which i made and discarded before sending. I will go and find it now. Regards Sudip
On 11/16/2016 08:52 AM, Sudip Mukherjee wrote: > On Tuesday 15 November 2016 11:42 PM, Marek Vasut wrote: >> On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: >>> debugfs_create_dir() and debugfs_create_file() returns NULL on error or >>> a pointer on success. They do not return the error value with ERR_PTR. >>> So we should not check the return with IS_ERR_OR_NULL, instead we >>> should just check for NULL. >>> >>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >>> --- >>> drivers/mtd/nand/nandsim.c | 9 +++------ >>> 1 file changed, 3 insertions(+), 6 deletions(-) >>> >>> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >>> index c76287a..9b0d79a 100644 >>> --- a/drivers/mtd/nand/nandsim.c >>> +++ b/drivers/mtd/nand/nandsim.c >>> @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct >>> nandsim *dev) >>> { >>> struct nandsim_debug_info *dbg = &dev->dbg; >>> struct dentry *dent; >>> - int err; >>> + int err = -ENODEV; >> >> Why don't you just nuke the err altogether and just return -ENODEV ? > > That was the first version which i made and discarded before sending. I > will go and find it now. Why did you discard it ?
On Wednesday 16 November 2016 04:56 PM, Marek Vasut wrote: > On 11/16/2016 08:52 AM, Sudip Mukherjee wrote: >> On Tuesday 15 November 2016 11:42 PM, Marek Vasut wrote: >>> On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: >>>> debugfs_create_dir() and debugfs_create_file() returns NULL on error or >>>> a pointer on success. They do not return the error value with ERR_PTR. >>>> So we should not check the return with IS_ERR_OR_NULL, instead we >>>> should just check for NULL. >>>> >>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >>>> --- >>>> drivers/mtd/nand/nandsim.c | 9 +++------ >>>> 1 file changed, 3 insertions(+), 6 deletions(-) >>>> >>>> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >>>> index c76287a..9b0d79a 100644 >>>> --- a/drivers/mtd/nand/nandsim.c >>>> +++ b/drivers/mtd/nand/nandsim.c >>>> @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct >>>> nandsim *dev) >>>> { >>>> struct nandsim_debug_info *dbg = &dev->dbg; >>>> struct dentry *dent; >>>> - int err; >>>> + int err = -ENODEV; >>> >>> Why don't you just nuke the err altogether and just return -ENODEV ? >> >> That was the first version which i made and discarded before sending. I >> will go and find it now. > > Why did you discard it ? > That is actually a stupid reason. I was confused with the print statement which uses err. NS_ERR("cannot create \"nandsim\" debugfs directory, err %d\n", err); Regards Sudip
On 11/16/2016 09:56 PM, Sudip Mukherjee wrote: > On Wednesday 16 November 2016 04:56 PM, Marek Vasut wrote: >> On 11/16/2016 08:52 AM, Sudip Mukherjee wrote: >>> On Tuesday 15 November 2016 11:42 PM, Marek Vasut wrote: >>>> On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: >>>>> debugfs_create_dir() and debugfs_create_file() returns NULL on >>>>> error or >>>>> a pointer on success. They do not return the error value with ERR_PTR. >>>>> So we should not check the return with IS_ERR_OR_NULL, instead we >>>>> should just check for NULL. >>>>> >>>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >>>>> --- >>>>> drivers/mtd/nand/nandsim.c | 9 +++------ >>>>> 1 file changed, 3 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >>>>> index c76287a..9b0d79a 100644 >>>>> --- a/drivers/mtd/nand/nandsim.c >>>>> +++ b/drivers/mtd/nand/nandsim.c >>>>> @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct >>>>> nandsim *dev) >>>>> { >>>>> struct nandsim_debug_info *dbg = &dev->dbg; >>>>> struct dentry *dent; >>>>> - int err; >>>>> + int err = -ENODEV; >>>> >>>> Why don't you just nuke the err altogether and just return -ENODEV ? >>> >>> That was the first version which i made and discarded before sending. I >>> will go and find it now. >> >> Why did you discard it ? >> > > That is actually a stupid reason. I was confused with the print > statement which uses err. > > NS_ERR("cannot create \"nandsim\" debugfs directory, err %d\n", > err); > Ah, got it, so yeah, nuke the err altogether :)
On Thursday 17 November 2016 01:21 PM, Marek Vasut wrote: > On 11/16/2016 09:56 PM, Sudip Mukherjee wrote: >> On Wednesday 16 November 2016 04:56 PM, Marek Vasut wrote: >>> On 11/16/2016 08:52 AM, Sudip Mukherjee wrote: >>>> On Tuesday 15 November 2016 11:42 PM, Marek Vasut wrote: >>>>> On 11/16/2016 12:09 AM, Sudip Mukherjee wrote: >>>>>> debugfs_create_dir() and debugfs_create_file() returns NULL on >>>>>> error or >>>>>> a pointer on success. They do not return the error value with ERR_PTR. >>>>>> So we should not check the return with IS_ERR_OR_NULL, instead we >>>>>> should just check for NULL. >>>>>> >>>>>> Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> >>>>>> --- >>>>>> drivers/mtd/nand/nandsim.c | 9 +++------ >>>>>> 1 file changed, 3 insertions(+), 6 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c >>>>>> index c76287a..9b0d79a 100644 >>>>>> --- a/drivers/mtd/nand/nandsim.c >>>>>> +++ b/drivers/mtd/nand/nandsim.c >>>>>> @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct >>>>>> nandsim *dev) >>>>>> { >>>>>> struct nandsim_debug_info *dbg = &dev->dbg; >>>>>> struct dentry *dent; >>>>>> - int err; >>>>>> + int err = -ENODEV; >>>>> >>>>> Why don't you just nuke the err altogether and just return -ENODEV ? >>>> >>>> That was the first version which i made and discarded before sending. I >>>> will go and find it now. >>> >>> Why did you discard it ? >>> >> >> That is actually a stupid reason. I was confused with the print >> statement which uses err. >> >> NS_ERR("cannot create \"nandsim\" debugfs directory, err %d\n", >> err); >> > > Ah, got it, so yeah, nuke the err altogether :) v2 is already sent yesterday. Regards Sudip
diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c index c76287a..9b0d79a 100644 --- a/drivers/mtd/nand/nandsim.c +++ b/drivers/mtd/nand/nandsim.c @@ -525,15 +525,13 @@ static int nandsim_debugfs_create(struct nandsim *dev) { struct nandsim_debug_info *dbg = &dev->dbg; struct dentry *dent; - int err; + int err = -ENODEV; if (!IS_ENABLED(CONFIG_DEBUG_FS)) return 0; dent = debugfs_create_dir("nandsim", NULL); - if (IS_ERR_OR_NULL(dent)) { - int err = dent ? -ENODEV : PTR_ERR(dent); - + if (!dent) { NS_ERR("cannot create \"nandsim\" debugfs directory, err %d\n", err); return err; @@ -542,7 +540,7 @@ static int nandsim_debugfs_create(struct nandsim *dev) dent = debugfs_create_file("wear_report", S_IRUSR, dbg->dfs_root, dev, &dfs_fops); - if (IS_ERR_OR_NULL(dent)) + if (!dent) goto out_remove; dbg->dfs_wear_report = dent; @@ -550,7 +548,6 @@ static int nandsim_debugfs_create(struct nandsim *dev) out_remove: debugfs_remove_recursive(dbg->dfs_root); - err = dent ? PTR_ERR(dent) : -ENODEV; return err; }
debugfs_create_dir() and debugfs_create_file() returns NULL on error or a pointer on success. They do not return the error value with ERR_PTR. So we should not check the return with IS_ERR_OR_NULL, instead we should just check for NULL. Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk> --- drivers/mtd/nand/nandsim.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-)