public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Joe Perches <joe@perches.com>
To: Suniel Mahesh <sunil.m@techveda.org>
Cc: gregkh@linuxfoundation.org,
	Dan Carpenter <dan.carpenter@oracle.com>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	karthiknishu@yahoo.com
Subject: Re: [PATCH v3 5/8] staging: rtl8192e: Fix unbalanced braces
Date: Wed, 15 Mar 2017 07:54:20 -0700	[thread overview]
Message-ID: <1489589660.2582.10.camel@perches.com> (raw)

As Dan said, the original was better, but
it could still be improved.

There is only a single case used in the switch.

Perhaps better would be to reduce indentation
by removing the switch and using another goto
label for the memory allocation freeing like the
below.

Perhaps better still, if there were to be more
cases added in the future, would be to move the
case handling logic into separate functions.

---
 drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 177 +++++++++++++--------------
 1 file changed, 82 insertions(+), 95 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
index 4c0caa6701a9..8c48b0a527ec 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_core.c
@@ -2283,115 +2283,102 @@ static int _rtl92e_ioctl(struct net_device *dev, struct ifreq *rq, int cmd)
 	int ret = -1;
 	struct rtllib_device *ieee = priv->rtllib;
 	u32 key[4];
-	const u8 broadcast_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
+	static const u8 broadcast_addr[ETH_ALEN] = {
+		0xff, 0xff, 0xff, 0xff, 0xff, 0xff
+	};
 	struct iw_point *p = &wrq->u.data;
 	struct ieee_param *ipw = NULL;
 
 	mutex_lock(&priv->wx_mutex);
 
-	switch (cmd) {
-	case RTL_IOCTL_WPA_SUPPLICANT:
-		if (p->length < sizeof(struct ieee_param) || !p->pointer) {
-			ret = -EINVAL;
-			goto out;
-		}
+	if (cmd != RTL_IOCTL_WPA_SUPPLICANT) {
+		ret = -EOPNOTSUPP;
+		goto out;
+	}
 
-		ipw = memdup_user(p->pointer, p->length);
-		if (IS_ERR(ipw)) {
-			ret = PTR_ERR(ipw);
-			goto out;
-		}
+	if (p->length < sizeof(struct ieee_param) || !p->pointer) {
+		ret = -EINVAL;
+		goto out;
+	}
 
-		if (ipw->cmd == IEEE_CMD_SET_ENCRYPTION) {
-			if (ipw->u.crypt.set_tx) {
-				if (strcmp(ipw->u.crypt.alg, "CCMP") == 0)
-					ieee->pairwise_key_type = KEY_TYPE_CCMP;
-				else if (strcmp(ipw->u.crypt.alg, "TKIP") == 0)
-					ieee->pairwise_key_type = KEY_TYPE_TKIP;
-				else if (strcmp(ipw->u.crypt.alg, "WEP") == 0) {
-					if (ipw->u.crypt.key_len == 13)
-						ieee->pairwise_key_type =
-							 KEY_TYPE_WEP104;
-					else if (ipw->u.crypt.key_len == 5)
-						ieee->pairwise_key_type =
-							 KEY_TYPE_WEP40;
-				} else {
-					ieee->pairwise_key_type = KEY_TYPE_NA;
-				}
+	ipw = memdup_user(p->pointer, p->length);
+	if (IS_ERR(ipw)) {
+		ret = PTR_ERR(ipw);
+		goto out;
+	}
 
-				if (ieee->pairwise_key_type) {
-					if (is_zero_ether_addr(ieee->ap_mac_addr))
-						ieee->iw_mode = IW_MODE_ADHOC;
-					memcpy((u8 *)key, ipw->u.crypt.key, 16);
-					rtl92e_enable_hw_security_config(dev);
-					rtl92e_set_swcam(dev, 4,
-							 ipw->u.crypt.idx,
-							 ieee->pairwise_key_type,
-							 (u8 *)ieee->ap_mac_addr,
-							 0, key, 0);
-					rtl92e_set_key(dev, 4, ipw->u.crypt.idx,
-						       ieee->pairwise_key_type,
-						       (u8 *)ieee->ap_mac_addr,
-						       0, key);
-					if (ieee->iw_mode == IW_MODE_ADHOC) {
-						rtl92e_set_swcam(dev,
-								 ipw->u.crypt.idx,
-								 ipw->u.crypt.idx,
-								 ieee->pairwise_key_type,
-								 (u8 *)ieee->ap_mac_addr,
-								 0, key, 0);
-						rtl92e_set_key(dev,
-							       ipw->u.crypt.idx,
-							       ipw->u.crypt.idx,
-							       ieee->pairwise_key_type,
-							       (u8 *)ieee->ap_mac_addr,
-							       0, key);
-					}
-				}
-				if ((ieee->pairwise_key_type == KEY_TYPE_CCMP)
-				     && ieee->pHTInfo->bCurrentHTSupport) {
-					rtl92e_writeb(dev, 0x173, 1);
-				}
+	if (ipw->cmd != IEEE_CMD_SET_ENCRYPTION)
+		goto out_free;
 
-			} else {
-				memcpy((u8 *)key, ipw->u.crypt.key, 16);
-				if (strcmp(ipw->u.crypt.alg, "CCMP") == 0)
-					ieee->group_key_type = KEY_TYPE_CCMP;
-				else if (strcmp(ipw->u.crypt.alg, "TKIP") == 0)
-					ieee->group_key_type = KEY_TYPE_TKIP;
-				else if (strcmp(ipw->u.crypt.alg, "WEP") == 0) {
-					if (ipw->u.crypt.key_len == 13)
-						ieee->group_key_type =
-							 KEY_TYPE_WEP104;
-					else if (ipw->u.crypt.key_len == 5)
-						ieee->group_key_type =
-							 KEY_TYPE_WEP40;
-				} else
-					ieee->group_key_type = KEY_TYPE_NA;
-
-				if (ieee->group_key_type) {
-					rtl92e_set_swcam(dev, ipw->u.crypt.idx,
-							 ipw->u.crypt.idx,
-							 ieee->group_key_type,
-							 broadcast_addr, 0, key,
-							 0);
-					rtl92e_set_key(dev, ipw->u.crypt.idx,
-						       ipw->u.crypt.idx,
-						       ieee->group_key_type,
-						       broadcast_addr, 0, key);
-				}
+	if (ipw->u.crypt.set_tx) {
+		if (strcmp(ipw->u.crypt.alg, "CCMP") == 0) {
+			ieee->pairwise_key_type = KEY_TYPE_CCMP;
+		} else if (strcmp(ipw->u.crypt.alg, "TKIP") == 0) {
+			ieee->pairwise_key_type = KEY_TYPE_TKIP;
+		} else if (strcmp(ipw->u.crypt.alg, "WEP") == 0) {
+			if (ipw->u.crypt.key_len == 13)
+				ieee->pairwise_key_type = KEY_TYPE_WEP104;
+			else if (ipw->u.crypt.key_len == 5)
+				ieee->pairwise_key_type = KEY_TYPE_WEP40;
+		} else {
+			ieee->pairwise_key_type = KEY_TYPE_NA;
+		}
+
+		if (ieee->pairwise_key_type) {
+			if (is_zero_ether_addr(ieee->ap_mac_addr))
+				ieee->iw_mode = IW_MODE_ADHOC;
+			memcpy((u8 *)key, ipw->u.crypt.key, 16);
+			rtl92e_enable_hw_security_config(dev);
+			rtl92e_set_swcam(dev, 4, ipw->u.crypt.idx,
+					 ieee->pairwise_key_type,
+					 (u8 *)ieee->ap_mac_addr, 0, key, 0);
+			rtl92e_set_key(dev, 4, ipw->u.crypt.idx,
+				       ieee->pairwise_key_type,
+				       (u8 *)ieee->ap_mac_addr, 0, key);
+			if (ieee->iw_mode == IW_MODE_ADHOC) {
+				rtl92e_set_swcam(dev, ipw->u.crypt.idx,
+						 ipw->u.crypt.idx,
+						 ieee->pairwise_key_type,
+						 (u8 *)ieee->ap_mac_addr,
+						 0, key, 0);
+				rtl92e_set_key(dev, ipw->u.crypt.idx,
+					       ipw->u.crypt.idx,
+					       ieee->pairwise_key_type,
+					       (u8 *)ieee->ap_mac_addr, 0, key);
 			}
 		}
+		if ((ieee->pairwise_key_type == KEY_TYPE_CCMP) &&
+		    ieee->pHTInfo->bCurrentHTSupport)
+			rtl92e_writeb(dev, 0x173, 1);
+	} else {
+		memcpy((u8 *)key, ipw->u.crypt.key, 16);
+		if (strcmp(ipw->u.crypt.alg, "CCMP") == 0) {
+			ieee->group_key_type = KEY_TYPE_CCMP;
+		} else if (strcmp(ipw->u.crypt.alg, "TKIP") == 0) {
+			ieee->group_key_type = KEY_TYPE_TKIP;
+		} else if (strcmp(ipw->u.crypt.alg, "WEP") == 0) {
+			if (ipw->u.crypt.key_len == 13)
+				ieee->group_key_type = KEY_TYPE_WEP104;
+			else if (ipw->u.crypt.key_len == 5)
+				ieee->group_key_type = KEY_TYPE_WEP40;
+		} else {
+			ieee->group_key_type = KEY_TYPE_NA;
+		}
 
-		ret = rtllib_wpa_supplicant_ioctl(priv->rtllib, &wrq->u.data,
-						  0);
-		kfree(ipw);
-		break;
-	default:
-		ret = -EOPNOTSUPP;
-		break;
+		if (ieee->group_key_type) {
+			rtl92e_set_swcam(dev, ipw->u.crypt.idx,
+					 ipw->u.crypt.idx, ieee->group_key_type,
+					 broadcast_addr, 0, key, 0);
+			rtl92e_set_key(dev, ipw->u.crypt.idx, ipw->u.crypt.idx,
+				       ieee->group_key_type, broadcast_addr,
+				       0, key);
+		}
 	}
 
+out_free:
+	ret = rtllib_wpa_supplicant_ioctl(priv->rtllib, &wrq->u.data, 0);
+	kfree(ipw);
+
 out:
 	mutex_unlock(&priv->wx_mutex);
 

             reply	other threads:[~2017-03-15 14:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-15 14:54 Joe Perches [this message]
  -- strict thread matches above, loose matches on Subject: below --
2017-03-12 13:41 [PATCH v2 1/5] staging: rtl8192e: Fix coding style issues Greg KH
2017-03-15  9:51 ` [PATCH v3 0/8] staging: rtl8192e: Fix coding style, warnings and checks sunil.m
2017-03-15  9:51   ` [PATCH v3 5/8] staging: rtl8192e: Fix unbalanced braces sunil.m
2017-03-15 10:24     ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1489589660.2582.10.camel@perches.com \
    --to=joe@perches.com \
    --cc=dan.carpenter@oracle.com \
    --cc=devel@driverdev.osuosl.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=karthiknishu@yahoo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sunil.m@techveda.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox