diff mbox series

qlcnic: Use common error handling code in four functions

Message ID 7c98349a-bfa5-409b-847e-ed8439e80afd@web.de (mailing list archive)
State Rejected
Delegated to: Netdev Maintainers
Headers show
Series qlcnic: Use common error handling code in four functions | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 16 this patch: 16
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers success CCed 7 of 7 maintainers
netdev/build_clang success Errors and warnings before: 16 this patch: 16
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 16 this patch: 16
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 182 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 28 this patch: 28
netdev/source_inline success Was 0 now: 0
netdev/contest success net-next-2024-09-20--15-00 (tests: 764)

Commit Message

Markus Elfring Sept. 19, 2024, 7:50 p.m. UTC 
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 19 Sep 2024 21:30:45 +0200

Add jump targets so that a bit of exception handling can be better reused
at the end of four function implementations.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 .../ethernet/qlogic/qlcnic/qlcnic_83xx_init.c | 12 +--
 .../net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c | 86 ++++++++-----------
 2 files changed, 42 insertions(+), 56 deletions(-)

--
2.46.0

Comments

Simon Horman Sept. 22, 2024, 4:56 p.m. UTC | #1 
On Thu, Sep 19, 2024 at 09:50:13PM +0200, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 19 Sep 2024 21:30:45 +0200
> 
> Add jump targets so that a bit of exception handling can be better reused
> at the end of four function implementations.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Hi Markus,

This is an old driver, that doesn't appear to have been under active
development for quite some time. Unless there is a way to exercise these
changes I don't think that clean-ups of this nature are worth the risk of
regressions they might introduce.

If we can see bugs in error paths, let's fix them.
Else, let's leave them be.
diff mbox series

Patch

diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
index b733374b4dc5..a8eaf10d9158 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_83xx_init.c
@@ -1333,17 +1333,13 @@  static int qlcnic_83xx_copy_bootloader(struct qlcnic_adapter *adapter)

 	ret = qlcnic_83xx_lockless_flash_read32(adapter, src, p_cache,
 						size / sizeof(u32));
-	if (ret) {
-		vfree(p_cache);
-		return ret;
-	}
+	if (ret)
+		goto free_cache;
+
 	/* 16 byte write to MS memory */
 	ret = qlcnic_ms_mem_write128(adapter, dest, (u32 *)p_cache,
 				     size / 16);
-	if (ret) {
-		vfree(p_cache);
-		return ret;
-	}
+free_cache:
 	vfree(p_cache);

 	return ret;
diff --git a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
index 74125188beb8..da1a6e68daf9 100644
--- a/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
+++ b/drivers/net/ethernet/qlogic/qlcnic/qlcnic_sysfs.c
@@ -959,8 +959,8 @@  static ssize_t qlcnic_83xx_sysfs_flash_read_handler(struct file *filp,
 	if (!p_read_buf)
 		return -ENOMEM;
 	if (qlcnic_83xx_lock_flash(adapter) != 0) {
-		kfree(p_read_buf);
-		return -EIO;
+		ret = -EIO;
+		goto free_read_buf;
 	}

 	ret = qlcnic_83xx_lockless_flash_read32(adapter, offset, p_read_buf,
@@ -968,8 +968,7 @@  static ssize_t qlcnic_83xx_sysfs_flash_read_handler(struct file *filp,

 	if (ret) {
 		qlcnic_83xx_unlock_flash(adapter);
-		kfree(p_read_buf);
-		return ret;
+		goto free_read_buf;
 	}

 	qlcnic_83xx_unlock_flash(adapter);
@@ -978,6 +977,10 @@  static ssize_t qlcnic_83xx_sysfs_flash_read_handler(struct file *filp,
 	kfree(p_read_buf);

 	return size;
+
+free_read_buf:
+	kfree(p_read_buf);
+	return ret;
 }

 static int qlcnic_83xx_sysfs_flash_bulk_write(struct qlcnic_adapter *adapter,
@@ -996,18 +999,13 @@  static int qlcnic_83xx_sysfs_flash_bulk_write(struct qlcnic_adapter *adapter,
 	memcpy(p_cache, buf, size);
 	p_src = p_cache;

-	if (qlcnic_83xx_lock_flash(adapter) != 0) {
-		kfree(p_cache);
-		return -EIO;
-	}
+	if (qlcnic_83xx_lock_flash(adapter))
+		goto free_cache;

 	if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 		ret = qlcnic_83xx_enable_flash_write(adapter);
-		if (ret) {
-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
-		}
+		if (ret)
+			goto unlock_adapter;
 	}

 	for (i = 0; i < count / QLC_83XX_FLASH_WRITE_MAX; i++) {
@@ -1018,16 +1016,11 @@  static int qlcnic_83xx_sysfs_flash_bulk_write(struct qlcnic_adapter *adapter,
 		if (ret) {
 			if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 				ret = qlcnic_83xx_disable_flash_write(adapter);
-				if (ret) {
-					kfree(p_cache);
-					qlcnic_83xx_unlock_flash(adapter);
-					return -EIO;
-				}
+				if (ret)
+					goto unlock_adapter;
 			}

-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
+			goto unlock_adapter;
 		}

 		p_src = p_src + sizeof(u32)*QLC_83XX_FLASH_WRITE_MAX;
@@ -1036,17 +1029,20 @@  static int qlcnic_83xx_sysfs_flash_bulk_write(struct qlcnic_adapter *adapter,

 	if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 		ret = qlcnic_83xx_disable_flash_write(adapter);
-		if (ret) {
-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
-		}
+		if (ret)
+			goto unlock_adapter;
 	}

 	kfree(p_cache);
 	qlcnic_83xx_unlock_flash(adapter);

 	return 0;
+
+unlock_adapter:
+	qlcnic_83xx_unlock_flash(adapter);
+free_cache:
+	kfree(p_cache);
+	return -EIO;
 }

 static int qlcnic_83xx_sysfs_flash_write(struct qlcnic_adapter *adapter,
@@ -1064,18 +1060,13 @@  static int qlcnic_83xx_sysfs_flash_write(struct qlcnic_adapter *adapter,
 	p_src = p_cache;
 	count = size / sizeof(u32);

-	if (qlcnic_83xx_lock_flash(adapter) != 0) {
-		kfree(p_cache);
-		return -EIO;
-	}
+	if (qlcnic_83xx_lock_flash(adapter))
+		goto free_cache;

 	if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 		ret = qlcnic_83xx_enable_flash_write(adapter);
-		if (ret) {
-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
-		}
+		if (ret)
+			goto unlock_adapter;
 	}

 	for (i = 0; i < count; i++) {
@@ -1083,15 +1074,11 @@  static int qlcnic_83xx_sysfs_flash_write(struct qlcnic_adapter *adapter,
 		if (ret) {
 			if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 				ret = qlcnic_83xx_disable_flash_write(adapter);
-				if (ret) {
-					kfree(p_cache);
-					qlcnic_83xx_unlock_flash(adapter);
-					return -EIO;
-				}
+				if (ret)
+					goto unlock_adapter;
 			}
-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
+
+			goto unlock_adapter;
 		}

 		p_src = p_src + sizeof(u32);
@@ -1100,17 +1087,20 @@  static int qlcnic_83xx_sysfs_flash_write(struct qlcnic_adapter *adapter,

 	if (adapter->ahw->fdt.mfg_id == adapter->flash_mfg_id) {
 		ret = qlcnic_83xx_disable_flash_write(adapter);
-		if (ret) {
-			kfree(p_cache);
-			qlcnic_83xx_unlock_flash(adapter);
-			return -EIO;
-		}
+		if (ret)
+			goto unlock_adapter;
 	}

 	kfree(p_cache);
 	qlcnic_83xx_unlock_flash(adapter);

 	return 0;
+
+unlock_adapter:
+	qlcnic_83xx_unlock_flash(adapter);
+free_cache:
+	kfree(p_cache);
+	return -EIO;
 }

 static ssize_t qlcnic_83xx_sysfs_flash_write_handler(struct file *filp,