2024-05-26 11:20:53

by Michael Straube

[permalink] [raw]
Subject: [PATCH 0/3] staging: rtl8192e: some refactoring

This series refactors two for loops in _rtl92e_dm_rx_path_sel_byrssi()
to reduce indentation and improve readability.

Compile-tested only, due to lack of hardware.

Michael Straube (3):
staging: rtl8192e: reduce indentation level
staging: rtl8192e: remove unnecessary line breaks
staging: rtl8192e: remove dead code

drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 154 ++++++++++-----------
1 file changed, 71 insertions(+), 83 deletions(-)

--
2.45.1



2024-05-26 11:21:03

by Michael Straube

[permalink] [raw]
Subject: [PATCH 1/3] staging: rtl8192e: reduce indentation level

Reduce indentation level in _rtl92e_dm_rx_path_sel_byrssi() by negating
if statements and use continue in for loops. This way the indentation
level of the code that is covered by the if checks can be reduced. This
improves readability and clears two checkpatch warnings.

WARNING: Too many leading tabs - consider code refactoring

Signed-off-by: Michael Straube <[email protected]>
---
drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 170 +++++++++++----------
1 file changed, 86 insertions(+), 84 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
index aebe67f1a46d..2fda44c5a412 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
@@ -1335,51 +1335,52 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
for (i = 0; i < RF90_PATH_MAX; i++) {
dm_rx_path_sel_table.rf_rssi[i] = priv->stats.rx_rssi_percentage[i];

- if (priv->brfpath_rxenable[i]) {
- rf_num++;
- cur_rf_rssi = dm_rx_path_sel_table.rf_rssi[i];
-
- if (rf_num == 1) {
- max_rssi_index = min_rssi_index = sec_rssi_index = i;
- tmp_max_rssi = tmp_min_rssi = tmp_sec_rssi = cur_rf_rssi;
- } else if (rf_num == 2) {
- if (cur_rf_rssi >= tmp_max_rssi) {
- tmp_max_rssi = cur_rf_rssi;
- max_rssi_index = i;
- } else {
- tmp_sec_rssi = tmp_min_rssi = cur_rf_rssi;
- sec_rssi_index = min_rssi_index = i;
- }
+ if (!priv->brfpath_rxenable[i])
+ continue;
+
+ rf_num++;
+ cur_rf_rssi = dm_rx_path_sel_table.rf_rssi[i];
+
+ if (rf_num == 1) {
+ max_rssi_index = min_rssi_index = sec_rssi_index = i;
+ tmp_max_rssi = tmp_min_rssi = tmp_sec_rssi = cur_rf_rssi;
+ } else if (rf_num == 2) {
+ if (cur_rf_rssi >= tmp_max_rssi) {
+ tmp_max_rssi = cur_rf_rssi;
+ max_rssi_index = i;
} else {
- if (cur_rf_rssi > tmp_max_rssi) {
- tmp_sec_rssi = tmp_max_rssi;
- sec_rssi_index = max_rssi_index;
- tmp_max_rssi = cur_rf_rssi;
- max_rssi_index = i;
- } else if (cur_rf_rssi == tmp_max_rssi) {
- tmp_sec_rssi = cur_rf_rssi;
- sec_rssi_index = i;
- } else if ((cur_rf_rssi < tmp_max_rssi) &&
- (cur_rf_rssi > tmp_sec_rssi)) {
+ tmp_sec_rssi = tmp_min_rssi = cur_rf_rssi;
+ sec_rssi_index = min_rssi_index = i;
+ }
+ } else {
+ if (cur_rf_rssi > tmp_max_rssi) {
+ tmp_sec_rssi = tmp_max_rssi;
+ sec_rssi_index = max_rssi_index;
+ tmp_max_rssi = cur_rf_rssi;
+ max_rssi_index = i;
+ } else if (cur_rf_rssi == tmp_max_rssi) {
+ tmp_sec_rssi = cur_rf_rssi;
+ sec_rssi_index = i;
+ } else if ((cur_rf_rssi < tmp_max_rssi) &&
+ (cur_rf_rssi > tmp_sec_rssi)) {
+ tmp_sec_rssi = cur_rf_rssi;
+ sec_rssi_index = i;
+ } else if (cur_rf_rssi == tmp_sec_rssi) {
+ if (tmp_sec_rssi == tmp_min_rssi) {
tmp_sec_rssi = cur_rf_rssi;
sec_rssi_index = i;
- } else if (cur_rf_rssi == tmp_sec_rssi) {
- if (tmp_sec_rssi == tmp_min_rssi) {
- tmp_sec_rssi = cur_rf_rssi;
- sec_rssi_index = i;
- }
- } else if ((cur_rf_rssi < tmp_sec_rssi) &&
- (cur_rf_rssi > tmp_min_rssi)) {
- ;
- } else if (cur_rf_rssi == tmp_min_rssi) {
- if (tmp_sec_rssi == tmp_min_rssi) {
- tmp_min_rssi = cur_rf_rssi;
- min_rssi_index = i;
- }
- } else if (cur_rf_rssi < tmp_min_rssi) {
+ }
+ } else if ((cur_rf_rssi < tmp_sec_rssi) &&
+ (cur_rf_rssi > tmp_min_rssi)) {
+ ;
+ } else if (cur_rf_rssi == tmp_min_rssi) {
+ if (tmp_sec_rssi == tmp_min_rssi) {
tmp_min_rssi = cur_rf_rssi;
min_rssi_index = i;
}
+ } else if (cur_rf_rssi < tmp_min_rssi) {
+ tmp_min_rssi = cur_rf_rssi;
+ min_rssi_index = i;
}
}
}
@@ -1387,59 +1388,60 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
rf_num = 0;
if (dm_rx_path_sel_table.cck_method == CCK_Rx_Version_2) {
for (i = 0; i < RF90_PATH_MAX; i++) {
- if (priv->brfpath_rxenable[i]) {
- rf_num++;
- cur_cck_pwdb =
- dm_rx_path_sel_table.cck_pwdb_sta[i];
+ if (!priv->brfpath_rxenable[i])
+ continue;

- if (rf_num == 1) {
+ rf_num++;
+ cur_cck_pwdb =
+ dm_rx_path_sel_table.cck_pwdb_sta[i];
+
+ if (rf_num == 1) {
+ cck_rx_ver2_max_index = i;
+ cck_rx_ver2_sec_index = i;
+ tmp_cck_max_pwdb = cur_cck_pwdb;
+ tmp_cck_min_pwdb = cur_cck_pwdb;
+ tmp_cck_sec_pwdb = cur_cck_pwdb;
+ } else if (rf_num == 2) {
+ if (cur_cck_pwdb >= tmp_cck_max_pwdb) {
+ tmp_cck_max_pwdb = cur_cck_pwdb;
cck_rx_ver2_max_index = i;
+ } else {
+ tmp_cck_sec_pwdb = cur_cck_pwdb;
+ tmp_cck_min_pwdb = cur_cck_pwdb;
cck_rx_ver2_sec_index = i;
+ }
+ } else {
+ if (cur_cck_pwdb > tmp_cck_max_pwdb) {
+ tmp_cck_sec_pwdb =
+ tmp_cck_max_pwdb;
+ cck_rx_ver2_sec_index =
+ cck_rx_ver2_max_index;
tmp_cck_max_pwdb = cur_cck_pwdb;
- tmp_cck_min_pwdb = cur_cck_pwdb;
+ cck_rx_ver2_max_index = i;
+ } else if (cur_cck_pwdb ==
+ tmp_cck_max_pwdb) {
tmp_cck_sec_pwdb = cur_cck_pwdb;
- } else if (rf_num == 2) {
- if (cur_cck_pwdb >= tmp_cck_max_pwdb) {
- tmp_cck_max_pwdb = cur_cck_pwdb;
- cck_rx_ver2_max_index = i;
- } else {
- tmp_cck_sec_pwdb = cur_cck_pwdb;
- tmp_cck_min_pwdb = cur_cck_pwdb;
- cck_rx_ver2_sec_index = i;
- }
- } else {
- if (cur_cck_pwdb > tmp_cck_max_pwdb) {
+ cck_rx_ver2_sec_index = i;
+ } else if (PWDB_IN_RANGE) {
+ tmp_cck_sec_pwdb = cur_cck_pwdb;
+ cck_rx_ver2_sec_index = i;
+ } else if (cur_cck_pwdb ==
+ tmp_cck_sec_pwdb) {
+ if (tmp_cck_sec_pwdb ==
+ tmp_cck_min_pwdb) {
tmp_cck_sec_pwdb =
- tmp_cck_max_pwdb;
+ cur_cck_pwdb;
cck_rx_ver2_sec_index =
- cck_rx_ver2_max_index;
- tmp_cck_max_pwdb = cur_cck_pwdb;
- cck_rx_ver2_max_index = i;
- } else if (cur_cck_pwdb ==
- tmp_cck_max_pwdb) {
- tmp_cck_sec_pwdb = cur_cck_pwdb;
- cck_rx_ver2_sec_index = i;
- } else if (PWDB_IN_RANGE) {
- tmp_cck_sec_pwdb = cur_cck_pwdb;
- cck_rx_ver2_sec_index = i;
- } else if (cur_cck_pwdb ==
- tmp_cck_sec_pwdb) {
- if (tmp_cck_sec_pwdb ==
- tmp_cck_min_pwdb) {
- tmp_cck_sec_pwdb =
- cur_cck_pwdb;
- cck_rx_ver2_sec_index =
- i;
- }
- } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
- (cur_cck_pwdb > tmp_cck_min_pwdb)) {
- ;
- } else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
- if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
- tmp_cck_min_pwdb = cur_cck_pwdb;
- } else if (cur_cck_pwdb < tmp_cck_min_pwdb) {
- tmp_cck_min_pwdb = cur_cck_pwdb;
+ i;
}
+ } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
+ (cur_cck_pwdb > tmp_cck_min_pwdb)) {
+ ;
+ } else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
+ if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
+ tmp_cck_min_pwdb = cur_cck_pwdb;
+ } else if (cur_cck_pwdb < tmp_cck_min_pwdb) {
+ tmp_cck_min_pwdb = cur_cck_pwdb;
}
}
}
--
2.45.1


2024-05-26 11:21:05

by Michael Straube

[permalink] [raw]
Subject: [PATCH 2/3] staging: rtl8192e: remove unnecessary line breaks

Remove some unnecessary line breaks after '=' and '==' to improve
readability.

Signed-off-by: Michael Straube <[email protected]>
---
drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 24 ++++++++--------------
1 file changed, 8 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
index 2fda44c5a412..5392d2daf870 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
@@ -1392,8 +1392,7 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
continue;

rf_num++;
- cur_cck_pwdb =
- dm_rx_path_sel_table.cck_pwdb_sta[i];
+ cur_cck_pwdb = dm_rx_path_sel_table.cck_pwdb_sta[i];

if (rf_num == 1) {
cck_rx_ver2_max_index = i;
@@ -1412,27 +1411,20 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
}
} else {
if (cur_cck_pwdb > tmp_cck_max_pwdb) {
- tmp_cck_sec_pwdb =
- tmp_cck_max_pwdb;
- cck_rx_ver2_sec_index =
- cck_rx_ver2_max_index;
+ tmp_cck_sec_pwdb = tmp_cck_max_pwdb;
+ cck_rx_ver2_sec_index = cck_rx_ver2_max_index;
tmp_cck_max_pwdb = cur_cck_pwdb;
cck_rx_ver2_max_index = i;
- } else if (cur_cck_pwdb ==
- tmp_cck_max_pwdb) {
+ } else if (cur_cck_pwdb == tmp_cck_max_pwdb) {
tmp_cck_sec_pwdb = cur_cck_pwdb;
cck_rx_ver2_sec_index = i;
} else if (PWDB_IN_RANGE) {
tmp_cck_sec_pwdb = cur_cck_pwdb;
cck_rx_ver2_sec_index = i;
- } else if (cur_cck_pwdb ==
- tmp_cck_sec_pwdb) {
- if (tmp_cck_sec_pwdb ==
- tmp_cck_min_pwdb) {
- tmp_cck_sec_pwdb =
- cur_cck_pwdb;
- cck_rx_ver2_sec_index =
- i;
+ } else if (cur_cck_pwdb == tmp_cck_sec_pwdb) {
+ if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb) {
+ tmp_cck_sec_pwdb = cur_cck_pwdb;
+ cck_rx_ver2_sec_index = i;
}
} else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
(cur_cck_pwdb > tmp_cck_min_pwdb)) {
--
2.45.1


2024-05-26 11:21:20

by Michael Straube

[permalink] [raw]
Subject: [PATCH 3/3] staging: rtl8192e: remove dead code

Remove two else-if arms that do nothing.

Signed-off-by: Michael Straube <[email protected]>
---
drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 6 ------
1 file changed, 6 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
index 5392d2daf870..4e03eb100175 100644
--- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
+++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
@@ -1370,9 +1370,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
tmp_sec_rssi = cur_rf_rssi;
sec_rssi_index = i;
}
- } else if ((cur_rf_rssi < tmp_sec_rssi) &&
- (cur_rf_rssi > tmp_min_rssi)) {
- ;
} else if (cur_rf_rssi == tmp_min_rssi) {
if (tmp_sec_rssi == tmp_min_rssi) {
tmp_min_rssi = cur_rf_rssi;
@@ -1426,9 +1423,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
tmp_cck_sec_pwdb = cur_cck_pwdb;
cck_rx_ver2_sec_index = i;
}
- } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
- (cur_cck_pwdb > tmp_cck_min_pwdb)) {
- ;
} else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
tmp_cck_min_pwdb = cur_cck_pwdb;
--
2.45.1


2024-05-26 14:23:29

by Nam Cao

[permalink] [raw]
Subject: Re: [PATCH 1/3] staging: rtl8192e: reduce indentation level

On Sun, May 26, 2024 at 01:19:26PM +0200, Michael Straube wrote:
> Reduce indentation level in _rtl92e_dm_rx_path_sel_byrssi() by negating
> if statements and use continue in for loops. This way the indentation
> level of the code that is covered by the if checks can be reduced. This
> improves readability and clears two checkpatch warnings.
>
> WARNING: Too many leading tabs - consider code refactoring
>
> Signed-off-by: Michael Straube <[email protected]>

I don't see any functional change. This patch does as described:

Reviewed-by: Nam Cao <[email protected]>

Best regards,
Nam

2024-05-26 14:31:16

by Nam Cao

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: rtl8192e: remove dead code

On Sun, May 26, 2024 at 01:19:28PM +0200, Michael Straube wrote:
> Remove two else-if arms that do nothing.
>
> Signed-off-by: Michael Straube <[email protected]>
> ---
> drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> index 5392d2daf870..4e03eb100175 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> @@ -1370,9 +1370,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
> tmp_sec_rssi = cur_rf_rssi;
> sec_rssi_index = i;
> }
> - } else if ((cur_rf_rssi < tmp_sec_rssi) &&
> - (cur_rf_rssi > tmp_min_rssi)) {
> - ;
> } else if (cur_rf_rssi == tmp_min_rssi) {
> if (tmp_sec_rssi == tmp_min_rssi) {
> tmp_min_rssi = cur_rf_rssi;
> @@ -1426,9 +1423,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
> tmp_cck_sec_pwdb = cur_cck_pwdb;
> cck_rx_ver2_sec_index = i;
> }
> - } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
> - (cur_cck_pwdb > tmp_cck_min_pwdb)) {
> - ;
> } else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
> if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
> tmp_cck_min_pwdb = cur_cck_pwdb;

I would be careful with these changes. These else-if do prevent the
execution of the other else-if, so the code do not behave the same anymore.

The only case this patch doesn't change anything functionally is when the
condition of the removed if-else is mutually exclusive with the conditions
of the following if-else. Are you sure this is the case?

Best regards,
Nam

2024-05-26 14:33:54

by Nam Cao

[permalink] [raw]
Subject: Re: [PATCH 2/3] staging: rtl8192e: remove unnecessary line breaks

On Sun, May 26, 2024 at 01:19:27PM +0200, Michael Straube wrote:
> Remove some unnecessary line breaks after '=' and '==' to improve
> readability.
>
> Signed-off-by: Michael Straube <[email protected]>
> ---
> drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 24 ++++++++--------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> index 2fda44c5a412..5392d2daf870 100644
> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
> @@ -1392,8 +1392,7 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
> continue;
>
> rf_num++;
> - cur_cck_pwdb =
> - dm_rx_path_sel_table.cck_pwdb_sta[i];
> + cur_cck_pwdb = dm_rx_path_sel_table.cck_pwdb_sta[i];
>
> if (rf_num == 1) {
> cck_rx_ver2_max_index = i;
> @@ -1412,27 +1411,20 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
> }
> } else {
> if (cur_cck_pwdb > tmp_cck_max_pwdb) {
> - tmp_cck_sec_pwdb =
> - tmp_cck_max_pwdb;
> - cck_rx_ver2_sec_index =
> - cck_rx_ver2_max_index;
> + tmp_cck_sec_pwdb = tmp_cck_max_pwdb;
> + cck_rx_ver2_sec_index = cck_rx_ver2_max_index;
> tmp_cck_max_pwdb = cur_cck_pwdb;
> cck_rx_ver2_max_index = i;
> - } else if (cur_cck_pwdb ==
> - tmp_cck_max_pwdb) {
> + } else if (cur_cck_pwdb == tmp_cck_max_pwdb) {
> tmp_cck_sec_pwdb = cur_cck_pwdb;
> cck_rx_ver2_sec_index = i;
> } else if (PWDB_IN_RANGE) {
> tmp_cck_sec_pwdb = cur_cck_pwdb;
> cck_rx_ver2_sec_index = i;
> - } else if (cur_cck_pwdb ==
> - tmp_cck_sec_pwdb) {
> - if (tmp_cck_sec_pwdb ==
> - tmp_cck_min_pwdb) {
> - tmp_cck_sec_pwdb =
> - cur_cck_pwdb;
> - cck_rx_ver2_sec_index =
> - i;
> + } else if (cur_cck_pwdb == tmp_cck_sec_pwdb) {
> + if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb) {
> + tmp_cck_sec_pwdb = cur_cck_pwdb;
> + cck_rx_ver2_sec_index = i;
> }
> } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
> (cur_cck_pwdb > tmp_cck_min_pwdb)) {

Reviewed-by: Nam Cao <[email protected]>

Best regards,
Nam

2024-05-27 04:39:00

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: rtl8192e: remove dead code

Am 26.05.24 um 16:31 schrieb Nam Cao:
> On Sun, May 26, 2024 at 01:19:28PM +0200, Michael Straube wrote:
>> Remove two else-if arms that do nothing.
>>
>> Signed-off-by: Michael Straube <[email protected]>
>> ---
>> drivers/staging/rtl8192e/rtl8192e/rtl_dm.c | 6 ------
>> 1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
>> index 5392d2daf870..4e03eb100175 100644
>> --- a/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
>> +++ b/drivers/staging/rtl8192e/rtl8192e/rtl_dm.c
>> @@ -1370,9 +1370,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
>> tmp_sec_rssi = cur_rf_rssi;
>> sec_rssi_index = i;
>> }
>> - } else if ((cur_rf_rssi < tmp_sec_rssi) &&
>> - (cur_rf_rssi > tmp_min_rssi)) {
>> - ;
>> } else if (cur_rf_rssi == tmp_min_rssi) {
>> if (tmp_sec_rssi == tmp_min_rssi) {
>> tmp_min_rssi = cur_rf_rssi;
>> @@ -1426,9 +1423,6 @@ static void _rtl92e_dm_rx_path_sel_byrssi(struct net_device *dev)
>> tmp_cck_sec_pwdb = cur_cck_pwdb;
>> cck_rx_ver2_sec_index = i;
>> }
>> - } else if ((cur_cck_pwdb < tmp_cck_sec_pwdb) &&
>> - (cur_cck_pwdb > tmp_cck_min_pwdb)) {
>> - ;
>> } else if (cur_cck_pwdb == tmp_cck_min_pwdb) {
>> if (tmp_cck_sec_pwdb == tmp_cck_min_pwdb)
>> tmp_cck_min_pwdb = cur_cck_pwdb;
>
> I would be careful with these changes. These else-if do prevent the
> execution of the other else-if, so the code do not behave the same anymore.
>
> The only case this patch doesn't change anything functionally is when the
> condition of the removed if-else is mutually exclusive with the conditions
> of the following if-else. Are you sure this is the case?

Ah yes, I had not thought about that. Thanks for pointing out.
I'll have a closer look and resend the series. Either without this patch
or, if it's safe to remove, state it in the commit message.

Thanks,
Michael

2024-05-27 07:39:57

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: rtl8192e: remove dead code

This patch doesn't affect behavior at all, but to me the original
author wrote the do nothing case for readability, and I don't have a
problem with that. In fact, I applaud the author for caring about
readability at all which is not a given in staging code. :P

regards,
dan carpenter


2024-05-27 08:35:15

by Michael Straube

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: rtl8192e: remove dead code

Am 27.05.24 um 09:39 schrieb Dan Carpenter:
> This patch doesn't affect behavior at all, but to me the original
> author wrote the do nothing case for readability, and I don't have a
> problem with that. In fact, I applaud the author for caring about
> readability at all which is not a given in staging code. :P

Then I think it's better to leave it as is. :)
Should I send a v2 with this patched removed or will Greg just apply
the first two patches and ignore this one?

thanks,
Michael


2024-05-27 08:51:47

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH 3/3] staging: rtl8192e: remove dead code

On Mon, May 27, 2024 at 10:34:41AM +0200, Michael Straube wrote:
> Am 27.05.24 um 09:39 schrieb Dan Carpenter:
> > This patch doesn't affect behavior at all, but to me the original
> > author wrote the do nothing case for readability, and I don't have a
> > problem with that. In fact, I applaud the author for caring about
> > readability at all which is not a given in staging code. :P
>
> Then I think it's better to leave it as is. :)
> Should I send a v2 with this patched removed or will Greg just apply
> the first two patches and ignore this one?

Resend with Nam's reviewed-by tags.

regards,
dan carpenter