linux-wpan.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
       [not found] <20190806161128.31232-1-gregkh@linuxfoundation.org>
@ 2019-08-06 16:11 ` Greg Kroah-Hartman
  2019-08-06 19:22   ` Stefan Schmidt
  0 siblings, 1 reply; 3+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 16:11 UTC (permalink / raw)
  To: netdev
  Cc: Greg Kroah-Hartman, Michael Hennerich, Alexander Aring,
	Stefan Schmidt, David S. Miller, Harry Morris, linux-wpan

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: Michael Hennerich <michael.hennerich@analog.com>
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@datenfreihafen.org>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Harry Morris <h.morris@cascoda.com>
Cc: linux-wpan@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 drivers/net/ieee802154/adf7242.c   | 12 +++---------
 drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
 drivers/net/ieee802154/ca8210.c    |  9 +--------
 3 files changed, 9 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
index c9392d70e639..7b95add2235a 100644
--- a/drivers/net/ieee802154/adf7242.c
+++ b/drivers/net/ieee802154/adf7242.c
@@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
 	return 0;
 }
 
-static int adf7242_debugfs_init(struct adf7242_local *lp)
+static void adf7242_debugfs_init(struct adf7242_local *lp)
 {
 	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
 	struct dentry *stats;
@@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
 	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
 
 	lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
-	if (IS_ERR_OR_NULL(lp->debugfs_root))
-		return PTR_ERR_OR_ZERO(lp->debugfs_root);
 
-	stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
-					    lp->debugfs_root,
-					    adf7242_stats_show);
-	return PTR_ERR_OR_ZERO(stats);
-
-	return 0;
+	debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
+				    adf7242_stats_show);
 }
 
 static const s32 adf7242_powers[] = {
diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
index 595cf7e2a651..7d67f41387f5 100644
--- a/drivers/net/ieee802154/at86rf230.c
+++ b/drivers/net/ieee802154/at86rf230.c
@@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
 }
 DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
 
-static int at86rf230_debugfs_init(struct at86rf230_local *lp)
+static void at86rf230_debugfs_init(struct at86rf230_local *lp)
 {
 	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
-	struct dentry *stats;
 
 	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
 
 	at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
-	if (!at86rf230_debugfs_root)
-		return -ENOMEM;
-
-	stats = debugfs_create_file("trac_stats", 0444,
-				    at86rf230_debugfs_root, lp,
-				    &at86rf230_stats_fops);
-	if (!stats)
-		return -ENOMEM;
 
-	return 0;
+	debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
+			    &at86rf230_stats_fops);
 }
 
 static void at86rf230_debugfs_remove(void)
@@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
 	debugfs_remove_recursive(at86rf230_debugfs_root);
 }
 #else
-static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
+static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
 static void at86rf230_debugfs_remove(void) { }
 #endif
 
@@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
 	/* going into sleep by default */
 	at86rf230_sleep(lp);
 
-	rc = at86rf230_debugfs_init(lp);
-	if (rc)
-		goto free_dev;
+	at86rf230_debugfs_init(lp);
 
 	rc = ieee802154_register_hw(lp->hw);
 	if (rc)
diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
index b188fce3f641..11402dc347db 100644
--- a/drivers/net/ieee802154/ca8210.c
+++ b/drivers/net/ieee802154/ca8210.c
@@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
 		priv,
 		&test_int_fops
 	);
-	if (IS_ERR(test->ca8210_dfs_spi_int)) {
-		dev_err(
-			&priv->spi->dev,
-			"Error %ld when creating debugfs node\n",
-			PTR_ERR(test->ca8210_dfs_spi_int)
-		);
-		return PTR_ERR(test->ca8210_dfs_spi_int);
-	}
+
 	debugfs_create_symlink("ca8210", NULL, node_name);
 	init_waitqueue_head(&test->readq);
 	return kfifo_alloc(
-- 
2.22.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
  2019-08-06 16:11 ` [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions Greg Kroah-Hartman
@ 2019-08-06 19:22   ` Stefan Schmidt
  2019-08-06 19:48     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 3+ messages in thread
From: Stefan Schmidt @ 2019-08-06 19:22 UTC (permalink / raw)
  To: Greg Kroah-Hartman, netdev
  Cc: Michael Hennerich, Alexander Aring, David S. Miller,
	Harry Morris, linux-wpan

Hello.

On 06.08.19 18:11, 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.
> 
> Cc: Michael Hennerich <michael.hennerich@analog.com>
> Cc: Alexander Aring <alex.aring@gmail.com>
> Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Harry Morris <h.morris@cascoda.com>
> Cc: linux-wpan@vger.kernel.org
> Cc: netdev@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  drivers/net/ieee802154/adf7242.c   | 12 +++---------
>  drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
>  drivers/net/ieee802154/ca8210.c    |  9 +--------
>  3 files changed, 9 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> index c9392d70e639..7b95add2235a 100644
> --- a/drivers/net/ieee802154/adf7242.c
> +++ b/drivers/net/ieee802154/adf7242.c
> @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
>  	return 0;
>  }
>  
> -static int adf7242_debugfs_init(struct adf7242_local *lp)
> +static void adf7242_debugfs_init(struct adf7242_local *lp)
>  {
>  	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
>  	struct dentry *stats;

A quick look over the code indicates that the stats variable can go as
well now as it is only used in the now removed code.

> @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
>  	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>  
>  	lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> -	if (IS_ERR_OR_NULL(lp->debugfs_root))
> -		return PTR_ERR_OR_ZERO(lp->debugfs_root);
>  
> -	stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> -					    lp->debugfs_root,
> -					    adf7242_stats_show);
> -	return PTR_ERR_OR_ZERO(stats);
> -
> -	return 0;
> +	debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> +				    adf7242_stats_show);
>  }
>  
>  static const s32 adf7242_powers[] = {
> diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> index 595cf7e2a651..7d67f41387f5 100644
> --- a/drivers/net/ieee802154/at86rf230.c
> +++ b/drivers/net/ieee802154/at86rf230.c
> @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
>  }
>  DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
>  
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
>  {
>  	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> -	struct dentry *stats;
>  
>  	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
>  
>  	at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> -	if (!at86rf230_debugfs_root)
> -		return -ENOMEM;
> -
> -	stats = debugfs_create_file("trac_stats", 0444,
> -				    at86rf230_debugfs_root, lp,
> -				    &at86rf230_stats_fops);
> -	if (!stats)
> -		return -ENOMEM;
>  
> -	return 0;
> +	debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> +			    &at86rf230_stats_fops);
>  }
>  
>  static void at86rf230_debugfs_remove(void)
> @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
>  	debugfs_remove_recursive(at86rf230_debugfs_root);
>  }
>  #else
> -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
>  static void at86rf230_debugfs_remove(void) { }
>  #endif
>  
> @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
>  	/* going into sleep by default */
>  	at86rf230_sleep(lp);
>  
> -	rc = at86rf230_debugfs_init(lp);
> -	if (rc)
> -		goto free_dev;
> +	at86rf230_debugfs_init(lp);
>  
>  	rc = ieee802154_register_hw(lp->hw);
>  	if (rc)
> diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> index b188fce3f641..11402dc347db 100644
> --- a/drivers/net/ieee802154/ca8210.c
> +++ b/drivers/net/ieee802154/ca8210.c
> @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
>  		priv,
>  		&test_int_fops
>  	);
> -	if (IS_ERR(test->ca8210_dfs_spi_int)) {
> -		dev_err(
> -			&priv->spi->dev,
> -			"Error %ld when creating debugfs node\n",
> -			PTR_ERR(test->ca8210_dfs_spi_int)
> -		);
> -		return PTR_ERR(test->ca8210_dfs_spi_int);
> -	}
> +
>  	debugfs_create_symlink("ca8210", NULL, node_name);
>  	init_waitqueue_head(&test->readq);
>  	return kfifo_alloc(
> 

With a fix for the above included you can have my


Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>

for version 2.

regards
Stefan Schmidt

^ permalink raw reply	[flat|nested] 3+ messages in thread
* Re: [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions
  2019-08-06 19:22   ` Stefan Schmidt
@ 2019-08-06 19:48     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 3+ messages in thread
From: Greg Kroah-Hartman @ 2019-08-06 19:48 UTC (permalink / raw)
  To: Stefan Schmidt
  Cc: netdev, Michael Hennerich, Alexander Aring, David S. Miller,
	Harry Morris, linux-wpan

On Tue, Aug 06, 2019 at 09:22:43PM +0200, Stefan Schmidt wrote:
> Hello.
> 
> On 06.08.19 18:11, 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.
> > 
> > Cc: Michael Hennerich <michael.hennerich@analog.com>
> > Cc: Alexander Aring <alex.aring@gmail.com>
> > Cc: Stefan Schmidt <stefan@datenfreihafen.org>
> > Cc: "David S. Miller" <davem@davemloft.net>
> > Cc: Harry Morris <h.morris@cascoda.com>
> > Cc: linux-wpan@vger.kernel.org
> > Cc: netdev@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  drivers/net/ieee802154/adf7242.c   | 12 +++---------
> >  drivers/net/ieee802154/at86rf230.c | 20 +++++---------------
> >  drivers/net/ieee802154/ca8210.c    |  9 +--------
> >  3 files changed, 9 insertions(+), 32 deletions(-)
> > 
> > diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c
> > index c9392d70e639..7b95add2235a 100644
> > --- a/drivers/net/ieee802154/adf7242.c
> > +++ b/drivers/net/ieee802154/adf7242.c
> > @@ -1158,7 +1158,7 @@ static int adf7242_stats_show(struct seq_file *file, void *offset)
> >  	return 0;
> >  }
> >  
> > -static int adf7242_debugfs_init(struct adf7242_local *lp)
> > +static void adf7242_debugfs_init(struct adf7242_local *lp)
> >  {
> >  	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "adf7242-";
> >  	struct dentry *stats;
> 
> A quick look over the code indicates that the stats variable can go as
> well now as it is only used in the now removed code.

Odd 0-day never gave me a build warning for it, sorry about that.

> 
> > @@ -1166,15 +1166,9 @@ static int adf7242_debugfs_init(struct adf7242_local *lp)
> >  	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >  
> >  	lp->debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > -	if (IS_ERR_OR_NULL(lp->debugfs_root))
> > -		return PTR_ERR_OR_ZERO(lp->debugfs_root);
> >  
> > -	stats = debugfs_create_devm_seqfile(&lp->spi->dev, "status",
> > -					    lp->debugfs_root,
> > -					    adf7242_stats_show);
> > -	return PTR_ERR_OR_ZERO(stats);
> > -
> > -	return 0;
> > +	debugfs_create_devm_seqfile(&lp->spi->dev, "status", lp->debugfs_root,
> > +				    adf7242_stats_show);
> >  }
> >  
> >  static const s32 adf7242_powers[] = {
> > diff --git a/drivers/net/ieee802154/at86rf230.c b/drivers/net/ieee802154/at86rf230.c
> > index 595cf7e2a651..7d67f41387f5 100644
> > --- a/drivers/net/ieee802154/at86rf230.c
> > +++ b/drivers/net/ieee802154/at86rf230.c
> > @@ -1626,24 +1626,16 @@ static int at86rf230_stats_show(struct seq_file *file, void *offset)
> >  }
> >  DEFINE_SHOW_ATTRIBUTE(at86rf230_stats);
> >  
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp)
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp)
> >  {
> >  	char debugfs_dir_name[DNAME_INLINE_LEN + 1] = "at86rf230-";
> > -	struct dentry *stats;
> >  
> >  	strncat(debugfs_dir_name, dev_name(&lp->spi->dev), DNAME_INLINE_LEN);
> >  
> >  	at86rf230_debugfs_root = debugfs_create_dir(debugfs_dir_name, NULL);
> > -	if (!at86rf230_debugfs_root)
> > -		return -ENOMEM;
> > -
> > -	stats = debugfs_create_file("trac_stats", 0444,
> > -				    at86rf230_debugfs_root, lp,
> > -				    &at86rf230_stats_fops);
> > -	if (!stats)
> > -		return -ENOMEM;
> >  
> > -	return 0;
> > +	debugfs_create_file("trac_stats", 0444, at86rf230_debugfs_root, lp,
> > +			    &at86rf230_stats_fops);
> >  }
> >  
> >  static void at86rf230_debugfs_remove(void)
> > @@ -1651,7 +1643,7 @@ static void at86rf230_debugfs_remove(void)
> >  	debugfs_remove_recursive(at86rf230_debugfs_root);
> >  }
> >  #else
> > -static int at86rf230_debugfs_init(struct at86rf230_local *lp) { return 0; }
> > +static void at86rf230_debugfs_init(struct at86rf230_local *lp) { }
> >  static void at86rf230_debugfs_remove(void) { }
> >  #endif
> >  
> > @@ -1751,9 +1743,7 @@ static int at86rf230_probe(struct spi_device *spi)
> >  	/* going into sleep by default */
> >  	at86rf230_sleep(lp);
> >  
> > -	rc = at86rf230_debugfs_init(lp);
> > -	if (rc)
> > -		goto free_dev;
> > +	at86rf230_debugfs_init(lp);
> >  
> >  	rc = ieee802154_register_hw(lp->hw);
> >  	if (rc)
> > diff --git a/drivers/net/ieee802154/ca8210.c b/drivers/net/ieee802154/ca8210.c
> > index b188fce3f641..11402dc347db 100644
> > --- a/drivers/net/ieee802154/ca8210.c
> > +++ b/drivers/net/ieee802154/ca8210.c
> > @@ -3019,14 +3019,7 @@ static int ca8210_test_interface_init(struct ca8210_priv *priv)
> >  		priv,
> >  		&test_int_fops
> >  	);
> > -	if (IS_ERR(test->ca8210_dfs_spi_int)) {
> > -		dev_err(
> > -			&priv->spi->dev,
> > -			"Error %ld when creating debugfs node\n",
> > -			PTR_ERR(test->ca8210_dfs_spi_int)
> > -		);
> > -		return PTR_ERR(test->ca8210_dfs_spi_int);
> > -	}
> > +
> >  	debugfs_create_symlink("ca8210", NULL, node_name);
> >  	init_waitqueue_head(&test->readq);
> >  	return kfifo_alloc(
> > 
> 
> With a fix for the above included you can have my
> 
> 
> Acked-by: Stefan Schmidt <stefan@datenfreihafen.org>
> 
> for version 2.

I'll fix it up and resend tomorrow, thanks!

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-08-06 19:48 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190806161128.31232-1-gregkh@linuxfoundation.org>
2019-08-06 16:11 ` [PATCH 17/17] ieee802154: no need to check return value of debugfs_create functions Greg Kroah-Hartman
2019-08-06 19:22   ` Stefan Schmidt
2019-08-06 19:48     ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).