netdevsim: remove check on return value of debugfs_create_dir

Started by Prashant Bhole on Dec 8, 2017 · 3 messages from 3 authors · Last reply Dec 8, 2017

Subsystems (3)

Maintainers (6)

Show 2 more
STALE3252d

open the first message on its own page

  1. [PATCH net-next] netdevsim: remove check on return value of debugfs_create_dir
    Related patches
Message details and related content
Initial discussion started about correct handling of this condition.
Later it was decided to remove this check altogether to make it
consistent.

Removal of this check isn't fatal to this driver.

Signed-off-by: Prashant Bhole <redacted>
---
 drivers/net/netdevsim/netdev.c | 2 --
 1 file changed, 2 deletions(-)
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index eb8c679fca9f..6d4b35f26ae5 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -469,8 +469,6 @@ static int __init nsim_module_init(void)
 	int err;
 
 	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
-	if (IS_ERR(nsim_ddir))
-		return PTR_ERR(nsim_ddir);
 
 	err = bus_register(&nsim_bus);
 	if (err)
-- 
2.13.6
Message details and related content
On Fri,  8 Dec 2017 11:14:56 +0900, Prashant Bhole wrote:
Initial discussion started about correct handling of this condition.
Later it was decided to remove this check altogether to make it
consistent.

Removal of this check isn't fatal to this driver.

Signed-off-by: Prashant Bhole <redacted>
Acked-by: Jakub Kicinski <redacted>

but as I mentioned there is another, similar check in netdevsim/bpf.c:
"... drop this error handling here and in nsim_bpf_create_prog()."
quoted hunk
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index eb8c679fca9f..6d4b35f26ae5 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -469,8 +469,6 @@ static int __init nsim_module_init(void)
 	int err;
 
 	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
-	if (IS_ERR(nsim_ddir))
-		return PTR_ERR(nsim_ddir);
 
 	err = bus_register(&nsim_bus);
 	if (err)
Message details and related content
From: Prashant Bhole <redacted>
Date: Fri,  8 Dec 2017 11:14:56 +0900
Initial discussion started about correct handling of this condition.
Later it was decided to remove this check altogether to make it
consistent.

Removal of this check isn't fatal to this driver.
...
quoted hunk
@@ -469,8 +469,6 @@ static int __init nsim_module_init(void)
 	int err;
 
 	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
-	if (IS_ERR(nsim_ddir))
-		return PTR_ERR(nsim_ddir);
 
 	err = bus_register(&nsim_bus);
Please stop this madness.

You cannot continue if this thing returns NULL.

WHY?

Because later if you pass NULL to debugfs_create_dir() in nsim_init() do
you have any idea what it is going to do?

It's going to put the netdevsim device files into the root!

Please:

1) Handle the errors

2) Make them fatal, if DEBUGFS is enabled this should never fail except
   for memory allocation failures and we have bigger problems than
   successfully loading the netdevsim driver

3) Fix debugfs_create_dir() to have sane return value semantics so that
   people do not have to check both NULL and error pointers

Thank you.
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help