[PATCH] staging: rtl8188eu: remove unnecessary self-assignment
This is a self-assignment which is redundant. Fix this by removing the
self-assignment.
Addresses-Coverity: ("Self assignment")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
index 086f98d38cba..57ae0e83dd3e 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_hal_init.c
@@ -552,7 +552,6 @@ void Hal_ReadAntennaDiversity88E(struct adapter *pAdapter,
u8 *PROMContent, bool
pHalData->AntDivCfg = 1; /* 0xC1[3] is ignored. */
} else {
pHalData->AntDivCfg = 0;
- pHalData->TRxAntDivType = pHalData->TRxAntDivType; /* The
value in the driver setting of device manager. */
}
DBG_88E("EEPROM : AntDivCfg = %x, TRxAntDivType = %x\n",
pHalData->AntDivCfg, pHalData->TRxAntDivType);
}
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: remove dead code in do-while conditional step
The local variable 'bcmd_down' is always set to true almost immediately
before the do-while's condition is checked. As a result, !bcmd_down
evaluates to false which short circuits the logical AND operator meaning
that the second operand is never reached and is therefore dead code.
Addresses-Coverity: ("Logically dead code")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 47352f210c0b..a4b317937b23 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -48,7 +48,6 @@ static u8 _is_fw_read_cmd_down(struct adapter *adapt, u8
msgbox_num)
static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32 CmdLen, u8
*pCmdBuffer)
{
u8 bcmd_down = false;
- s32 retry_cnts = 100;
u8 h2c_box_num;
u32 msgbox_addr;
u32 msgbox_ex_addr;
@@ -103,7 +102,7 @@ static s32 FillH2CCmd_88E(struct adapter *adapt, u8
ElementID, u32 CmdLen, u8 *p
adapt->HalData->LastHMEBoxNum =
(h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
- } while ((!bcmd_down) && (retry_cnts--));
+ } while (!bcmd_down);
ret = _SUCCESS;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: remove dead code in do-while conditional step
On 9/23/19 1:38 PM, Larry Finger wrote:
On 9/23/19 2:48 PM, Connor Kuehl wrote:
The local variable 'bcmd_down' is always set to true almost immediately
before the do-while's condition is checked. As a result, !bcmd_down
evaluates to false which short circuits the logical AND operator meaning
that the second operand is never reached and is therefore dead code.
Addresses-Coverity: ("Logically dead code")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 47352f210c0b..a4b317937b23 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -48,7 +48,6 @@ static u8 _is_fw_read_cmd_down(struct adapter
*adapt, u8 msgbox_num)
static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32
CmdLen, u8 *pCmdBuffer)
{
u8 bcmd_down = false;
- s32 retry_cnts = 100;
u8 h2c_box_num;
u32 msgbox_addr;
u32 msgbox_ex_addr;
@@ -103,7 +102,7 @@ static s32 FillH2CCmd_88E(struct adapter *adapt,
u8 ElementID, u32 CmdLen, u8 *p
adapt->HalData->LastHMEBoxNum =
(h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
- } while ((!bcmd_down) && (retry_cnts--));
+ } while (!bcmd_down);
ret = _SUCCESS;
This patch is correct; however, the do..while loop will always be
executed once, thus you could remove the loop and the loop variable
bcmd_down.
Ah, yes! That makes sense, good catch.
@greg: If you would prefer a two-step process, then this one is OK.
I'll do whichever is preferred. I'm happy to NACK this and send a v2
with the dead code and loop removed or I can send a separate patch based
on this one to remove the loop.
Thank you,
Connor
Larry
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
NACK: [PATCH] staging: rtl8188eu: remove dead code in do-while conditional step
I'm sending a V2 with the loop removed. Thanks, Connor ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8188eu: remove dead code/vestigial do..while loop
The local variable 'bcmd_down' is always set to true almost immediately
before the do-while's condition is checked. As a result, !bcmd_down
evaluates to false which short circuits the logical AND operator meaning
that the second operand is never reached and is therefore dead code.
Furthermore, the do..while loop may be removed since it will always only
execute once because 'bcmd_down' is always set to true, so the
!bcmd_down evaluates to false and the loop exits immediately after the
first pass.
Fix this by removing the loop and its condition variables 'bcmd_down'
and 'retry_cnts'
While we're in there, also fix some checkpatch.pl suggestions regarding
spaces around arithmetic operators like '+'
Addresses-Coverity: ("Logically dead code")
Signed-off-by: Connor Kuehl
---
v1 -> v2:
- remove the loop and its condition variable bcmd_down
- address some non-invasive checkpatch.pl suggestions as a result of
deleting the loop
drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c | 55 +---
1 file changed, 24 insertions(+), 31 deletions(-)
diff --git a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
index 47352f210c0b..7646167a0b36 100644
--- a/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
+++ b/drivers/staging/rtl8188eu/hal/rtl8188e_cmd.c
@@ -47,8 +47,6 @@ static u8 _is_fw_read_cmd_down(struct adapter *adapt, u8
msgbox_num)
**/
static s32 FillH2CCmd_88E(struct adapter *adapt, u8 ElementID, u32 CmdLen, u8
*pCmdBuffer)
{
- u8 bcmd_down = false;
- s32 retry_cnts = 100;
u8 h2c_box_num;
u32 msgbox_addr;
u32 msgbox_ex_addr;
@@ -71,39 +69,34 @@ static s32 FillH2CCmd_88E(struct adapter *adapt, u8
ElementID, u32 CmdLen, u8 *p
goto exit;
/* pay attention to if race condition happened in H2C cmd setting. */
- do {
- h2c_box_num = adapt->HalData->LastHMEBoxNum;
-
- if (!_is_fw_read_cmd_down(adapt, h2c_box_num)) {
- DBG_88E(" fw read cmd failed...\n");
- goto exit;
- }
-
- *(u8 *)(&h2c_cmd) = ElementID;
-
- if (CmdLen <= 3) {
- memcpy((u8 *)(&h2c_cmd)+1, pCmdBuffer, CmdLen);
- } else {
- memcpy((u8 *)(&h2c_cmd)+1, pCmdBuffer, 3);
- ext_cmd_len = CmdLen-3;
- memcpy((u8 *)(&h2c_cmd_ex), pCmdBuffer+3, ext_cmd_len);
+ h2c_box_num = adapt->HalData->LastHMEBoxNum;
- /* Write Ext command */
- msgbox_ex_addr = REG_HMEBOX_EXT_0 + (h2c_box_num *
RTL88E_EX_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++)
- usb_write8(adapt, msgbox_ex_addr+cmd_idx, *((u8
*)(&h2c_cmd_ex)+cmd_idx));
- }
- /* Write command */
- msgbox_addr = REG_HMEBOX_0 + (h2c_box_num *
RTL88E_MESSAGE_BOX_SIZE);
- for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++)
- usb_write8(adapt, msgbox_addr+cmd_idx, *((u8
*)(&h2c_cmd)+cmd_idx));
+ if (!_is_fw_read_cmd_down(adapt, h2c_box_num)) {
+ DBG_88E(" fw read cmd failed...\n");
+ goto exit;
+ }
- bcmd_down = true;
+ *(u8 *)(&h2c_cmd) = ElementID;
- adapt->HalData->LastHMEBoxNum =
- (h2c_box_num+1) % RTL88E_MAX_H2C_BOX_NUMS;
+ if (CmdLen <= 3) {
+ memcpy((u8 *)(&h2c_cmd) + 1, pCmdBuffer, CmdLen);
+ } else {
+ memcpy((u8 *)(&h2c_cmd) + 1, pCmdBuffer, 3);
+ ext_cmd_len = CmdLen - 3;
+ memcpy((u8 *)(&h2c_cmd_ex), pCmdBuffer + 3, ext_cmd_len);
+
+ /* Write Ext command */
+ msgbox_ex_addr = REG_HMEBOX_EXT_0 + (h2c_box_num *
RTL88E_EX_MESSAGE_BOX_SIZE);
+ for (cmd_idx = 0; cmd_idx < ext_cmd_len; cmd_idx++)
+ usb_write8(adapt, msgbox_ex_addr + cmd_idx, *((u8
*)(&h2c_cmd_ex) + cmd_idx));
+ }
+ /* Write command */
+ msgbox_addr = REG_HMEBOX_0 + (h2c_box_num * RTL88E_MESSAGE_BOX_SIZE);
+ for (cmd_idx = 0; cmd_idx < RTL88E_MESSAGE_BOX_SIZE; cmd_idx++)
+ usb_write8(adapt, msgbox_addr + cmd_idx, *((u8 *)(&h2c_cmd) +
cmd_idx));
- } while ((!bcmd_down) && (retry_cnts--));
+ adapt->HalData->LastHMEBoxNum =
+ (h2c_box_num + 1) % RTL88E_MAX_H2C_BOX_NUMS;
ret = _SUCCESS;
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: fix possible null dereference
Inside a nested 'else' block at the beginning of this function is a
call that assigns 'psta' to the return value of 'rtw_get_stainfo()'.
If 'rtw_get_stainfo()' returns NULL and the flow of control reaches
the 'else if' where 'psta' is dereferenced, then we will dereference
a NULL pointer.
Fix this by checking if 'psta' is not NULL before reading its
'psta->qos_option' data member.
Addresses-Coverity: ("Dereference null return value")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/core/rtw_xmit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 952f2ab51347..bf8877cbe9b6 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -784,7 +784,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, pattrib->src, ETH_ALEN);
memcpy(pwlanhdr->addr3, get_bssid(pmlmepriv), ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else {
RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
("fw_state:%x is not allowed to xmit frame\n", get_fwstate(pmlmepriv)));
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: fix possible null dereference
On 9/25/19 5:05 PM, Larry Finger wrote: This change is a good one, but why not get the same fix at line 779? Ah yes! Thanks for pointing that out. I missed that. I will send a V2 shortly. Thank you, Connor Larry ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v2] staging: rtl8188eu: fix possible null dereference
Inside a nested 'else' block at the beginning of this function is a
call that assigns 'psta' to the return value of 'rtw_get_stainfo()'.
If 'rtw_get_stainfo()' returns NULL and the flow of control reaches
the 'else if' where 'psta' is dereferenced, then we will dereference
a NULL pointer.
Fix this by checking if 'psta' is not NULL before reading its
'psta->qos_option' data member.
Addresses-Coverity: ("Dereference null return value")
Signed-off-by: Connor Kuehl
---
v1 -> v2:
- Add the same null check to line 779
drivers/staging/rtl8188eu/core/rtw_xmit.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_xmit.c
b/drivers/staging/rtl8188eu/core/rtw_xmit.c
index 952f2ab51347..c37591657bac 100644
--- a/drivers/staging/rtl8188eu/core/rtw_xmit.c
+++ b/drivers/staging/rtl8188eu/core/rtw_xmit.c
@@ -776,7 +776,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, get_bssid(pmlmepriv), ETH_ALEN);
memcpy(pwlanhdr->addr3, pattrib->src, ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else if (check_fwstate(pmlmepriv, WIFI_ADHOC_STATE) ||
check_fwstate(pmlmepriv, WIFI_ADHOC_MASTER_STATE)) {
@@ -784,7 +784,7 @@ s32 rtw_make_wlanhdr(struct adapter *padapter, u8 *hdr,
struct pkt_attrib *pattr
memcpy(pwlanhdr->addr2, pattrib->src, ETH_ALEN);
memcpy(pwlanhdr->addr3, get_bssid(pmlmepriv), ETH_ALEN);
- if (psta->qos_option)
+ if (psta && psta->qos_option)
qos_option = true;
} else {
RT_TRACE(_module_rtl871x_xmit_c_, _drv_err_,
("fw_state:%x is not allowed to xmit frame\n", get_fwstate(pmlmepriv)));
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails
If kzalloc() returns NULL, the error path doesn't stop the flow of
control from entering rtw_hal_read_chip_version() which dereferences the
null pointer. Fix this by adding a 'goto' to the error path to more
gracefully handle the issue and avoid proceeding with initialization
steps that we're no longer prepared to handle.
Also update the debug message to be more consistent with the other debug
messages in this function.
Addresses-Coverity: ("Dereference after null check")
Signed-off-by: Connor Kuehl
---
drivers/staging/rtl8188eu/os_dep/usb_intf.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
index 664d93a7f90d..4fac9dca798e 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_intf.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_intf.c
@@ -348,8 +348,10 @@ static struct adapter *rtw_usb_if1_init(struct dvobj_priv
*dvobj,
}
padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
- if (!padapter->HalData)
- DBG_88E("cant not alloc memory for HAL DATA\n");
+ if (!padapter->HalData) {
+ DBG_88E("Failed to allocate memory for HAL data\n");
+ goto free_adapter;
+ }
/* step read_chip_version */
rtw_hal_read_chip_version(padapter);
--
2.17.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rtl8188eu: fix null dereference when kzalloc fails
On 10/1/19 6:11 AM, Dan Carpenter wrote:
There is another one earlier in the function as well.
drivers/staging/rtl8188eu/os_dep/usb_intf.c
336
337 pnetdev = rtw_init_netdev(padapter);
338 if (!pnetdev)
339 goto free_adapter;
340 SET_NETDEV_DEV(pnetdev, dvobj_to_dev(dvobj));
341 padapter = rtw_netdev_priv(pnetdev);
342
343 if (padapter->registrypriv.monitor_enable) {
344 pmondev = rtl88eu_mon_init();
345 if (!pmondev)
346 netdev_warn(pnetdev, "Failed to initialize monitor
interface");
goto free_adapter.
347 padapter->pmondev = pmondev;
348 }
349
350 padapter->HalData = kzalloc(sizeof(struct hal_data_8188e),
GFP_KERNEL);
351 if (!padapter->HalData)
352 DBG_88E("cant not alloc memory for HAL DATA\n");
353
padapter->HalData = kzalloc(sizeof(struct hal_data_8188e), GFP_KERNEL);
- if (!padapter->HalData)
- DBG_88E("cant not alloc memory for HAL DATA\n");
+ if (!padapter->HalData) {
+ DBG_88E("Failed to allocate memory for HAL data\n");
Remove this debug printk.
+ goto free_adapter;
+ }
Hi Dan,
Sorry for such a late response! By the time I saw the e-mail with your
feedback I also saw another e-mail saying this patch was accepted into a
staging-linus tree. I'll address your comments in a separate patch.
Thank you,
Connor
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
