[PATCH 0/4] staging: ks7010: refactor ks7010_sdio_remove()

[PATCH 0/4] staging: ks7010: refactor ks7010_sdio_remove()

[Tobin C. Harding]

Function is a candidate for refactoring. None of the kernel static
analysis tools warn about this function but the function has a number
of code smells. There is a block of code separated into a anonymous
compound statement, this is not usual kernel code style and can easily
be factored into a separate function. There are a number of function
calls that could be formatted better to make the code more
readable. Finally there is extraneous whitespace that can be remove,
also mildly improving readability.

Patch 01 reduces the level of indentation.

Patch 02 factors out compound statement.

Patch 03 removes extraneous newlines.

Code is untested. Builds on x86_64 and PowerPC.

Tobin C. Harding (3):
  staging: ks7010: invert conditional, reduce indentation
  staging: ks7010: factor out send stop request
  staging: ks7010: remove extraneous newlines.

 drivers/staging/ks7010/ks7010_sdio.c | 108 ++++++++++++++++++-----------------
 1 file changed, 57 insertions(+), 51 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/3] staging: ks7010: invert conditional, reduce indentation

[Tobin C. Harding]

Function contains a large block of code that is guarded by an if
statement. There is some remaining code after block that does final
clean up of driver. If we add a goto label labeling the final cleanup
code we can invert the if statement and jump to goto label. This
allows the subsequent code to be indented one level less while not
changing the program logic.

Add goto label. Invert if statement conditional, jump to label if new
conditional evaluates to true. Reduce indentation of subsequent code
by one level. Do not change the program logic.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 91 +++++++++++++++++++-----------------
 1 file changed, 47 insertions(+), 44 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index b16618b..0f5ff68 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1067,6 +1067,7 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 	int ret;
 	struct ks_sdio_card *card;
 	struct ks_wlan_private *priv;
+	struct net_device *netdev;
 
 	DPRINTK(1, "ks7010_sdio_remove()\n");
 
@@ -1077,60 +1078,62 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 
 	DPRINTK(1, "priv = card->priv\n");
 	priv = card->priv;
-	if (priv) {
-		struct net_device *netdev = priv->net_dev;
+	if (!priv)
+		goto out;
 
-		ks_wlan_net_stop(netdev);
-		DPRINTK(1, "ks_wlan_net_stop\n");
+	netdev = priv->net_dev;
 
-		/* interrupt disable */
-		sdio_claim_host(func);
-		sdio_writeb(func, 0, INT_ENABLE, &ret);
-		sdio_writeb(func, 0xff, INT_PENDING, &ret);
-		sdio_release_host(func);
-		DPRINTK(1, "interrupt disable\n");
+	ks_wlan_net_stop(netdev);
+	DPRINTK(1, "ks_wlan_net_stop\n");
 
-		/* send stop request to MAC */
-		{
-			struct hostif_stop_request_t *pp;
+	/* interrupt disable */
+	sdio_claim_host(func);
+	sdio_writeb(func, 0, INT_ENABLE, &ret);
+	sdio_writeb(func, 0xff, INT_PENDING, &ret);
+	sdio_release_host(func);
+	DPRINTK(1, "interrupt disable\n");
 
-			pp = kzalloc(hif_align_size(sizeof(*pp)), GFP_KERNEL);
-			if (!pp) {
-				DPRINTK(3, "allocate memory failed..\n");
-				return;	/* to do goto ni suru */
-			}
-			pp->header.size =
-			    cpu_to_le16((uint16_t)
-					(sizeof(*pp) -
-					 sizeof(pp->header.size)));
-			pp->header.event = cpu_to_le16((uint16_t)HIF_STOP_REQ);
-
-			sdio_claim_host(func);
-			write_to_device(priv, (unsigned char *)pp,
-					hif_align_size(sizeof(*pp)));
-			sdio_release_host(func);
-			kfree(pp);
-		}
-		DPRINTK(1, "STOP Req\n");
+	/* send stop request to MAC */
+	{
+		struct hostif_stop_request_t *pp;
 
-		if (priv->ks_wlan_hw.ks7010sdio_wq) {
-			flush_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
-			destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
+		pp = kzalloc(hif_align_size(sizeof(*pp)), GFP_KERNEL);
+		if (!pp) {
+			DPRINTK(3, "allocate memory failed..\n");
+			return;	/* to do goto ni suru */
 		}
-		DPRINTK(1,
-			"destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);\n");
+		pp->header.size =
+			cpu_to_le16((uint16_t)
+				(sizeof(*pp) -
+					sizeof(pp->header.size)));
+		pp->header.event = cpu_to_le16((uint16_t)HIF_STOP_REQ);
 
-		hostif_exit(priv);
-		DPRINTK(1, "hostif_exit\n");
-
-		unregister_netdev(netdev);
+		sdio_claim_host(func);
+		write_to_device(priv, (unsigned char *)pp,
+				hif_align_size(sizeof(*pp)));
+		sdio_release_host(func);
+		kfree(pp);
+	}
+	DPRINTK(1, "STOP Req\n");
 
-		trx_device_exit(priv);
-		kfree(priv->ks_wlan_hw.read_buf);
-		free_netdev(priv->net_dev);
-		card->priv = NULL;
+	if (priv->ks_wlan_hw.ks7010sdio_wq) {
+		flush_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
+		destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);
 	}
+	DPRINTK(1,
+		"destroy_workqueue(priv->ks_wlan_hw.ks7010sdio_wq);\n");
+
+	hostif_exit(priv);
+	DPRINTK(1, "hostif_exit\n");
+
+	unregister_netdev(netdev);
+
+	trx_device_exit(priv);
+	kfree(priv->ks_wlan_hw.read_buf);
+	free_netdev(priv->net_dev);
+	card->priv = NULL;
 
+out:
 	sdio_claim_host(func);
 	sdio_release_irq(func);
 	DPRINTK(1, "sdio_release_irq()\n");
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/3] staging: ks7010: factor out send stop request

[Tobin C. Harding]

Function contains a block of code combined as a compound
statement. This would be better factored out into a separate function.

Factor compound statement out into a separate function.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 46 ++++++++++++++++++++----------------
 1 file changed, 26 insertions(+), 20 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index 0f5ff68..a820371 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1062,6 +1062,29 @@ static int ks7010_sdio_probe(struct sdio_func *func,
 	return -ENODEV;
 }
 
+/* send stop request to MAC */
+static int send_stop_request(struct ks_sdio_card *card)
+{
+	struct hostif_stop_request_t *pp;
+	size_t size;
+
+	pp = kzalloc(hif_align_size(sizeof(*pp)), GFP_KERNEL);
+	if (!pp)
+		return -ENOMEM;
+
+	size = sizeof(*pp) - sizeof(pp->header.size);
+	pp->header.size = cpu_to_le16((uint16_t)size);
+	pp->header.event = cpu_to_le16((uint16_t)HIF_STOP_REQ);
+
+	sdio_claim_host(card->func);
+	write_to_device(card->priv, (unsigned char *)pp,
+			hif_align_size(sizeof(*pp)));
+	sdio_release_host(card->func);
+
+	kfree(pp);
+	return 0;
+}
+
 static void ks7010_sdio_remove(struct sdio_func *func)
 {
 	int ret;
@@ -1093,27 +1116,10 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 	sdio_release_host(func);
 	DPRINTK(1, "interrupt disable\n");
 
-	/* send stop request to MAC */
-	{
-		struct hostif_stop_request_t *pp;
+	ret = send_stop_request(card);
+	if (ret)
+		return;
 
-		pp = kzalloc(hif_align_size(sizeof(*pp)), GFP_KERNEL);
-		if (!pp) {
-			DPRINTK(3, "allocate memory failed..\n");
-			return;	/* to do goto ni suru */
-		}
-		pp->header.size =
-			cpu_to_le16((uint16_t)
-				(sizeof(*pp) -
-					sizeof(pp->header.size)));
-		pp->header.event = cpu_to_le16((uint16_t)HIF_STOP_REQ);
-
-		sdio_claim_host(func);
-		write_to_device(priv, (unsigned char *)pp,
-				hif_align_size(sizeof(*pp)));
-		sdio_release_host(func);
-		kfree(pp);
-	}
 	DPRINTK(1, "STOP Req\n");
 
 	if (priv->ks_wlan_hw.ks7010sdio_wq) {
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 3/3] staging: ks7010: remove extraneous newlines.

[Tobin C. Harding]

Function contains extraneous newlines. Removing these lines aids
readability.

Remove extraneous newlines.

Signed-off-by: Tobin C. Harding <me@tobin.cc>
---
 drivers/staging/ks7010/ks7010_sdio.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/ks7010/ks7010_sdio.c b/drivers/staging/ks7010/ks7010_sdio.c
index a820371..9b4caa0 100644
--- a/drivers/staging/ks7010/ks7010_sdio.c
+++ b/drivers/staging/ks7010/ks7010_sdio.c
@@ -1095,7 +1095,6 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 	DPRINTK(1, "ks7010_sdio_remove()\n");
 
 	card = sdio_get_drvdata(func);
-
 	if (!card)
 		return;
 
@@ -1148,10 +1147,8 @@ static void ks7010_sdio_remove(struct sdio_func *func)
 	sdio_release_host(func);
 
 	sdio_set_drvdata(func, NULL);
-
 	kfree(card);
 	DPRINTK(1, "kfree()\n");
-
 	DPRINTK(5, " Bye !!\n");
 }
 
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel