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

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

[Prashant Bhole]

- Checking return value with IS_ERROR_OR_NULL
- Added error handling where it was not handled

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

diff --git a/drivers/net/netdevsim/bpf.c b/drivers/net/netdevsim/bpf.c
index 078d2c37a6c1..aeb429428cc5 100644
--- a/drivers/net/netdevsim/bpf.c
+++ b/drivers/net/netdevsim/bpf.c
@@ -201,7 +201,6 @@ static int nsim_bpf_create_prog(struct netdevsim *ns, struct bpf_prog *prog)
 {
 	struct nsim_bpf_bound_prog *state;
 	char name[16];
-	int err;
 
 	state = kzalloc(sizeof(*state), GFP_KERNEL);
 	if (!state)
@@ -214,10 +213,9 @@ 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);
+	if (IS_ERR_OR_NULL(state->ddir)) {
 		kfree(state);
-		return err;
+		return -ENOMEM;
 	}
 
 	debugfs_create_u32("id", 0400, state->ddir, &prog->aux->id);
@@ -349,6 +347,8 @@ int nsim_bpf_init(struct netdevsim *ns)
 			   &ns->bpf_bind_verifier_delay);
 	ns->ddir_bpf_bound_progs =
 		debugfs_create_dir("bpf_bound_progs", ns->ddir);
+	if (IS_ERR_OR_NULL(ns->ddir_bpf_bound_progs))
+		return -ENOMEM;
 
 	ns->bpf_tc_accept = true;
 	debugfs_create_bool("bpf_tc_accept", 0600, ns->ddir,
diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index eb8c679fca9f..c2a02d1944b8 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -147,10 +147,12 @@ struct device_type nsim_dev_type = {
 static int nsim_init(struct net_device *dev)
 {
 	struct netdevsim *ns = netdev_priv(dev);
-	int err;
+	int err = -ENOMEM;
 
 	ns->netdev = dev;
 	ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
+	if (IS_ERR_OR_NULL(ns->ddir))
+		goto err;
 
 	err = nsim_bpf_init(ns);
 	if (err)
@@ -171,6 +173,7 @@ static int nsim_init(struct net_device *dev)
 	nsim_bpf_uninit(ns);
 err_debugfs_destroy:
 	debugfs_remove_recursive(ns->ddir);
+err:
 	return err;
 }
 
@@ -466,11 +469,11 @@ struct dentry *nsim_ddir;
 
 static int __init nsim_module_init(void)
 {
-	int err;
+	int err = -ENOMEM;
 
 	nsim_ddir = debugfs_create_dir(DRV_NAME, NULL);
-	if (IS_ERR(nsim_ddir))
-		return PTR_ERR(nsim_ddir);
+	if (IS_ERR_OR_NULL(nsim_ddir))
+		goto err;
 
 	err = bus_register(&nsim_bus);
 	if (err)
@@ -486,6 +489,7 @@ static int __init nsim_module_init(void)
 	bus_unregister(&nsim_bus);
 err_debugfs_destroy:
 	debugfs_remove_recursive(nsim_ddir);
+err:
 	return err;
 }
 
-- 
2.13.6

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

[Jakub Kicinski]

On Wed, 20 Dec 2017 11:27:15 +0900, Prashant Bhole wrote:
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index eb8c679fca9f..c2a02d1944b8 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -147,10 +147,12 @@ struct device_type nsim_dev_type = {
>  static int nsim_init(struct net_device *dev)
>  {
>  	struct netdevsim *ns = netdev_priv(dev);
> -	int err;
> +	int err = -ENOMEM;
>  
>  	ns->netdev = dev;
>  	ns->ddir = debugfs_create_dir(netdev_name(dev), nsim_ddir);
> +	if (IS_ERR_OR_NULL(ns->ddir))
> +		goto err;

nit:
Could you return err; here directly instead of go(ing )to return
and having label and variable of the same name?  Same in
nsim_module_init().

With that feel free to add:

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

Thanks!

>  	err = nsim_bpf_init(ns);
>  	if (err)
> @@ -171,6 +173,7 @@ static int nsim_init(struct net_device *dev)
>  	nsim_bpf_uninit(ns);
>  err_debugfs_destroy:
>  	debugfs_remove_recursive(ns->ddir);
> +err:
>  	return err;
>  }
>