gpiolib: no need to cast away return value of debugfs_create_file()

PATCH In review Greg Kroah-Hartman 5 participants 12 messages Jun 18, 2019 – Jun 25, 2019

Subsystems (3)

GPIO Tegra architecture support GPIO mockup driver

lore

Share & tools

lore

Greg Kroah-Hartman

author

2019-06-18 15:50 UTC #0 lore raw

PATCH 1/3 gpiolib: no need to cast away return value of debugfs_create_file()

It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all.

Cc Linus Walleij <[email protected]>

Cc Bartosz Golaszewski <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/gpio/gpiolib.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/gpio/gpiolib.c +2 -2

index e013d417a936..d378774d3456 100644
@@ -4805,8 +4805,8 @@ static const struct file_operations gpiolib_operations = {
 static int __init gpiolib_debugfs_init(void)
 {
 	/* /sys/kernel/debug/gpio */
-	(void) debugfs_create_file("gpio", S_IFREG | S_IRUGO,
-				NULL, NULL, &gpiolib_operations);
+	debugfs_create_file("gpio", S_IFREG | S_IRUGO, NULL, NULL,
+			    &gpiolib_operations);
 	return 0;
 }
 subsys_initcall(gpiolib_debugfs_init);

Signature · 1 line

2.22.0

Greg Kroah-Hartman

author

2019-06-18 15:51 UTC #1 lore raw

PATCH 2/3 gpio: tegra: no need to cast away return value of debugfs_create_file()

It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all.

Cc Linus Walleij <[email protected]>

Cc Bartosz Golaszewski <[email protected]>

Cc Thierry Reding <[email protected]>

Cc Jonathan Hunter <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/gpio/gpio-tegra.c +2 -2

index 6d9b6906b9d0..a54bba1bda6c 100644
@@ -550,8 +550,8 @@ DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
 
 static void tegra_gpio_debuginit(struct tegra_gpio_info *tgi)
 {
-	(void) debugfs_create_file("tegra_gpio", 0444,
-				   NULL, tgi, &tegra_dbg_gpio_fops);
+	debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
+			    &tegra_dbg_gpio_fops);
 }
 
 #else

Signature · 1 line

2.22.0

Jon Hunter

maintainer Acked-by on v1 2/3

2019-06-18 16:26 UTC #2 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

Quoted 1 line · Greg Kroah-Hartman

It is fine to ignore the return value (and encouraged), so need to cast

I assume you mean 'no need' here.

Quoted 18 lines

away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpio/gpio-tegra.c +0 -0
index 6d9b6906b9d0..a54bba1bda6c 100644

drivers/gpio/gpio-tegra.c DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio); · line 557

Quoted 10 lines

drivers/gpio/gpio-tegra.c +2 -2

@@ -550,8 +550,8 @@ DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
 
 static void tegra_gpio_debuginit(struct tegra_gpio_info *tgi)
 {
-	(void) debugfs_create_file("tegra_gpio", 0444,
-				   NULL, tgi, &tegra_dbg_gpio_fops);
+	debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
+			    &tegra_dbg_gpio_fops);
 }
 
 #else

Looks like we can get rid of this entire function and just call debugfs_create_file directly to simplify the code even more. We can handle that.

Acked-by Jon Hunter <[email protected]>

Thanks
Jon

Signature · 1 line

nvpublic

Greg Kroah-Hartman

author

2019-06-18 17:17 UTC #3 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

Quoted 5 lines · Jon Hunter

Quoted 1 line · Greg Kroah-Hartman
It is fine to ignore the return value (and encouraged), so need to cast
I assume you mean 'no need' here.

{sigh} yes :)

Quoted 27 lines

Quoted 27 lines
away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpio/gpio-tegra.c +2 -2
index 6d9b6906b9d0..a54bba1bda6c 100644
@@ -550,8 +550,8 @@ DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
 
 static void tegra_gpio_debuginit(struct tegra_gpio_info *tgi)
 {
-	(void) debugfs_create_file("tegra_gpio", 0444,
-				   NULL, tgi, &tegra_dbg_gpio_fops);
+	debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
+			    &tegra_dbg_gpio_fops);
 }
 

Quoted 7 lines

drivers/gpio/gpio-tegra.c DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio); · line 557
Quoted 1 line
drivers/gpio/gpio-tegra.c +0 -0
 #else
Looks like we can get rid of this entire function and just call debugfs_create_file directly to simplify the code even more. We can handle that.
Acked-by Jon Hunter <[email protected]>

Great! Do you want to just take this through your tree and then add that change on top of it? Or do you need me to take it and make that change?

thanks,

greg k-h

Jon Hunter

maintainer

2019-06-18 17:49 UTC #4 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

Quoted 42 lines · Greg Kroah-Hartman

Quoted 5 lines · Jon Hunter
Quoted 1 line · Greg Kroah-Hartman
It is fine to ignore the return value (and encouraged), so need to cast
I assume you mean 'no need' here.
{sigh} yes :)
Quoted 33 lines
Quoted 18 lines
away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpio/gpio-tegra.c +0 -0
index 6d9b6906b9d0..a54bba1bda6c 100644
drivers/gpio/gpio-tegra.c DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio); · line 557
Quoted 10 lines
drivers/gpio/gpio-tegra.c +2 -2
@@ -550,8 +550,8 @@ DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
 
 static void tegra_gpio_debuginit(struct tegra_gpio_info *tgi)
 {
-	(void) debugfs_create_file("tegra_gpio", 0444,
-				   NULL, tgi, &tegra_dbg_gpio_fops);
+	debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
+			    &tegra_dbg_gpio_fops);
 }
 
 #else
Looks like we can get rid of this entire function and just call debugfs_create_file directly to simplify the code even more. We can handle that.

Quoted 5 lines

Quoted 1 line
Acked-by Jon Hunter <[email protected]>
Great! Do you want to just take this through your tree and then add that change on top of it? Or do you need me to take it and make that change?

No its fine, we can fix this.

Cheers
Jon

Signature · 1 line

nvpublic

Greg Kroah-Hartman

author

2019-06-18 18:05 UTC #5 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

Quoted 44 lines · Jon Hunter

Quoted 42 lines · Greg Kroah-Hartman
Quoted 5 lines · Jon Hunter
Quoted 1 line · Greg Kroah-Hartman
It is fine to ignore the return value (and encouraged), so need to cast
I assume you mean 'no need' here.
{sigh} yes :)
Quoted 33 lines
Quoted 18 lines
away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>
---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/gpio/gpio-tegra.c +0 -0
index 6d9b6906b9d0..a54bba1bda6c 100644
drivers/gpio/gpio-tegra.c DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio); · line 557
Quoted 10 lines
drivers/gpio/gpio-tegra.c +2 -2
@@ -550,8 +550,8 @@ DEFINE_SHOW_ATTRIBUTE(tegra_dbg_gpio);
 
 static void tegra_gpio_debuginit(struct tegra_gpio_info *tgi)
 {
-	(void) debugfs_create_file("tegra_gpio", 0444,
-				   NULL, tgi, &tegra_dbg_gpio_fops);
+	debugfs_create_file("tegra_gpio", 0444, NULL, tgi,
+			    &tegra_dbg_gpio_fops);
 }
 
 #else
Looks like we can get rid of this entire function and just call debugfs_create_file directly to simplify the code even more. We can handle that.

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Acked-by Jon Hunter <[email protected]>
Great! Do you want to just take this through your tree and then add that change on top of it? Or do you need me to take it and make that change?
No its fine, we can fix this.

Wonderful, thank you!

greg k-h

Thierry Reding

Acked-by on v1 2/3

2019-06-19 08:19 UTC #6 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

In reply to Greg Kroah-Hartman

Quoted 10 lines · Greg Kroah-Hartman

It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>

Quoted 3 lines

---
 drivers/gpio/gpio-tegra.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by Thierry Reding <[email protected]>

Linus Walleij

2019-06-25 12:45 UTC #7 lore raw

Re: [PATCH 2/3] gpio: tegra: no need to cast away return value of debugfs_create_file()

In reply to Greg Kroah-Hartman

Quoted 10 lines · Greg Kroah-Hartman

It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc Thierry Reding <[email protected]>
Cc Jonathan Hunter <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>

Patch applied, fixing wording and stacking up the ACKs.

Yours,
Linus Walleij

Greg Kroah-Hartman

author

2019-06-18 15:51 UTC #8 lore raw

PATCH 3/3 gpio: mockup: no need to check return value of debugfs_create functions

In reply to Greg Kroah-Hartman

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 Bamvor Jian Zhang <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Bartosz Golaszewski <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/gpio/gpio-mockup.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

drivers/gpio/gpio-mockup.c +2 ~~-11~~

index b6a4efce7c92..f955aff6dd82 100644
@@ -325,8 +325,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	devname = dev_name(&gc->gpiodev->dev);
 
 	chip->dbg_dir = debugfs_create_dir(devname, gpio_mockup_dbg_dir);
-	if (IS_ERR_OR_NULL(chip->dbg_dir))
-		goto err;
 
 	for (i = 0; i < gc->ngpio; i++) {
 		name = devm_kasprintf(dev, GFP_KERNEL, "%d", i);

@@ -341,16 +339,11 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 		priv->offset = i;
 		priv->desc = &gc->gpiodev->descs[i];
 
-		evfile = debugfs_create_file(name, 0200, chip->dbg_dir, priv,
-					     &gpio_mockup_debugfs_ops);
-		if (IS_ERR_OR_NULL(evfile))
-			goto err;
+		debugfs_create_file(name, 0200, chip->dbg_dir, priv,
+				    &gpio_mockup_debugfs_ops);
 	}
 
 	return;
-
-err:
-	dev_err(dev, "error creating debugfs files\n");
 }
 
 static int gpio_mockup_name_lines(struct device *dev,

@@ -501,8 +494,6 @@ static int __init gpio_mockup_init(void)
 	}
 
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup", NULL);
-	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
-		gpio_mockup_err("error creating debugfs directory\n");
 
 	err = platform_driver_register(&gpio_mockup_driver);
 	if (err) {

Signature · 1 line

2.22.0

Greg Kroah-Hartman

author

2019-06-20 07:25 UTC #9 lore raw

PATCH v2 1/1 gpio: mockup: no need to check return value of debugfs_create functions

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 Bamvor Jian Zhang <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Bartosz Golaszewski <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
v2: fix build warning found by kbuild
fix build error found by kbuild. Did I even build this thing myself???

 drivers/gpio/gpio-mockup.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

drivers/gpio/gpio-mockup.c +4 ~~-14~~

index b6a4efce7c92..135dac099d1e 100644
@@ -315,7 +315,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 				      struct gpio_mockup_chip *chip)
 {
 	struct gpio_mockup_dbgfs_private *priv;
-	struct dentry *evfile;
 	struct gpio_chip *gc;
 	const char *devname;
 	char *name;

@@ -325,32 +324,25 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
 	devname = dev_name(&gc->gpiodev->dev);
 
 	chip->dbg_dir = debugfs_create_dir(devname, gpio_mockup_dbg_dir);
-	if (IS_ERR_OR_NULL(chip->dbg_dir))
-		goto err;
 
 	for (i = 0; i < gc->ngpio; i++) {
 		name = devm_kasprintf(dev, GFP_KERNEL, "%d", i);
 		if (!name)
-			goto err;
+			return;
 
 		priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
 		if (!priv)
-			goto err;
+			return;
 
 		priv->chip = chip;
 		priv->offset = i;
 		priv->desc = &gc->gpiodev->descs[i];
 
-		evfile = debugfs_create_file(name, 0200, chip->dbg_dir, priv,
-					     &gpio_mockup_debugfs_ops);
-		if (IS_ERR_OR_NULL(evfile))
-			goto err;
+		debugfs_create_file(name, 0200, chip->dbg_dir, priv,
+				    &gpio_mockup_debugfs_ops);
 	}
 
 	return;
-
-err:
-	dev_err(dev, "error creating debugfs files\n");
 }
 
 static int gpio_mockup_name_lines(struct device *dev,

@@ -501,8 +493,6 @@ static int __init gpio_mockup_init(void)
 	}
 
 	gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup", NULL);
-	if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
-		gpio_mockup_err("error creating debugfs directory\n");
 
 	err = platform_driver_register(&gpio_mockup_driver);
 	if (err) {

Signature · 1 line

2.22.0

Bartosz Golaszewski

2019-06-24 08:21 UTC #10 lore raw

Re: [PATCH v2] gpio: mockup: no need to check return value of debugfs_create functions

czw., 20 cze 2019 o 09:25 Greg Kroah-Hartman
<[email protected]> napisał(a):

Quoted 77 lines

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 Bamvor Jian Zhang <[email protected]>

Cc Linus Walleij <[email protected]>

Cc Bartosz Golaszewski <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
v2: fix build warning found by kbuild
fix build error found by kbuild. Did I even build this thing myself???

 drivers/gpio/gpio-mockup.c | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

drivers/gpio/gpio-mockup.c +4 ~~-15~~

index b6a4efce7c92..135dac099d1e 100644
@@ -315,7 +315,6 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
                                      struct gpio_mockup_chip *chip)
 {
        struct gpio_mockup_dbgfs_private *priv;
-       struct dentry *evfile;
        struct gpio_chip *gc;
        const char *devname;
        char *name;

@@ -325,32 +324,25 @@ static void gpio_mockup_debugfs_setup(struct device *dev,
        devname = dev_name(&gc->gpiodev->dev);
 
        chip->dbg_dir = debugfs_create_dir(devname, gpio_mockup_dbg_dir);
-       if (IS_ERR_OR_NULL(chip->dbg_dir))
-               goto err;
 
        for (i = 0; i < gc->ngpio; i++) {
                name = devm_kasprintf(dev, GFP_KERNEL, "%d", i);
                if (!name)
-                       goto err;
+                       return;
 
                priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
                if (!priv)
-                       goto err;
+                       return;
 
                priv->chip = chip;
                priv->offset = i;
                priv->desc = &gc->gpiodev->descs[i];
 
-               evfile = debugfs_create_file(name, 0200, chip->dbg_dir, priv,
-                                            &gpio_mockup_debugfs_ops);
-               if (IS_ERR_OR_NULL(evfile))
-                       goto err;
+               debugfs_create_file(name, 0200, chip->dbg_dir, priv,
+                                   &gpio_mockup_debugfs_ops);
        }
 
        return;
-
-err:
-       dev_err(dev, "error creating debugfs files\n");
 }
 
 static int gpio_mockup_name_lines(struct device *dev,

@@ -501,8 +493,6 @@ static int __init gpio_mockup_init(void)
        }
 
        gpio_mockup_dbg_dir = debugfs_create_dir("gpio-mockup", NULL);
-       if (IS_ERR_OR_NULL(gpio_mockup_dbg_dir))
-               gpio_mockup_err("error creating debugfs directory\n");
 
        err = platform_driver_register(&gpio_mockup_driver);
        if (err) {
--

Quoted 2 lines

2.22.0

Shouldn't this patch also make the driver unconditionally call gpio_mockup_debugfs_setup() at line 450?

Bart

Linus Walleij

2019-06-25 12:29 UTC #11 lore raw

In reply to Greg Kroah-Hartman

Quoted 7 lines · Greg Kroah-Hartman

It is fine to ignore the return value (and encouraged), so need to cast away the return value, you will not get a build warning at all.
Cc Linus Walleij <[email protected]>
Cc Bartosz Golaszewski <[email protected]>
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>

Patch applied.

Yours,
Linus Walleij