[PATCH v1] drivers/net: marvell: mvpp2: fix debugfs_create_dir() return check in three functions

[PATCH v1] drivers/net: marvell: mvpp2: fix debugfs_create_dir() return check in three functions

[Minjie Du]

Make IS_ERR() judge the debugfs_create_dir() function return
in mvpp2_dbgfs_c2_entry_init(), mvpp2_dbgfs_flow_tbl_entry_init()
 and mvpp2_dbgfs_cls_init()
Line 562 deletes the space because checkpatch reports an error.

Signed-off-by: Minjie Du <duminjie@vivo.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 75e83ea2a..37bff3304 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -559,7 +559,7 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
 			    &mvpp2_dbgfs_prs_hits_fops);
 
 	debugfs_create_file("pmap", 0444, prs_entry_dir, entry,
-			     &mvpp2_dbgfs_prs_pmap_fops);
+			    &mvpp2_dbgfs_prs_pmap_fops);
 
 	return 0;
 }
@@ -593,7 +593,7 @@ static int mvpp2_dbgfs_c2_entry_init(struct dentry *parent,
 	sprintf(c2_entry_name, "%03d", id);
 
 	c2_entry_dir = debugfs_create_dir(c2_entry_name, parent);
-	if (!c2_entry_dir)
+	if (IS_ERR(c2_entry_dir))
 		return -ENOMEM;
 
 	entry = &priv->dbgfs_entries->c2_entries[id];
@@ -626,7 +626,7 @@ static int mvpp2_dbgfs_flow_tbl_entry_init(struct dentry *parent,
 	sprintf(flow_tbl_entry_name, "%03d", id);
 
 	flow_tbl_entry_dir = debugfs_create_dir(flow_tbl_entry_name, parent);
-	if (!flow_tbl_entry_dir)
+	if (IS_ERR(flow_tbl_entry_dir))
 		return -ENOMEM;
 
 	entry = &priv->dbgfs_entries->flt_entries[id];
@@ -646,11 +646,11 @@ static int mvpp2_dbgfs_cls_init(struct dentry *parent, struct mvpp2 *priv)
 	int i, ret;
 
 	cls_dir = debugfs_create_dir("classifier", parent);
-	if (!cls_dir)
+	if (IS_ERR(cls_dir))
 		return -ENOMEM;
 
 	c2_dir = debugfs_create_dir("c2", cls_dir);
-	if (!c2_dir)
+	if (IS_ERR(c2_dir))
 		return -ENOMEM;
 
 	for (i = 0; i < MVPP22_CLS_C2_N_ENTRIES; i++) {
@@ -660,7 +660,7 @@ static int mvpp2_dbgfs_cls_init(struct dentry *parent, struct mvpp2 *priv)
 	}
 
 	flow_tbl_dir = debugfs_create_dir("flow_table", cls_dir);
-	if (!flow_tbl_dir)
+	if (IS_ERR(flow_tbl_dir))
 		return -ENOMEM;
 
 	for (i = 0; i < MVPP2_CLS_FLOWS_TBL_SIZE; i++) {
-- 
2.39.0

Re: [PATCH] drivers/net: marvell: mvpp2: fix debugfs_create_dir() return check in three functions

[Markus Elfring]

> Make IS_ERR() judge the debugfs_create_dir() function return

Would the term “return value” be relevant here?

Would you like to improve this change description also according to
review comments from other patches?


How do you think about to add the tag “Fixes” because of corrected error detections?


Would a subject like “[PATCH v2] net: mvpp2: debugfs: Fix error checks in three functions”
be more appropriate?


> Line 562 deletes the space because checkpatch reports an error.

Please put this adjustment into a separate update step
(also with a better wording).

See also:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v6.5-rc1#n81

Regards,
Markus

Re: [PATCH v1] drivers/net: marvell: mvpp2: fix debugfs_create_dir() return check in three functions

[Simon Horman]

On Thu, Jul 13, 2023 at 12:45:07PM +0800, Minjie Du wrote:
> Make IS_ERR() judge the debugfs_create_dir() function return
> in mvpp2_dbgfs_c2_entry_init(), mvpp2_dbgfs_flow_tbl_entry_init()
>  and mvpp2_dbgfs_cls_init()
> Line 562 deletes the space because checkpatch reports an error.
> 
> Signed-off-by: Minjie Du <duminjie@vivo.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> index 75e83ea2a..37bff3304 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> @@ -559,7 +559,7 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
>  			    &mvpp2_dbgfs_prs_hits_fops);
>  
>  	debugfs_create_file("pmap", 0444, prs_entry_dir, entry,
> -			     &mvpp2_dbgfs_prs_pmap_fops);
> +			    &mvpp2_dbgfs_prs_pmap_fops);

Unfortunately net-next does not accept patches for Checkpatch warnings
of this nature.

>  
>  	return 0;
>  }
> @@ -593,7 +593,7 @@ static int mvpp2_dbgfs_c2_entry_init(struct dentry *parent,
>  	sprintf(c2_entry_name, "%03d", id);
>  
>  	c2_entry_dir = debugfs_create_dir(c2_entry_name, parent);
> -	if (!c2_entry_dir)
> +	if (IS_ERR(c2_entry_dir))
>  		return -ENOMEM;

As per the comment above debugfs_create_dir(), it is not
expected to return an error, and in general callers should
ignore the return value.

I expect a good approach here is to simply remove the error handling
for debugfs_create_dir(). e.g.

-	if (!c2_entry_dir)
-		return -ENOMEM;

-- 
pw-bot: changes-requested