Hi
The static code analysis program called cppcheck.
http://cppcheck.sourceforge.net/
I found code that I think are bugs, or at least inappropriate or
unnecessary code, in the files:
drivers/staging/rtl8712/rtl871x_ioctl_linux.c
drivers/staging/rtl8712/rtl871x_mlme.c
drivers/staging/rtl8712/usb_intf.c
I have created a patch, and inluderat the error file generated by cppcheck.
My goal was not to change any functionality, but it does not mean for
example the unused variables can't mean that there are other
problems/mistakes in the code. So a proper code review :)
Is there anything else I can help with regarding the patch or
cppcheck, do not hesitate to contact me.
If you like this type of code analysis, it is possible to get more
warnings, which are not as serious, but that may well indicate other
mistakes.
Best regards
Rickard Strandqvist
From 0ef1cda18e05aa6d0b0ea745ce194f33d8f03973 Mon Sep 17 00:00:00 2001
From: Rickard Strandqvist <rickard_strandqvist@xxxxxxxxxxxxxxxxxx>
Date: Wed, 30 Apr 2014 16:27:31 +0200
Subject: [PATCH] Found same errors using a static code analysis program
called cppcheck.
---
drivers/staging/rtl8712/rtl871x_ioctl_linux.c | 19 +++++-----------
drivers/staging/rtl8712/rtl871x_mlme.c | 29 +++++++++++++------------
drivers/staging/rtl8712/usb_intf.c | 8 +------
3 filer ändrade, 22 tillägg(+), 34 borttagningar(-)
diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
index 23d539d..1d4475d 100644
--- a/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
+++ b/drivers/staging/rtl8712/rtl871x_ioctl_linux.c
@@ -170,7 +170,7 @@ static inline char *translate_scan(struct _adapter *padapter,
s8 *p;
u32 i = 0, ht_ielen = 0;
u16 cap, ht_cap = false, mcs_rate;
- u8 rssi, bw_40MHz = 0, short_GI = 0;
+ u8 rssi;
if ((pnetwork->network.Configuration.DSConfig < 1) ||
(pnetwork->network.Configuration.DSConfig > 14)) {
@@ -197,10 +197,6 @@ static inline char *translate_scan(struct _adapter *padapter,
ht_cap = true;
pht_capie = (struct ieee80211_ht_cap *)(p + 2);
memcpy(&mcs_rate , pht_capie->supp_mcs_set, 2);
- bw_40MHz = (pht_capie->cap_info&IEEE80211_HT_CAP_SUP_WIDTH)
- ? 1 : 0;
- short_GI = (pht_capie->cap_info&(IEEE80211_HT_CAP_SGI_20 |
- IEEE80211_HT_CAP_SGI_40)) ? 1 : 0;
}
/* Add the protocol name */
iwe.cmd = SIOCGIWNAME;
@@ -286,8 +282,8 @@ static inline char *translate_scan(struct _adapter *padapter,
u8 wpa_ie[255], rsn_ie[255];
u16 wpa_len = 0, rsn_len = 0;
int n;
- sint out_len = 0;
- out_len = r8712_get_sec_ie(pnetwork->network.IEs,
+
+ r8712_get_sec_ie(pnetwork->network.IEs,
pnetwork->network.
IELength, rsn_ie, &rsn_len,
wpa_ie, &wpa_len);
@@ -1133,13 +1129,12 @@ static int r871x_wx_set_mlme(struct net_device *dev,
union iwreq_data *wrqu, char *extra)
{
int ret = 0;
- u16 reason;
struct _adapter *padapter = (struct _adapter *)netdev_priv(dev);
struct iw_mlme *mlme = (struct iw_mlme *) extra;
if (mlme == NULL)
return -1;
- reason = cpu_to_le16(mlme->reason_code);
+ cpu_to_le16(mlme->reason_code);
switch (mlme->cmd) {
case IW_MLME_DEAUTH:
if (!r8712_set_802_11_disassociate(padapter))
@@ -1465,10 +1460,7 @@ static int r8711_wx_get_rate(struct net_device *dev,
RTL8712_RF_2T2R == rf_type)
max_rate = (bw_40MHz) ? ((short_GI) ? 300 :
270) : ((short_GI) ? 144 : 130);
- else if (mcs_rate & 0x0080) /* MCS7 */
- max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
- 135) : ((short_GI) ? 72 : 65);
- else /* default MCS7 */
+ else /* default MCS7 and MCS7 */
max_rate = (bw_40MHz) ? ((short_GI) ? 150 :
135) : ((short_GI) ? 72 : 65);
max_rate *= 2; /* Mbps/2 */
@@ -1822,6 +1814,7 @@ static int r871x_wx_set_enc_ext(struct net_device *dev,
alg_name = "CCMP";
break;
default:
+ kfree(param);
return -EINVAL;
}
strncpy((char *)param->u.crypt.alg, alg_name, IEEE_CRYPT_ALG_NAME_LEN);
diff --git a/drivers/staging/rtl8712/rtl871x_mlme.c b/drivers/staging/rtl8712/rtl871x_mlme.c
index 3ea99ae..f126763 100644
--- a/drivers/staging/rtl8712/rtl871x_mlme.c
+++ b/drivers/staging/rtl8712/rtl871x_mlme.c
@@ -1274,22 +1274,30 @@ sint r8712_set_key(struct _adapter *adapter,
psecuritypriv->DefKey[keyid].skey, keylen);
break;
case _TKIP_:
- if (keyid < 1 || keyid > 2)
+ if (keyid < 1 || keyid > 2) {
+ kfree((unsigned char *)pcmd);
+ kfree((unsigned char *)psetkeyparm);
return _FAIL;
+ }
keylen = 16;
memcpy(psetkeyparm->key,
&psecuritypriv->XGrpKey[keyid - 1], keylen);
psetkeyparm->grpkey = 1;
break;
case _AES_:
- if (keyid < 1 || keyid > 2)
+ if (keyid < 1 || keyid > 2) {
+ kfree((unsigned char *)pcmd);
+ kfree((unsigned char *)psetkeyparm);
return _FAIL;
+ }
keylen = 16;
memcpy(psetkeyparm->key,
&psecuritypriv->XGrpKey[keyid - 1], keylen);
psetkeyparm->grpkey = 1;
break;
default:
+ kfree((unsigned char *)pcmd);
+ kfree((unsigned char *)psetkeyparm);
return _FAIL;
}
pcmd->cmdcode = _SetKey_CMD_;
@@ -1364,7 +1372,7 @@ static int SecIsInPMKIDList(struct _adapter *Adapter, u8 *bssid)
sint r8712_restruct_sec_ie(struct _adapter *adapter, u8 *in_ie,
u8 *out_ie, uint in_len)
{
- u8 authmode = 0, securitytype, match;
+ u8 authmode = 0, match;
u8 sec_ie[255], uncst_oui[4], bkup_ie[255];
u8 wpa_oui[4] = {0x0, 0x50, 0xf2, 0x01};
uint ielength, cnt, remove_cnt;
@@ -1391,21 +1399,17 @@ sint r8712_restruct_sec_ie(struct _adapter *adapter, u8 *in_ie,
switch (ndissecuritytype) {
case Ndis802_11Encryption1Enabled:
case Ndis802_11Encryption1KeyAbsent:
- securitytype = _WEP40_;
uncst_oui[3] = 0x1;
break;
case Ndis802_11Encryption2Enabled:
case Ndis802_11Encryption2KeyAbsent:
- securitytype = _TKIP_;
uncst_oui[3] = 0x2;
break;
case Ndis802_11Encryption3Enabled:
case Ndis802_11Encryption3KeyAbsent:
- securitytype = _AES_;
uncst_oui[3] = 0x4;
break;
default:
- securitytype = _NO_PRIVACY_;
break;
}
/*Search required WPA or WPA2 IE and copy to sec_ie[] */
@@ -1697,7 +1701,7 @@ unsigned int r8712_restructure_ht_ie(struct _adapter *padapter, u8 *in_ie,
u8 *out_ie, uint in_len, uint *pout_len)
{
u32 ielen, out_len;
- unsigned char *p, *pframe;
+ unsigned char *p;
struct ieee80211_ht_cap ht_capie;
unsigned char WMM_IE[] = {0x00, 0x50, 0xf2, 0x02, 0x00, 0x01, 0x00};
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
@@ -1709,7 +1713,7 @@ unsigned int r8712_restructure_ht_ie(struct _adapter *padapter, u8 *in_ie,
if (p && (ielen > 0)) {
if (pqospriv->qos_option == 0) {
out_len = *pout_len;
- pframe = r8712_set_ie(out_ie+out_len,
+ r8712_set_ie(out_ie+out_len,
_VENDOR_SPECIFIC_IE_,
_WMM_IE_Length_,
WMM_IE, pout_len);
@@ -1725,7 +1729,7 @@ unsigned int r8712_restructure_ht_ie(struct _adapter *padapter, u8 *in_ie,
IEEE80211_HT_CAP_DSSSCCK40;
ht_capie.ampdu_params_info = (IEEE80211_HT_CAP_AMPDU_FACTOR &
0x03) | (IEEE80211_HT_CAP_AMPDU_DENSITY & 0x00);
- pframe = r8712_set_ie(out_ie+out_len, _HT_CAPABILITY_IE_,
+ r8712_set_ie(out_ie+out_len, _HT_CAPABILITY_IE_,
sizeof(struct ieee80211_ht_cap),
(unsigned char *)&ht_capie, pout_len);
phtpriv->ht_option = 1;
@@ -1740,7 +1744,6 @@ static void update_ht_cap(struct _adapter *padapter, u8 *pie, uint ie_len)
int i, len;
struct sta_info *bmc_sta, *psta;
struct ieee80211_ht_cap *pht_capie;
- struct ieee80211_ht_addt_info *pht_addtinfo;
struct recv_reorder_ctrl *preorder_ctrl;
struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
struct ht_priv *phtpriv = &pmlmepriv->htpriv;
@@ -1790,11 +1793,9 @@ static void update_ht_cap(struct _adapter *padapter, u8 *pie, uint ie_len)
}
}
len = 0;
- p = r8712_get_ie(pie + sizeof(struct NDIS_802_11_FIXED_IEs),
+ r8712_get_ie(pie + sizeof(struct NDIS_802_11_FIXED_IEs),
_HT_ADD_INFO_IE_, &len,
ie_len-sizeof(struct NDIS_802_11_FIXED_IEs));
- if (p && len > 0)
- pht_addtinfo = (struct ieee80211_ht_addt_info *)(p + 2);
}
void r8712_issue_addbareq_cmd(struct _adapter *padapter, int priority)
diff --git a/drivers/staging/rtl8712/usb_intf.c b/drivers/staging/rtl8712/usb_intf.c
index bbd5888..46eec91 100644
--- a/drivers/staging/rtl8712/usb_intf.c
+++ b/drivers/staging/rtl8712/usb_intf.c
@@ -255,9 +255,6 @@ static struct drv_priv drvpriv = {
static uint r8712_usb_dvobj_init(struct _adapter *padapter)
{
uint status = _SUCCESS;
- struct usb_device_descriptor *pdev_desc;
- struct usb_host_config *phost_conf;
- struct usb_config_descriptor *pconf_desc;
struct usb_host_interface *phost_iface;
struct usb_interface_descriptor *piface_desc;
struct dvobj_priv *pdvobjpriv = &padapter->dvobjpriv;
@@ -265,9 +262,6 @@ static uint r8712_usb_dvobj_init(struct _adapter *padapter)
pdvobjpriv->padapter = padapter;
padapter->EepromAddressSize = 6;
- pdev_desc = &pusbd->descriptor;
- phost_conf = pusbd->actconfig;
- pconf_desc = &phost_conf->desc;
phost_iface = &pintf->altsetting[0];
piface_desc = &phost_iface->desc;
pdvobjpriv->nr_endpoint = piface_desc->bNumEndpoints;
@@ -595,7 +589,7 @@ static int r871xu_drv_init(struct usb_interface *pusb_intf,
error:
usb_put_dev(udev);
usb_set_intfdata(pusb_intf, NULL);
- if (padapter->dvobj_deinit != NULL)
+ if (padapter && padapter->dvobj_deinit != NULL)
padapter->dvobj_deinit(padapter);
if (pnetdev)
free_netdev(pnetdev);
--
1.7.10.4
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 1142] : (style) Variable 'reason' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 1471] -> [drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 1468] : (style, inconclusive) Found duplicate branches for 'if' and 'else'.
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 1825] : (error) Memory leak : param
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 1142] : (style) Variable 'reason' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 200] : (style) Variable 'bw_40MHz' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 202] : (style) Variable 'short_GI' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_ioctl_linux.c : 290] : (style) Variable 'out_len' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_mlme.c : 1728] : (style) Variable 'pframe' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_mlme.c : 1797] : (style) Variable 'pht_addtinfo' is assigned a value that is never used.
drivers/staging/rtl8712/rtl871x_mlme.c : 1293] : (error) Memory leak : pcmd
drivers/staging/rtl8712/rtl871x_mlme.c : 1293] : (error) Memory leak : psetkeyparm
drivers/staging/rtl8712/rtl871x_mlme.c : 1408] : (style) Variable 'securitytype' is assigned a value that is never used.
drivers/staging/rtl8712/usb_intf.c : 270] : (style) Variable 'pconf_desc' is assigned a value that is never used.
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
