Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753470AbdCOOy4 (ORCPT ); Wed, 15 Mar 2017 10:54:56 -0400 Received: from smtprelay0252.hostedemail.com ([216.40.44.252]:48814 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752841AbdCOOyf (ORCPT ); Wed, 15 Mar 2017 10:54:35 -0400 X-Session-Marker: 6A6F6540706572636865732E636F6D X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,joe@perches.com,:::::::::::,RULES_HIT:2:41:69:355:379:541:599:960:966:973:988:989:1260:1277:1311:1313:1314:1345:1373:1437:1515:1516:1518:1535:1593:1594:1605:1730:1747:1777:1792:2196:2199:2393:2559:2562:2828:3138:3139:3140:3141:3142:3622:3865:3867:3868:3870:3871:3872:4050:4118:4321:4385:4605:5007:6119:9592:10004:10848:11026:11473:11657:11658:11914:12043:12295:12296:12438:12555:12679:12760:12986:13149:13161:13229:13230:13439:14096:14097:14659:21080:30054,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0,LFtime:2,LUA_SUMMARY:none X-HE-Tag: way24_607f25df24459 X-Filterd-Recvd-Size: 7950 Message-ID: <1489589660.2582.10.camel@perches.com> Subject: Re: [PATCH v3 5/8] staging: rtl8192e: Fix unbalanced braces From: Joe Perches To: Suniel Mahesh Cc: gregkh@linuxfoundation.org, Dan Carpenter , devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org, karthiknishu@yahoo.com Date: Wed, 15 Mar 2017 07:54:20 -0700 Content-Type: text/plain; charset="ISO-8859-1" X-Mailer: Evolution 3.22.3-0ubuntu0.1 Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6982 Lines: 220 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);