Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

7 messages, 4 authors, 2018-10-08 · open the first message on its own page

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Kalle Valo <hidden>
Date: 2018-10-06 19:30:22

Colin King [off-list ref] writes:
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
 drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.

-- 
Kalle Valo

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Larry Finger <hidden>
Date: 2018-10-06 20:05:26

On 10/6/18 2:30 PM, Kalle Valo wrote:
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
  drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi: rtl8821ae: Move driver 
from staging to regular tree").

This driver was initially placed in staging as it was needed for a special 
project, which is the commit that Colin used. As the patch subject states, the 
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger [off-list ref]

thanks,

Larry

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Joe Perches <joe@perches.com>
Date: 2018-10-06 20:17:50

On Sat, 2018-10-06 at 15:05 -0500, Larry Finger wrote:
On 10/6/18 2:30 PM, Kalle Valo wrote:
quoted
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
  drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi: rtl8821ae: Move driver  
from staging to regular tree").

This driver was initially placed in staging as it was needed for a special 
project, which is the commit that Colin used. As the patch subject states, the 
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger [off-list ref]
Why not remove this entirely and use the generic routine in
drivers/net/wireless/realtek/rtlwifi/base.c?

Is there a real difference?

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Larry Finger <hidden>
Date: 2018-10-06 22:00:31

On 10/6/18 3:17 PM, Joe Perches wrote:
On Sat, 2018-10-06 at 15:05 -0500, Larry Finger wrote:
quoted
On 10/6/18 2:30 PM, Kalle Valo wrote:
quoted
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
   drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi: rtl8821ae: Move driver
from staging to regular tree").

This driver was initially placed in staging as it was needed for a special
project, which is the commit that Colin used. As the patch subject states, the
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger [off-list ref]
Why not remove this entirely and use the generic routine in
drivers/net/wireless/realtek/rtlwifi/base.c?

Is there a real difference?
I did not see any difference other than the removal of a bunch of magic numbers 
and better formatting.

Larry

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Joe Perches <joe@perches.com>
Date: 2018-10-06 22:03:26

On Sat, 2018-10-06 at 17:00 -0500, Larry Finger wrote:
On 10/6/18 3:17 PM, Joe Perches wrote:
quoted
On Sat, 2018-10-06 at 15:05 -0500, Larry Finger wrote:
quoted
On 10/6/18 2:30 PM, Kalle Valo wrote:
quoted
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
   drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi: rtl8821ae: Move driver
from staging to regular tree").

This driver was initially placed in staging as it was needed for a special
project, which is the commit that Colin used. As the patch subject states, the
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger [off-list ref]
Why not remove this entirely and use the generic routine in
drivers/net/wireless/realtek/rtlwifi/base.c?

Is there a real difference?
I did not see any difference other than the removal of a bunch of magic numbers 
and better formatting.
Me neither.

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Larry Finger <hidden>
Date: 2018-10-07 00:48:14

On 10/6/18 5:03 PM, Joe Perches wrote:
On Sat, 2018-10-06 at 17:00 -0500, Larry Finger wrote:
quoted
On 10/6/18 3:17 PM, Joe Perches wrote:
quoted
On Sat, 2018-10-06 at 15:05 -0500, Larry Finger wrote:
quoted
On 10/6/18 2:30 PM, Kalle Valo wrote:
quoted
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
    drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi: rtl8821ae: Move driver
from staging to regular tree").

This driver was initially placed in staging as it was needed for a special
project, which is the commit that Colin used. As the patch subject states, the
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger [off-list ref]
Why not remove this entirely and use the generic routine in
drivers/net/wireless/realtek/rtlwifi/base.c?

Is there a real difference?
I did not see any difference other than the removal of a bunch of magic numbers
and better formatting.
Me neither.
Colin,

Do you want to push the new patch removing the duplicate routine from rtl8821ae?

Larry

Re: [PATCH] rtlwifi: rtl8821ae: add in a missing break in switch statement

From: Colin Ian King <hidden>
Date: 2018-10-08 08:55:17

On 07/10/18 01:48, Larry Finger wrote:
On 10/6/18 5:03 PM, Joe Perches wrote:
quoted
On Sat, 2018-10-06 at 17:00 -0500, Larry Finger wrote:
quoted
On 10/6/18 3:17 PM, Joe Perches wrote:
quoted
On Sat, 2018-10-06 at 15:05 -0500, Larry Finger wrote:
quoted
On 10/6/18 2:30 PM, Kalle Valo wrote:
quoted
Colin King [off-list ref] writes:
quoted
From: Colin Ian King <redacted>

The switch case RATR_INX_WIRELESS_MC has a missing break, this seems
to be unintentional as the setting of variable ret gets overwritten
when the case falls through to the following RATR_INX_WIRELESS_AC_5N
case.  Fix this by adding in the missing break.

Detected by CoverityScan, CID#1167237 ("Missing break in switch")

Fixes: 3c05bedb5fef ("Staging: rtl8812ae: Add Realtek 8821 PCI
WIFI driver")
Signed-off-by: Colin Ian King <redacted>
---
    drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 1 +
Is the fixes line correct? This patch is not for staging.
No, the correct fixes commit is 21e4b0726dc67 (" rtlwifi:
rtl8821ae: Move driver
from staging to regular tree").

This driver was initially placed in staging as it was needed for a
special
project, which is the commit that Colin used. As the patch subject
states, the
driver was later moved to the regular wireless tree.

That break is required, thus ACKed-by: Larry Finger
[off-list ref]
Why not remove this entirely and use the generic routine in
drivers/net/wireless/realtek/rtlwifi/base.c?

Is there a real difference?
I did not see any difference other than the removal of a bunch of
magic numbers
and better formatting.
Me neither.
Colin,

Do you want to push the new patch removing the duplicate routine from
rtl8821ae?
Indeed. Sent.
Larry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help