Remove explicit true/false comparations to bool variables.
Signed-off-by: Luis de Bethencourt <[email protected]>
---
drivers/staging/rtl8192u/r8192U_core.c | 7 ++++---
drivers/staging/rtl8192u/r8192U_dm.c | 21 +++++++++++----------
2 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
index a4795af..c53d670 100644
--- a/drivers/staging/rtl8192u/r8192U_core.c
+++ b/drivers/staging/rtl8192u/r8192U_core.c
@@ -2047,7 +2047,7 @@ static bool GetHalfNmodeSupportByAPs819xUsb(struct net_device *dev)
struct r8192_priv *priv = ieee80211_priv(dev);
struct ieee80211_device *ieee = priv->ieee80211;
- if (ieee->bHalfWirelessN24GMode == true)
+ if (ieee->bHalfWirelessN24GMode)
Reval = true;
else
Reval = false;
@@ -2762,7 +2762,7 @@ static bool rtl8192_adapter_start(struct net_device *dev)
//
#ifdef TO_DO_LIST
if (Adapter->ResetProgress == RESET_TYPE_NORESET) {
- if (pMgntInfo->RegRfOff == true) { /* User disable RF via registry. */
+ if (pMgntInfo->RegRfOff) { /* User disable RF via registry. */
RT_TRACE((COMP_INIT|COMP_RF), DBG_LOUD, ("InitializeAdapter819xUsb(): Turn off RF for RegRfOff ----------\n"));
MgntActSet_RF_State(Adapter, eRfOff, RF_CHANGE_BY_SW);
// Those actions will be discard in MgntActSet_RF_State because of the same state
@@ -4406,7 +4406,8 @@ static void query_rxdesc_status(struct sk_buff *skb,
/* RTL8190 set this bit to indicate that Hw does not decrypt packet */
stats->Decrypted = !desc->SWDec;
- if ((priv->ieee80211->pHTInfo->bCurrentHTSupport == true) && (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
+ if ((priv->ieee80211->pHTInfo->bCurrentHTSupport) &&
+ (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
stats->bHwError = false;
else
stats->bHwError = stats->bCRC|stats->bICV;
diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
index 12dd19e..9946615 100644
--- a/drivers/staging/rtl8192u/r8192U_dm.c
+++ b/drivers/staging/rtl8192u/r8192U_dm.c
@@ -438,7 +438,7 @@ static void dm_bandwidth_autoswitch(struct net_device *dev)
if (priv->CurrentChannelBW == HT_CHANNEL_WIDTH_20 || !priv->ieee80211->bandwidth_auto_switch.bautoswitch_enable)
return;
- if (priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz == false) { /* If send packets in 40 Mhz in 20/40 */
+ if (!priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz) { /* If send packets in 40 Mhz in 20/40 */
if (priv->undecorated_smoothed_pwdb <= priv->ieee80211->bandwidth_auto_switch.threshold_40Mhzto20Mhz)
priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz = true;
} else { /* in force send packets in 20 Mhz in 20/40 */
@@ -563,7 +563,7 @@ static void dm_TXPowerTrackingCallback_TSSI(struct net_device *dev)
break;
}
}
- if (viviflag == true) {
+ if (viviflag) {
write_nic_byte(dev, 0x1ba, 0);
viviflag = false;
RT_TRACE(COMP_POWER_TRACKING, "we filtered the data\n");
@@ -766,7 +766,7 @@ void dm_txpower_trackingcallback(struct work_struct *work)
struct r8192_priv *priv = container_of(dwork, struct r8192_priv, txpower_tracking_wq);
struct net_device *dev = priv->ieee80211->dev;
- if (priv->bDcut == true)
+ if (priv->bDcut)
dm_TXPowerTrackingCallback_TSSI(dev);
else
dm_TXPowerTrackingCallback_ThermalMeter(dev);
@@ -1301,7 +1301,7 @@ void dm_initialize_txpower_tracking(struct net_device *dev)
{
struct r8192_priv *priv = ieee80211_priv(dev);
- if (priv->bDcut == true)
+ if (priv->bDcut)
dm_InitializeTXPowerTracking_TSSI(dev);
else
dm_InitializeTXPowerTracking_ThermalMeter(dev);
@@ -1357,7 +1357,7 @@ static void dm_check_txpower_tracking(struct net_device *dev)
#ifdef RTL8190P
dm_CheckTXPowerTracking_TSSI(dev);
#else
- if (priv->bDcut == true)
+ if (priv->bDcut)
dm_CheckTXPowerTracking_TSSI(dev);
else
dm_CheckTXPowerTracking_ThermalMeter(dev);
@@ -1467,7 +1467,7 @@ void dm_cck_txpower_adjust(struct net_device *dev, bool binch14)
{ /* dm_CCKTxPowerAdjust */
struct r8192_priv *priv = ieee80211_priv(dev);
- if (priv->bDcut == true)
+ if (priv->bDcut)
dm_CCKTxPowerAdjust_TSSI(dev, binch14);
else
dm_CCKTxPowerAdjust_ThermalMeter(dev, binch14);
@@ -1731,7 +1731,7 @@ static void dm_dig_init(struct net_device *dev)
*---------------------------------------------------------------------------*/
static void dm_ctrl_initgain_byrssi(struct net_device *dev)
{
- if (dm_digtable.dig_enable_flag == false)
+ if (!dm_digtable.dig_enable_flag)
return;
if (dm_digtable.dig_algorithm == DIG_ALGO_BY_FALSE_ALARM)
@@ -1750,7 +1750,7 @@ static void dm_ctrl_initgain_byrssi_by_driverrssi(
u8 i;
static u8 fw_dig;
- if (dm_digtable.dig_enable_flag == false)
+ if (!dm_digtable.dig_enable_flag)
return;
/*DbgPrint("Dig by Sw Rssi\n");*/
@@ -1792,7 +1792,7 @@ static void dm_ctrl_initgain_byrssi_by_fwfalse_alarm(
static u32 reset_cnt;
u8 i;
- if (dm_digtable.dig_enable_flag == false)
+ if (!dm_digtable.dig_enable_flag)
return;
if (dm_digtable.dig_algorithm_switch) {
@@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
priv->bDynamicTxLowPower = false;
} else {
/* high power state check */
- if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
+ if (priv->undecorated_smoothed_pwdb <
+ txlowpower_threshold && priv->bDynamicTxHighPower)
priv->bDynamicTxHighPower = false;
/* low power state check */
--
2.1.4
On Tue, Jun 23, 2015 at 2:52 PM, Luis de Bethencourt
<[email protected]> wrote:
> Remove explicit true/false comparations to bool variables.
>
> Signed-off-by: Luis de Bethencourt <[email protected]>
> ---
> drivers/staging/rtl8192u/r8192U_core.c | 7 ++++---
> drivers/staging/rtl8192u/r8192U_dm.c | 21 +++++++++++----------
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index a4795af..c53d670 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -2047,7 +2047,7 @@ static bool GetHalfNmodeSupportByAPs819xUsb(struct net_device *dev)
> struct r8192_priv *priv = ieee80211_priv(dev);
> struct ieee80211_device *ieee = priv->ieee80211;
>
> - if (ieee->bHalfWirelessN24GMode == true)
> + if (ieee->bHalfWirelessN24GMode)
> Reval = true;
> else
> Reval = false;
With this one I'd go as far as saying that
Reval = ieee->bHalfWirelessN24GMode;
Frans
On Tue, Jun 23, 2015 at 2:52 PM, Luis de Bethencourt
<[email protected]> wrote:
> Remove explicit true/false comparations to bool variables.
>
> Signed-off-by: Luis de Bethencourt <[email protected]>
> ---
> drivers/staging/rtl8192u/r8192U_core.c | 7 ++++---
> drivers/staging/rtl8192u/r8192U_dm.c | 21 +++++++++++----------
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> index a4795af..c53d670 100644
> --- a/drivers/staging/rtl8192u/r8192U_core.c
> +++ b/drivers/staging/rtl8192u/r8192U_core.c
> @@ -2047,7 +2047,7 @@ static bool GetHalfNmodeSupportByAPs819xUsb(struct net_device *dev)
> struct r8192_priv *priv = ieee80211_priv(dev);
> struct ieee80211_device *ieee = priv->ieee80211;
>
> - if (ieee->bHalfWirelessN24GMode == true)
> + if (ieee->bHalfWirelessN24GMode)
> Reval = true;
> else
> Reval = false;
> @@ -2762,7 +2762,7 @@ static bool rtl8192_adapter_start(struct net_device *dev)
> //
> #ifdef TO_DO_LIST
> if (Adapter->ResetProgress == RESET_TYPE_NORESET) {
> - if (pMgntInfo->RegRfOff == true) { /* User disable RF via registry. */
> + if (pMgntInfo->RegRfOff) { /* User disable RF via registry. */
> RT_TRACE((COMP_INIT|COMP_RF), DBG_LOUD, ("InitializeAdapter819xUsb(): Turn off RF for RegRfOff ----------\n"));
> MgntActSet_RF_State(Adapter, eRfOff, RF_CHANGE_BY_SW);
> // Those actions will be discard in MgntActSet_RF_State because of the same state
> @@ -4406,7 +4406,8 @@ static void query_rxdesc_status(struct sk_buff *skb,
> /* RTL8190 set this bit to indicate that Hw does not decrypt packet */
> stats->Decrypted = !desc->SWDec;
>
> - if ((priv->ieee80211->pHTInfo->bCurrentHTSupport == true) && (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
> + if ((priv->ieee80211->pHTInfo->bCurrentHTSupport) &&
> + (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
> stats->bHwError = false;
> else
> stats->bHwError = stats->bCRC|stats->bICV;
> diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> index 12dd19e..9946615 100644
> --- a/drivers/staging/rtl8192u/r8192U_dm.c
> +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> @@ -438,7 +438,7 @@ static void dm_bandwidth_autoswitch(struct net_device *dev)
>
> if (priv->CurrentChannelBW == HT_CHANNEL_WIDTH_20 || !priv->ieee80211->bandwidth_auto_switch.bautoswitch_enable)
> return;
> - if (priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz == false) { /* If send packets in 40 Mhz in 20/40 */
> + if (!priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz) { /* If send packets in 40 Mhz in 20/40 */
> if (priv->undecorated_smoothed_pwdb <= priv->ieee80211->bandwidth_auto_switch.threshold_40Mhzto20Mhz)
> priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz = true;
> } else { /* in force send packets in 20 Mhz in 20/40 */
> @@ -563,7 +563,7 @@ static void dm_TXPowerTrackingCallback_TSSI(struct net_device *dev)
> break;
> }
> }
> - if (viviflag == true) {
> + if (viviflag) {
> write_nic_byte(dev, 0x1ba, 0);
> viviflag = false;
> RT_TRACE(COMP_POWER_TRACKING, "we filtered the data\n");
> @@ -766,7 +766,7 @@ void dm_txpower_trackingcallback(struct work_struct *work)
> struct r8192_priv *priv = container_of(dwork, struct r8192_priv, txpower_tracking_wq);
> struct net_device *dev = priv->ieee80211->dev;
>
> - if (priv->bDcut == true)
> + if (priv->bDcut)
> dm_TXPowerTrackingCallback_TSSI(dev);
> else
> dm_TXPowerTrackingCallback_ThermalMeter(dev);
> @@ -1301,7 +1301,7 @@ void dm_initialize_txpower_tracking(struct net_device *dev)
> {
> struct r8192_priv *priv = ieee80211_priv(dev);
>
> - if (priv->bDcut == true)
> + if (priv->bDcut)
> dm_InitializeTXPowerTracking_TSSI(dev);
> else
> dm_InitializeTXPowerTracking_ThermalMeter(dev);
> @@ -1357,7 +1357,7 @@ static void dm_check_txpower_tracking(struct net_device *dev)
> #ifdef RTL8190P
> dm_CheckTXPowerTracking_TSSI(dev);
> #else
> - if (priv->bDcut == true)
> + if (priv->bDcut)
> dm_CheckTXPowerTracking_TSSI(dev);
> else
> dm_CheckTXPowerTracking_ThermalMeter(dev);
> @@ -1467,7 +1467,7 @@ void dm_cck_txpower_adjust(struct net_device *dev, bool binch14)
> { /* dm_CCKTxPowerAdjust */
> struct r8192_priv *priv = ieee80211_priv(dev);
>
> - if (priv->bDcut == true)
> + if (priv->bDcut)
> dm_CCKTxPowerAdjust_TSSI(dev, binch14);
> else
> dm_CCKTxPowerAdjust_ThermalMeter(dev, binch14);
> @@ -1731,7 +1731,7 @@ static void dm_dig_init(struct net_device *dev)
> *---------------------------------------------------------------------------*/
> static void dm_ctrl_initgain_byrssi(struct net_device *dev)
> {
> - if (dm_digtable.dig_enable_flag == false)
> + if (!dm_digtable.dig_enable_flag)
> return;
>
> if (dm_digtable.dig_algorithm == DIG_ALGO_BY_FALSE_ALARM)
> @@ -1750,7 +1750,7 @@ static void dm_ctrl_initgain_byrssi_by_driverrssi(
> u8 i;
> static u8 fw_dig;
>
> - if (dm_digtable.dig_enable_flag == false)
> + if (!dm_digtable.dig_enable_flag)
> return;
>
> /*DbgPrint("Dig by Sw Rssi\n");*/
> @@ -1792,7 +1792,7 @@ static void dm_ctrl_initgain_byrssi_by_fwfalse_alarm(
> static u32 reset_cnt;
> u8 i;
>
> - if (dm_digtable.dig_enable_flag == false)
> + if (!dm_digtable.dig_enable_flag)
> return;
>
> if (dm_digtable.dig_algorithm_switch) {
> @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
> priv->bDynamicTxLowPower = false;
> } else {
> /* high power state check */
> - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
> + if (priv->undecorated_smoothed_pwdb <
> + txlowpower_threshold && priv->bDynamicTxHighPower)
> priv->bDynamicTxHighPower = false;
Oh, this has a misleading air hanging over it. It focuses the eyes on
"txlowpower_threshold && priv->bDynamicTxHighPower", while that
probably isn't the intent.
Frans
On Tue, Jun 23, 2015 at 02:56:38PM +0200, Frans Klaver wrote:
> On Tue, Jun 23, 2015 at 2:52 PM, Luis de Bethencourt
> <[email protected]> wrote:
> > Remove explicit true/false comparations to bool variables.
> >
> > Signed-off-by: Luis de Bethencourt <[email protected]>
> > ---
> > drivers/staging/rtl8192u/r8192U_core.c | 7 ++++---
> > drivers/staging/rtl8192u/r8192U_dm.c | 21 +++++++++++----------
> > 2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index a4795af..c53d670 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -2047,7 +2047,7 @@ static bool GetHalfNmodeSupportByAPs819xUsb(struct net_device *dev)
> > struct r8192_priv *priv = ieee80211_priv(dev);
> > struct ieee80211_device *ieee = priv->ieee80211;
> >
> > - if (ieee->bHalfWirelessN24GMode == true)
> > + if (ieee->bHalfWirelessN24GMode)
> > Reval = true;
> > else
> > Reval = false;
>
> With this one I'd go as far as saying that
>
> Reval = ieee->bHalfWirelessN24GMode;
>
> Frans
Completely missed that. Good catch.
Thanks for the review! Sending an updated version now.
Luis
On Tue, Jun 23, 2015 at 03:04:32PM +0200, Frans Klaver wrote:
> On Tue, Jun 23, 2015 at 2:52 PM, Luis de Bethencourt
> <[email protected]> wrote:
> > Remove explicit true/false comparations to bool variables.
> >
> > Signed-off-by: Luis de Bethencourt <[email protected]>
> > ---
> > drivers/staging/rtl8192u/r8192U_core.c | 7 ++++---
> > drivers/staging/rtl8192u/r8192U_dm.c | 21 +++++++++++----------
> > 2 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/staging/rtl8192u/r8192U_core.c b/drivers/staging/rtl8192u/r8192U_core.c
> > index a4795af..c53d670 100644
> > --- a/drivers/staging/rtl8192u/r8192U_core.c
> > +++ b/drivers/staging/rtl8192u/r8192U_core.c
> > @@ -2047,7 +2047,7 @@ static bool GetHalfNmodeSupportByAPs819xUsb(struct net_device *dev)
> > struct r8192_priv *priv = ieee80211_priv(dev);
> > struct ieee80211_device *ieee = priv->ieee80211;
> >
> > - if (ieee->bHalfWirelessN24GMode == true)
> > + if (ieee->bHalfWirelessN24GMode)
> > Reval = true;
> > else
> > Reval = false;
> > @@ -2762,7 +2762,7 @@ static bool rtl8192_adapter_start(struct net_device *dev)
> > //
> > #ifdef TO_DO_LIST
> > if (Adapter->ResetProgress == RESET_TYPE_NORESET) {
> > - if (pMgntInfo->RegRfOff == true) { /* User disable RF via registry. */
> > + if (pMgntInfo->RegRfOff) { /* User disable RF via registry. */
> > RT_TRACE((COMP_INIT|COMP_RF), DBG_LOUD, ("InitializeAdapter819xUsb(): Turn off RF for RegRfOff ----------\n"));
> > MgntActSet_RF_State(Adapter, eRfOff, RF_CHANGE_BY_SW);
> > // Those actions will be discard in MgntActSet_RF_State because of the same state
> > @@ -4406,7 +4406,8 @@ static void query_rxdesc_status(struct sk_buff *skb,
> > /* RTL8190 set this bit to indicate that Hw does not decrypt packet */
> > stats->Decrypted = !desc->SWDec;
> >
> > - if ((priv->ieee80211->pHTInfo->bCurrentHTSupport == true) && (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
> > + if ((priv->ieee80211->pHTInfo->bCurrentHTSupport) &&
> > + (priv->ieee80211->pairwise_key_type == KEY_TYPE_CCMP))
> > stats->bHwError = false;
> > else
> > stats->bHwError = stats->bCRC|stats->bICV;
> > diff --git a/drivers/staging/rtl8192u/r8192U_dm.c b/drivers/staging/rtl8192u/r8192U_dm.c
> > index 12dd19e..9946615 100644
> > --- a/drivers/staging/rtl8192u/r8192U_dm.c
> > +++ b/drivers/staging/rtl8192u/r8192U_dm.c
> > @@ -438,7 +438,7 @@ static void dm_bandwidth_autoswitch(struct net_device *dev)
> >
> > if (priv->CurrentChannelBW == HT_CHANNEL_WIDTH_20 || !priv->ieee80211->bandwidth_auto_switch.bautoswitch_enable)
> > return;
> > - if (priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz == false) { /* If send packets in 40 Mhz in 20/40 */
> > + if (!priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz) { /* If send packets in 40 Mhz in 20/40 */
> > if (priv->undecorated_smoothed_pwdb <= priv->ieee80211->bandwidth_auto_switch.threshold_40Mhzto20Mhz)
> > priv->ieee80211->bandwidth_auto_switch.bforced_tx20Mhz = true;
> > } else { /* in force send packets in 20 Mhz in 20/40 */
> > @@ -563,7 +563,7 @@ static void dm_TXPowerTrackingCallback_TSSI(struct net_device *dev)
> > break;
> > }
> > }
> > - if (viviflag == true) {
> > + if (viviflag) {
> > write_nic_byte(dev, 0x1ba, 0);
> > viviflag = false;
> > RT_TRACE(COMP_POWER_TRACKING, "we filtered the data\n");
> > @@ -766,7 +766,7 @@ void dm_txpower_trackingcallback(struct work_struct *work)
> > struct r8192_priv *priv = container_of(dwork, struct r8192_priv, txpower_tracking_wq);
> > struct net_device *dev = priv->ieee80211->dev;
> >
> > - if (priv->bDcut == true)
> > + if (priv->bDcut)
> > dm_TXPowerTrackingCallback_TSSI(dev);
> > else
> > dm_TXPowerTrackingCallback_ThermalMeter(dev);
> > @@ -1301,7 +1301,7 @@ void dm_initialize_txpower_tracking(struct net_device *dev)
> > {
> > struct r8192_priv *priv = ieee80211_priv(dev);
> >
> > - if (priv->bDcut == true)
> > + if (priv->bDcut)
> > dm_InitializeTXPowerTracking_TSSI(dev);
> > else
> > dm_InitializeTXPowerTracking_ThermalMeter(dev);
> > @@ -1357,7 +1357,7 @@ static void dm_check_txpower_tracking(struct net_device *dev)
> > #ifdef RTL8190P
> > dm_CheckTXPowerTracking_TSSI(dev);
> > #else
> > - if (priv->bDcut == true)
> > + if (priv->bDcut)
> > dm_CheckTXPowerTracking_TSSI(dev);
> > else
> > dm_CheckTXPowerTracking_ThermalMeter(dev);
> > @@ -1467,7 +1467,7 @@ void dm_cck_txpower_adjust(struct net_device *dev, bool binch14)
> > { /* dm_CCKTxPowerAdjust */
> > struct r8192_priv *priv = ieee80211_priv(dev);
> >
> > - if (priv->bDcut == true)
> > + if (priv->bDcut)
> > dm_CCKTxPowerAdjust_TSSI(dev, binch14);
> > else
> > dm_CCKTxPowerAdjust_ThermalMeter(dev, binch14);
> > @@ -1731,7 +1731,7 @@ static void dm_dig_init(struct net_device *dev)
> > *---------------------------------------------------------------------------*/
> > static void dm_ctrl_initgain_byrssi(struct net_device *dev)
> > {
> > - if (dm_digtable.dig_enable_flag == false)
> > + if (!dm_digtable.dig_enable_flag)
> > return;
> >
> > if (dm_digtable.dig_algorithm == DIG_ALGO_BY_FALSE_ALARM)
> > @@ -1750,7 +1750,7 @@ static void dm_ctrl_initgain_byrssi_by_driverrssi(
> > u8 i;
> > static u8 fw_dig;
> >
> > - if (dm_digtable.dig_enable_flag == false)
> > + if (!dm_digtable.dig_enable_flag)
> > return;
> >
> > /*DbgPrint("Dig by Sw Rssi\n");*/
> > @@ -1792,7 +1792,7 @@ static void dm_ctrl_initgain_byrssi_by_fwfalse_alarm(
> > static u32 reset_cnt;
> > u8 i;
> >
> > - if (dm_digtable.dig_enable_flag == false)
> > + if (!dm_digtable.dig_enable_flag)
> > return;
> >
> > if (dm_digtable.dig_algorithm_switch) {
> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
> > priv->bDynamicTxLowPower = false;
> > } else {
> > /* high power state check */
> > - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
> > + if (priv->undecorated_smoothed_pwdb <
> > + txlowpower_threshold && priv->bDynamicTxHighPower)
> > priv->bDynamicTxHighPower = false;
>
> Oh, this has a misleading air hanging over it. It focuses the eyes on
> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
> probably isn't the intent.
>
> Frans
I agree, and wasn't sure what the best way to deal with was.
The following doesn't mislead but goes above 80 characters.
if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
priv->bDynamicTxHighPower == true)
It is better than the original but it doesn't completely fix it.
If this is a better compromise I can update the patch.
Thanks,
Luis
On Tue, Jun 23, 2015 at 3:21 PM, Luis de Bethencourt
<[email protected]> wrote:
>> > if (dm_digtable.dig_algorithm_switch) {
>> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
>> > priv->bDynamicTxLowPower = false;
>> > } else {
>> > /* high power state check */
>> > - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
>> > + if (priv->undecorated_smoothed_pwdb <
>> > + txlowpower_threshold && priv->bDynamicTxHighPower)
>> > priv->bDynamicTxHighPower = false;
>>
>> Oh, this has a misleading air hanging over it. It focuses the eyes on
>> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
>> probably isn't the intent.
>>
>> Frans
>
> I agree, and wasn't sure what the best way to deal with was.
>
> The following doesn't mislead but goes above 80 characters.
> if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
> priv->bDynamicTxHighPower == true)
>
> It is better than the original but it doesn't completely fix it.
>
> If this is a better compromise I can update the patch.
If we keep people's internal parsers working properly, I think having
a line of three characters too long is a fair compromise. Besides
that, there are a lot more lines of code in that file that need to be
brought back to under 80 characters.
If you really care about that line length, precede with a patch (or
two) that changes those insanely long (local!) variable names, so that
you can break up the line right away.
Have fun,
Frans
On Tue, Jun 23, 2015 at 03:37:20PM +0200, Frans Klaver wrote:
> On Tue, Jun 23, 2015 at 3:21 PM, Luis de Bethencourt
> <[email protected]> wrote:
>
> >> > if (dm_digtable.dig_algorithm_switch) {
> >> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
> >> > priv->bDynamicTxLowPower = false;
> >> > } else {
> >> > /* high power state check */
> >> > - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
> >> > + if (priv->undecorated_smoothed_pwdb <
> >> > + txlowpower_threshold && priv->bDynamicTxHighPower)
> >> > priv->bDynamicTxHighPower = false;
> >>
> >> Oh, this has a misleading air hanging over it. It focuses the eyes on
> >> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
> >> probably isn't the intent.
> >>
> >> Frans
> >
> > I agree, and wasn't sure what the best way to deal with was.
> >
> > The following doesn't mislead but goes above 80 characters.
> > if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
> > priv->bDynamicTxHighPower == true)
> >
> > It is better than the original but it doesn't completely fix it.
> >
> > If this is a better compromise I can update the patch.
>
> If we keep people's internal parsers working properly, I think having
> a line of three characters too long is a fair compromise. Besides
> that, there are a lot more lines of code in that file that need to be
> brought back to under 80 characters.
>
> If you really care about that line length, precede with a patch (or
> two) that changes those insanely long (local!) variable names, so that
> you can break up the line right away.
>
> Have fun,
> Frans
Very true. There are a *lot* of massively long lines.
This has been a learning experience. I wasn't sure how strict the rules for
submissions were.
There are other things besides line lengths that I want to fix in
rtl8192u. Related to that, I just sent a 3rd version which includes fixes for
these bool comparisons for the rest of the files in drivers/staging/rtl8192u/
Thanks so much for taking the time to review. Appreciated.
Luis
On Tue, Jun 23, 2015 at 3:59 PM, Luis de Bethencourt
<[email protected]> wrote:
> On Tue, Jun 23, 2015 at 03:37:20PM +0200, Frans Klaver wrote:
>> On Tue, Jun 23, 2015 at 3:21 PM, Luis de Bethencourt
>> <[email protected]> wrote:
>>
>> >> > if (dm_digtable.dig_algorithm_switch) {
>> >> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
>> >> > priv->bDynamicTxLowPower = false;
>> >> > } else {
>> >> > /* high power state check */
>> >> > - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
>> >> > + if (priv->undecorated_smoothed_pwdb <
>> >> > + txlowpower_threshold && priv->bDynamicTxHighPower)
>> >> > priv->bDynamicTxHighPower = false;
>> >>
>> >> Oh, this has a misleading air hanging over it. It focuses the eyes on
>> >> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
>> >> probably isn't the intent.
>> >>
>> >> Frans
>> >
>> > I agree, and wasn't sure what the best way to deal with was.
>> >
>> > The following doesn't mislead but goes above 80 characters.
>> > if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
>> > priv->bDynamicTxHighPower == true)
>> >
>> > It is better than the original but it doesn't completely fix it.
>> >
>> > If this is a better compromise I can update the patch.
>>
>> If we keep people's internal parsers working properly, I think having
>> a line of three characters too long is a fair compromise. Besides
>> that, there are a lot more lines of code in that file that need to be
>> brought back to under 80 characters.
>>
>> If you really care about that line length, precede with a patch (or
>> two) that changes those insanely long (local!) variable names, so that
>> you can break up the line right away.
>>
>> Have fun,
>> Frans
>
> Very true. There are a *lot* of massively long lines.
>
> This has been a learning experience. I wasn't sure how strict the rules for
> submissions were.
Well, as far as I know "Don't break internal parsers" wins over
"Checkpatch complains". However, checkpatch usually does have a nose
for smelly code (as does sparse, btw), so it pays to look around a bit
if it complains. In the end the maintainer decides whether a patch
passes the criteria.
> There are other things besides line lengths that I want to fix in
> rtl8192u. Related to that, I just sent a 3rd version which includes fixes for
> these bool comparisons for the rest of the files in drivers/staging/rtl8192u/
>
> Thanks so much for taking the time to review. Appreciated.
No problem. Was waiting for a yocto build to finish anyway.
Frans
On Tue, Jun 23, 2015 at 03:59:41PM +0200, Frans Klaver wrote:
> On Tue, Jun 23, 2015 at 3:59 PM, Luis de Bethencourt
> <[email protected]> wrote:
> > On Tue, Jun 23, 2015 at 03:37:20PM +0200, Frans Klaver wrote:
> >> On Tue, Jun 23, 2015 at 3:21 PM, Luis de Bethencourt
> >> <[email protected]> wrote:
> >>
> >> >> > if (dm_digtable.dig_algorithm_switch) {
> >> >> > @@ -3062,7 +3062,8 @@ static void dm_dynamic_txpower(struct net_device *dev)
> >> >> > priv->bDynamicTxLowPower = false;
> >> >> > } else {
> >> >> > /* high power state check */
> >> >> > - if (priv->undecorated_smoothed_pwdb < txlowpower_threshold && priv->bDynamicTxHighPower == true)
> >> >> > + if (priv->undecorated_smoothed_pwdb <
> >> >> > + txlowpower_threshold && priv->bDynamicTxHighPower)
> >> >> > priv->bDynamicTxHighPower = false;
> >> >>
> >> >> Oh, this has a misleading air hanging over it. It focuses the eyes on
> >> >> "txlowpower_threshold && priv->bDynamicTxHighPower", while that
> >> >> probably isn't the intent.
> >> >>
> >> >> Frans
> >> >
> >> > I agree, and wasn't sure what the best way to deal with was.
> >> >
> >> > The following doesn't mislead but goes above 80 characters.
> >> > if (priv->undecorated_smoothed_pwdb < txlowpower_threshold &&
> >> > priv->bDynamicTxHighPower == true)
> >> >
> >> > It is better than the original but it doesn't completely fix it.
> >> >
> >> > If this is a better compromise I can update the patch.
> >>
> >> If we keep people's internal parsers working properly, I think having
> >> a line of three characters too long is a fair compromise. Besides
> >> that, there are a lot more lines of code in that file that need to be
> >> brought back to under 80 characters.
> >>
> >> If you really care about that line length, precede with a patch (or
> >> two) that changes those insanely long (local!) variable names, so that
> >> you can break up the line right away.
> >>
> >> Have fun,
> >> Frans
> >
> > Very true. There are a *lot* of massively long lines.
> >
> > This has been a learning experience. I wasn't sure how strict the rules for
> > submissions were.
>
> Well, as far as I know "Don't break internal parsers" wins over
> "Checkpatch complains". However, checkpatch usually does have a nose
> for smelly code (as does sparse, btw), so it pays to look around a bit
> if it complains. In the end the maintainer decides whether a patch
> passes the criteria.
>
Even if analyzers miss things and give false positives, they are interesting
tools to find interesting code to read and maybe double-check things before
submitting.
> > There are other things besides line lengths that I want to fix in
> > rtl8192u. Related to that, I just sent a 3rd version which includes fixes for
> > these bool comparisons for the rest of the files in drivers/staging/rtl8192u/
> >
> > Thanks so much for taking the time to review. Appreciated.
>
> No problem. Was waiting for a yocto build to finish anyway.
>
> Frans
Have fun with yocto!
Luis