[PATCH net-next v2] netdevsim: remove return value check of debugfs_create_dir

[PATCH net-next v2] netdevsim: remove return value check of debugfs_create_dir

[Prashant Bhole]

Reason:
Discussion started about adding error check on return value where
it was not handled. Also handling the error using IS_ERR_OR_NULL
instead of IS_ERR(), because debugfs_create_dir() doesn't return
error. It returns NULL or a valid pointer when DebugFS is enabled.

Finally it was decided to remove error handling altogether to
make it consistent and removal of this check isn't fatal to the driver.

Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
---
 drivers/net/netdevsim/bpf.c    | 5 -----
 drivers/net/netdevsim/netdev.c | 2 --
 2 files changed, 7 deletions(-)

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 078d2c37a6c1..03c09e8b99bd 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -214,11 +214,6 @@ static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
 	/* Program id is not populated yet when we create the state. */
 	sprintf(name, "%u", ns->prog_id_gen++);
 	state->ddir = debugfs_create_dir(name, ns->ddir_bpf_bound_progs);
-	if (IS_ERR(state->ddir)) {
-		err = PTR_ERR(state->ddir);
-		kfree(state);
-		return err;
-	}
 
 	debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id);
 	debugfs_create_file("state", 0400, state->ddir,
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

Re: [PATCH net-next v2] netdevsim: remove return value check of debugfs_create_dir

[Jakub Kicinski]

On Fri,  8 Dec 2017 14:32:18 +0900, Prashant Bhole wrote:
> Reason:
> Discussion started about adding error check on return value where
> it was not handled. Also handling the error using IS_ERR_OR_NULL
> instead of IS_ERR(), because debugfs_create_dir() doesn't return
> error. It returns NULL or a valid pointer when DebugFS is enabled.
> 
> Finally it was decided to remove error handling altogether to
> make it consistent and removal of this check isn't fatal to the driver.
> 
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>

Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>

Thank you!

Re: [PATCH net-next v2] netdevsim: remove return value check of debugfs_create_dir

[David Miller]

From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Date: Fri,  8 Dec 2017 14:32:18 +0900

> Reason:
> Discussion started about adding error check on return value where
> it was not handled. Also handling the error using IS_ERR_OR_NULL
> instead of IS_ERR(), because debugfs_create_dir() doesn't return
> error. It returns NULL or a valid pointer when DebugFS is enabled.
> 
> Finally it was decided to remove error handling altogether to
> make it consistent and removal of this check isn't fatal to the driver.
> 
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>

I'm not apply this.

If it fails with NULL at init time the will potentially place
all the netdevsim device files in the ROOT of debugfs which
is strongly undesriable.

Check the errors, fix the debugfs interface error return semantics,
and make errors fatal for module load.

Thank you.