crypto: hisilicon - still no need to check return value of debugfs_create functions

PATCH In review Greg Kroah-Hartman 4 participants 4 messages Dec 9, 2019 – Dec 20, 2019

Subsystems (2)

Crypto API HiSilicon high performance rsa engine driver (hpre)

lore

Share & tools

lore

Greg Kroah-Hartman

author

2019-12-09 15:21 UTC #0 lore raw

PATCH 1/1 crypto: hisilicon - still no need to check return value of debugfs_create functions

Just like in 4a97bfc79619 ("crypto: hisilicon - no need to check return value of debugfs_create functions"), there still 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 Zhou Wang <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 28 +++++------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

drivers/crypto/hisilicon/hpre/hpre_main.c +6 ~~-22~~

index 34e0424410bf..711f5d18b641 100644
@@ -557,7 +557,7 @@ static const struct file_operations hpre_ctrl_debug_fops = {
 static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
 				    enum hpre_ctrl_dbgfs_file type, int indx)
 {
-	struct dentry *tmp, *file_dir;
+	struct dentry *file_dir;
 
 	if (dir)
 		file_dir = dir;

@@ -571,10 +571,8 @@ static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
 	dbg->files[indx].debug = dbg;
 	dbg->files[indx].type = type;
 	dbg->files[indx].index = indx;
-	tmp = debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
-				  dbg->files + indx, &hpre_ctrl_debug_fops);
-	if (!tmp)
-		return -ENOENT;
+	debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
+			    dbg->files + indx, &hpre_ctrl_debug_fops);
 
 	return 0;
 }

@@ -585,7 +583,6 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
 	struct hisi_qm *qm = &hpre->qm;
 	struct device *dev = &qm->pdev->dev;
 	struct debugfs_regset32 *regset;
-	struct dentry *tmp;
 
 	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
 	if (!regset)

@@ -595,10 +592,7 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
 	regset->nregs = ARRAY_SIZE(hpre_com_dfx_regs);
 	regset->base = qm->io_base;
 
-	tmp = debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
-	if (!tmp)
-		return -ENOENT;
-
+	debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
 	return 0;
 }

@@ -609,15 +603,12 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
 	struct device *dev = &qm->pdev->dev;
 	char buf[HPRE_DBGFS_VAL_MAX_LEN];
 	struct debugfs_regset32 *regset;
-	struct dentry *tmp_d, *tmp;
+	struct dentry *tmp_d;
 	int i, ret;
 
 	for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
 		sprintf(buf, "cluster%d", i);
-
 		tmp_d = debugfs_create_dir(buf, debug->debug_root);
-		if (!tmp_d)
-			return -ENOENT;
 
 		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
 		if (!regset)

@@ -627,9 +618,7 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
 		regset->nregs = ARRAY_SIZE(hpre_cluster_dfx_regs);
 		regset->base = qm->io_base + hpre_cluster_offsets[i];
 
-		tmp = debugfs_create_regset32("regs", 0444, tmp_d, regset);
-		if (!tmp)
-			return -ENOENT;
+		debugfs_create_regset32("regs", 0444, tmp_d, regset);
 		ret = hpre_create_debugfs_file(debug, tmp_d, HPRE_CLUSTER_CTRL,
 					       i + HPRE_CLUSTER_CTRL);
 		if (ret)

@@ -668,9 +657,6 @@ static int hpre_debugfs_init(struct hpre *hpre)
 	int ret;
 
 	dir = debugfs_create_dir(dev_name(dev), hpre_debugfs_root);
-	if (!dir)
-		return -ENOENT;
-
 	qm->debug.debug_root = dir;
 
 	ret = hisi_qm_debug_init(qm);

@@ -1014,8 +1000,6 @@ static void hpre_register_debugfs(void)
 		return;
 
 	hpre_debugfs_root = debugfs_create_dir(hpre_name, NULL);
-	if (IS_ERR_OR_NULL(hpre_debugfs_root))
-		hpre_debugfs_root = NULL;
 }
 
 static void hpre_unregister_debugfs(void)

Signature · 1 line

2.24.0

Zhou Wang

2019-12-10 04:05 UTC #1 lore raw

Quoted 4 lines · Greg Kroah-Hartman

Just like in 4a97bfc79619 ("crypto: hisilicon - no need to check return value of debugfs_create functions"), there still 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.

Thanks for fix this, +To Zaibo Xu who is the maintainer of this module :)

Best,
Zhou

Quoted 105 lines

Cc Zhou Wang <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 28 +++++------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

drivers/crypto/hisilicon/hpre/hpre_main.c +6 ~~-22~~

index 34e0424410bf..711f5d18b641 100644
@@ -557,7 +557,7 @@ static const struct file_operations hpre_ctrl_debug_fops = {
 static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
 				    enum hpre_ctrl_dbgfs_file type, int indx)
 {
-	struct dentry *tmp, *file_dir;
+	struct dentry *file_dir;
 
 	if (dir)
 		file_dir = dir;

@@ -571,10 +571,8 @@ static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
 	dbg->files[indx].debug = dbg;
 	dbg->files[indx].type = type;
 	dbg->files[indx].index = indx;
-	tmp = debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
-				  dbg->files + indx, &hpre_ctrl_debug_fops);
-	if (!tmp)
-		return -ENOENT;
+	debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
+			    dbg->files + indx, &hpre_ctrl_debug_fops);
 
 	return 0;
 }

@@ -585,7 +583,6 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
 	struct hisi_qm *qm = &hpre->qm;
 	struct device *dev = &qm->pdev->dev;
 	struct debugfs_regset32 *regset;
-	struct dentry *tmp;
 
 	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
 	if (!regset)

@@ -595,10 +592,7 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
 	regset->nregs = ARRAY_SIZE(hpre_com_dfx_regs);
 	regset->base = qm->io_base;
 
-	tmp = debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
-	if (!tmp)
-		return -ENOENT;
-
+	debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
 	return 0;
 }

@@ -609,15 +603,12 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
 	struct device *dev = &qm->pdev->dev;
 	char buf[HPRE_DBGFS_VAL_MAX_LEN];
 	struct debugfs_regset32 *regset;
-	struct dentry *tmp_d, *tmp;
+	struct dentry *tmp_d;
 	int i, ret;
 
 	for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
 		sprintf(buf, "cluster%d", i);
-
 		tmp_d = debugfs_create_dir(buf, debug->debug_root);
-		if (!tmp_d)
-			return -ENOENT;
 
 		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
 		if (!regset)

@@ -627,9 +618,7 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
 		regset->nregs = ARRAY_SIZE(hpre_cluster_dfx_regs);
 		regset->base = qm->io_base + hpre_cluster_offsets[i];
 
-		tmp = debugfs_create_regset32("regs", 0444, tmp_d, regset);
-		if (!tmp)
-			return -ENOENT;
+		debugfs_create_regset32("regs", 0444, tmp_d, regset);
 		ret = hpre_create_debugfs_file(debug, tmp_d, HPRE_CLUSTER_CTRL,
 					       i + HPRE_CLUSTER_CTRL);
 		if (ret)

@@ -668,9 +657,6 @@ static int hpre_debugfs_init(struct hpre *hpre)
 	int ret;
 
 	dir = debugfs_create_dir(dev_name(dev), hpre_debugfs_root);
-	if (!dir)
-		return -ENOENT;
-
 	qm->debug.debug_root = dir;
 
 	ret = hisi_qm_debug_init(qm);

@@ -1014,8 +1000,6 @@ static void hpre_register_debugfs(void)
 		return;
 
 	hpre_debugfs_root = debugfs_create_dir(hpre_name, NULL);
-	if (IS_ERR_OR_NULL(hpre_debugfs_root))
-		hpre_debugfs_root = NULL;
 }
 
 static void hpre_unregister_debugfs(void)

Xu Zaibo

2019-12-10 06:19 UTC #2 lore raw

Okay, I will fix this, thanks.

Cheers,

Zaibo

Quoted 116 lines · Zhou Wang

Quoted 4 lines · Greg Kroah-Hartman

Just like in 4a97bfc79619 ("crypto: hisilicon - no need to check return value of debugfs_create functions"), there still 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.

Thanks for fix this, +To Zaibo Xu who is the maintainer of this module :)

Best,
Zhou

Quoted 96 lines

Cc Zhou Wang <[email protected]>

Cc Herbert Xu <[email protected]>

Cc "David S. Miller" <[email protected]>

Cc [email protected]

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
  drivers/crypto/hisilicon/hpre/hpre_main.c | 28 +++++------------------
  1 file changed, 6 insertions(+), 22 deletions(-)

drivers/crypto/hisilicon/hpre/hpre_main.c +6 ~~-20~~

index 34e0424410bf..711f5d18b641 100644
@@ -557,7 +557,7 @@ static const struct file_operations hpre_ctrl_debug_fops = {
  static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
  				    enum hpre_ctrl_dbgfs_file type, int indx)
  {
-	struct dentry *tmp, *file_dir;
+	struct dentry *file_dir;
 
  	if (dir)
  		file_dir = dir;

@@ -571,10 +571,8 @@ static int hpre_create_debugfs_file(struct hpre_debug *dbg, struct dentry *dir,
  	dbg->files[indx].debug = dbg;
  	dbg->files[indx].type = type;
  	dbg->files[indx].index = indx;
-	tmp = debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
-				  dbg->files + indx, &hpre_ctrl_debug_fops);
-	if (!tmp)
-		return -ENOENT;
+	debugfs_create_file(hpre_debug_file_name[type], 0600, file_dir,
+			    dbg->files + indx, &hpre_ctrl_debug_fops);
 
  	return 0;
  }

@@ -585,7 +583,6 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
  	struct hisi_qm *qm = &hpre->qm;
  	struct device *dev = &qm->pdev->dev;
  	struct debugfs_regset32 *regset;
-	struct dentry *tmp;
 
  	regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
  	if (!regset)

@@ -595,10 +592,7 @@ static int hpre_pf_comm_regs_debugfs_init(struct hpre_debug *debug)
  	regset->nregs = ARRAY_SIZE(hpre_com_dfx_regs);
  	regset->base = qm->io_base;
 
-	tmp = debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
-	if (!tmp)
-		return -ENOENT;
-
+	debugfs_create_regset32("regs", 0444,  debug->debug_root, regset);
  	return 0;
  }

@@ -609,15 +603,12 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
  	struct device *dev = &qm->pdev->dev;
  	char buf[HPRE_DBGFS_VAL_MAX_LEN];
  	struct debugfs_regset32 *regset;
-	struct dentry *tmp_d, *tmp;
+	struct dentry *tmp_d;
  	int i, ret;
 
  	for (i = 0; i < HPRE_CLUSTERS_NUM; i++) {
  		sprintf(buf, "cluster%d", i);
-
  		tmp_d = debugfs_create_dir(buf, debug->debug_root);
-		if (!tmp_d)
-			return -ENOENT;
 
  		regset = devm_kzalloc(dev, sizeof(*regset), GFP_KERNEL);
  		if (!regset)

@@ -627,9 +618,7 @@ static int hpre_cluster_debugfs_init(struct hpre_debug *debug)
  		regset->nregs = ARRAY_SIZE(hpre_cluster_dfx_regs);
  		regset->base = qm->io_base + hpre_cluster_offsets[i];
 
-		tmp = debugfs_create_regset32("regs", 0444, tmp_d, regset);
-		if (!tmp)
-			return -ENOENT;
+		debugfs_create_regset32("regs", 0444, tmp_d, regset);
  		ret = hpre_create_debugfs_file(debug, tmp_d, HPRE_CLUSTER_CTRL,
  					       i + HPRE_CLUSTER_CTRL);
  		if (ret)

@@ -668,9 +657,6 @@ static int hpre_debugfs_init(struct hpre *hpre)
  	int ret;
 
  	dir = debugfs_create_dir(dev_name(dev), hpre_debugfs_root);
-	if (!dir)
-		return -ENOENT;
-
  	qm->debug.debug_root = dir;
 
  	ret = hisi_qm_debug_init(qm);

drivers/crypto/hisilicon/hpre/hpre_main.c static void hpre_register_debugfs(void) · line 1005

Quoted 8 lines

drivers/crypto/hisilicon/hpre/hpre_main.c +0 -2

@@ -1014,8 +1000,6 @@ static void hpre_register_debugfs(void)
  		return;
 
  	hpre_debugfs_root = debugfs_create_dir(hpre_name, NULL);
-	if (IS_ERR_OR_NULL(hpre_debugfs_root))
-		hpre_debugfs_root = NULL;
  }
 
  static void hpre_unregister_debugfs(void)

Herbert Xu

maintainer

2019-12-20 07:05 UTC #3 lore raw

In reply to Greg Kroah-Hartman

Quoted 11 lines · Greg Kroah-Hartman

Just like in 4a97bfc79619 ("crypto: hisilicon - no need to check return value of debugfs_create functions"), there still 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 Zhou Wang <[email protected]>
Cc Herbert Xu <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc [email protected]
Cc [email protected]
Signed-off-by Greg Kroah-Hartman <[email protected]>

Quoted 3 lines

---
 drivers/crypto/hisilicon/hpre/hpre_main.c | 28 +++++------------------
 1 file changed, 6 insertions(+), 22 deletions(-)

Patch applied. Thanks.

Signature · 3 lines

Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt