| Message ID | 20190613183729.GA32085@kroah.com (mailing list archive) |
|---|---|
| State | Mainlined |
| Delegated to: | Eduardo Valentin |
| Headers | show |
| Series | thermal: bcm2835: no need to check return value of debugfs_create functions | expand |
On 13/06/2019 20:37, Greg Kroah-Hartman wrote: > 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. Sorry if I'm missing some context but does it mean we do no longer take care of roll-backing anything? It is acceptable to have half of the debugfs set for example? Or a parent failing to create and the children polluting the root debugfs dir because the parent is NULL? > Cc: Zhang Rui <rui.zhang@intel.com> > Cc: Eduardo Valentin <edubezval@gmail.com> > Cc: Daniel Lezcano <daniel.lezcano@linaro.org> > Cc: Florian Fainelli <f.fainelli@gmail.com> > Cc: Ray Jui <rjui@broadcom.com> > Cc: Scott Branden <sbranden@broadcom.com> > Cc: bcm-kernel-feedback-list@broadcom.com > Cc: linux-pm@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/thermal/broadcom/bcm2835_thermal.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c > index ba39647a690c..3199977f1e73 100644 > --- a/drivers/thermal/broadcom/bcm2835_thermal.c > +++ b/drivers/thermal/broadcom/bcm2835_thermal.c > @@ -123,8 +123,6 @@ static void bcm2835_thermal_debugfs(struct platform_device *pdev) > struct debugfs_regset32 *regset; > > data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL); > - if (!data->debugfsdir) > - return; > > regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL); > if (!regset) >
On Thu, Jun 13, 2019 at 10:18:13PM +0200, Daniel Lezcano wrote: > On 13/06/2019 20:37, Greg Kroah-Hartman wrote: > > 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. > > Sorry if I'm missing some context but does it mean we do no longer take > care of roll-backing anything? Yes, but how can that happen here? > It is acceptable to have half of the debugfs set for example? Yes, your code should never care about this. > Or a parent failing to create and the children polluting the root > debugfs dir because the parent is NULL? How can the parent be NULL? Remember, debugfs_create_dir() can never return NULL, so that should not happen. And even if it does, that's fine, who cares :) thanks, greg k-h
On 14/06/2019 07:05, Greg Kroah-Hartman wrote: > On Thu, Jun 13, 2019 at 10:18:13PM +0200, Daniel Lezcano wrote: >> On 13/06/2019 20:37, Greg Kroah-Hartman wrote: >>> 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. >> >> Sorry if I'm missing some context but does it mean we do no longer take >> care of roll-backing anything? > > Yes, but how can that happen here? > >> It is acceptable to have half of the debugfs set for example? > > Yes, your code should never care about this. > >> Or a parent failing to create and the children polluting the root >> debugfs dir because the parent is NULL? > > How can the parent be NULL? Remember, debugfs_create_dir() can never > return NULL, so that should not happen. And even if it does, that's > fine, who cares :) Right, debugfs_create_dir() returns an PTR_ERR which can be passed to debugfs_create_file() as the parent parameter but this is handled in start_creating(). Ok, thanks for the clarification. -- Daniel
diff --git a/drivers/thermal/broadcom/bcm2835_thermal.c b/drivers/thermal/broadcom/bcm2835_thermal.c index ba39647a690c..3199977f1e73 100644 --- a/drivers/thermal/broadcom/bcm2835_thermal.c +++ b/drivers/thermal/broadcom/bcm2835_thermal.c @@ -123,8 +123,6 @@ static void bcm2835_thermal_debugfs(struct platform_device *pdev) struct debugfs_regset32 *regset; data->debugfsdir = debugfs_create_dir("bcm2835_thermal", NULL); - if (!data->debugfsdir) - return; regset = devm_kzalloc(&pdev->dev, sizeof(*regset), GFP_KERNEL); if (!regset)
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. Cc: Zhang Rui <rui.zhang@intel.com> Cc: Eduardo Valentin <edubezval@gmail.com> Cc: Daniel Lezcano <daniel.lezcano@linaro.org> Cc: Florian Fainelli <f.fainelli@gmail.com> Cc: Ray Jui <rjui@broadcom.com> Cc: Scott Branden <sbranden@broadcom.com> Cc: bcm-kernel-feedback-list@broadcom.com Cc: linux-pm@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/thermal/broadcom/bcm2835_thermal.c | 2 -- 1 file changed, 2 deletions(-)