Skip to contentSkip to conversation

lvts_thermal.c: Fix error checking for debugfs_create_dir

PATCH In review Osama Muhammad 2 participants 4 messages May 17, 2023 – May 30, 2023 Thermal

lore
Share & tools
lore

Osama Muhammad

author
#0 lore raw

PATCH 1/1 lvts_thermal.c: Fix error checking for debugfs_create_dir

This patch fixes the error checking in lvts_thermal.c in debugfs_create_dir. The correct way to check if an error occurred is 'IS_ERR' inline function.

Signed-off-by Osama Muhammad <[email protected]>
---
 drivers/thermal/mediatek/lvts_thermal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/thermal/mediatek/lvts_thermal.c +2 -2
index d0a3f95b7884..61386be78fa0 100644
@@ -188,7 +188,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 	int i;
 
 	lvts_td->dom_dentry = debugfs_create_dir(dev_name(dev), NULL);
-	if (!lvts_td->dom_dentry)
+	if (IS_ERR(lvts_td->dom_dentry))
 		return 0;
 
 	for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {

@@ -197,7 +197,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 
 		sprintf(name, "controller%d", i);
 		dentry = debugfs_create_dir(name, lvts_td->dom_dentry);
-		if (!dentry)
+		if (IS_ERR(dentry))
 			continue;
 
 		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
Signature · 1 line
2.34.1
#1 lore raw
Quoted 3 lines · Osama Muhammad

This patch fixes the error checking in lvts_thermal.c in debugfs_create_dir. The correct way to check if an error occurred is 'IS_ERR' inline function.

We do no longer check debugfs functions return values.

eg.

https://www.spinics.net/lists/linux-spi/msg37903.html
https://lore.kernel.org/lkml/2023052835-oxidant-doily-404f@gregkh/

Quoted 27 lines
Signed-off-by Osama Muhammad <[email protected]>
---
  drivers/thermal/mediatek/lvts_thermal.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
drivers/thermal/mediatek/lvts_thermal.c +2 -2
index d0a3f95b7884..61386be78fa0 100644
@@ -188,7 +188,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
  	int i;
 
  	lvts_td->dom_dentry = debugfs_create_dir(dev_name(dev), NULL);
-	if (!lvts_td->dom_dentry)
+	if (IS_ERR(lvts_td->dom_dentry))
  		return 0;
 
  	for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {

@@ -197,7 +197,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 
  		sprintf(name, "controller%d", i);
  		dentry = debugfs_create_dir(name, lvts_td->dom_dentry);
-		if (!dentry)
+		if (IS_ERR(dentry))
  			continue;
 
  		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
author
#2 lore raw

Hi,

After Reading more about it I also learned about this and I was planning to send a patch to remove the error checking for debugfs API.Should I send the patch for it?

Thanks
osmten

Quoted 49 lines · Daniel Lezcano
Quoted 3 lines · Osama Muhammad

This patch fixes the error checking in lvts_thermal.c in debugfs_create_dir. The correct way to check if an error occurred is 'IS_ERR' inline function.

We do no longer check debugfs functions return values.

eg.

https://www.spinics.net/lists/linux-spi/msg37903.html
https://lore.kernel.org/lkml/2023052835-oxidant-doily-404f@gregkh/

Quoted 19 lines
Signed-off-by Osama Muhammad <[email protected]>
---
  drivers/thermal/mediatek/lvts_thermal.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)
drivers/thermal/mediatek/lvts_thermal.c +1 -1
index d0a3f95b7884..61386be78fa0 100644
@@ -188,7 +188,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
      int i;
 
      lvts_td->dom_dentry = debugfs_create_dir(dev_name(dev), NULL);
-     if (!lvts_td->dom_dentry)
+     if (IS_ERR(lvts_td->dom_dentry))
              return 0;
 
      for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {

drivers/thermal/mediatek/lvts_thermal.c static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td) · line 203

Quoted 8 lines
drivers/thermal/mediatek/lvts_thermal.c +1 -1
@@ -197,7 +197,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 
              sprintf(name, "controller%d", i);
              dentry = debugfs_create_dir(name, lvts_td->dom_dentry);
-             if (!dentry)
+             if (IS_ERR(dentry))
                      continue;
 
              regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

#3 lore raw
Quoted 6 lines · Osama Muhammad

Hi,

After Reading more about it I also learned about this and I was planning to send a patch to remove the error checking for debugfs API.Should I send the patch for it?

Yes, you can change also the prototype of the lvts_debugfs_init() function by returning void

And then replace the call site:

        return lvts_debugfs_init(dev, lvts_td);

by:

	lvts_debugfs_init(dev, lvts_td);

	return 0;
Quoted 53 lines

osmten

Quoted 49 lines · Daniel Lezcano
Quoted 3 lines · Osama Muhammad

This patch fixes the error checking in lvts_thermal.c in debugfs_create_dir. The correct way to check if an error occurred is 'IS_ERR' inline function.

We do no longer check debugfs functions return values.

eg.

   https://www.spinics.net/lists/linux-spi/msg37903.html
   https://lore.kernel.org/lkml/2023052835-oxidant-doily-404f@gregkh/
Quoted 19 lines
Signed-off-by Osama Muhammad <[email protected]>
---
   drivers/thermal/mediatek/lvts_thermal.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)
drivers/thermal/mediatek/lvts_thermal.c +1 -1
index d0a3f95b7884..61386be78fa0 100644
@@ -188,7 +188,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
       int i;
 
       lvts_td->dom_dentry = debugfs_create_dir(dev_name(dev), NULL);
-     if (!lvts_td->dom_dentry)
+     if (IS_ERR(lvts_td->dom_dentry))
               return 0;
 
       for (i = 0; i < lvts_td->num_lvts_ctrl; i++) {

drivers/thermal/mediatek/lvts_thermal.c static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td) · line 203

Quoted 8 lines
drivers/thermal/mediatek/lvts_thermal.c +1 -1
@@ -197,7 +197,7 @@ static int lvts_debugfs_init(struct device *dev, struct lvts_domain *lvts_td)
 
               sprintf(name, "controller%d", i);
               dentry = debugfs_create_dir(name, lvts_td->dom_dentry);
-             if (!dentry)
+             if (IS_ERR(dentry))
                       continue;
 
               regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);

--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Signature · 5 lines
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog