* [Patch v3 1/4] Staging: vt6655: Limit return statements.
2020-04-01 18:03 [Patch v3 0/4] Staging: vt6655: Simplify statement logic Briana Oursler
@ 2020-04-01 18:03 ` Briana Oursler
2020-04-01 18:03 ` [Patch v3 2/4] Staging: vt6655: Move rate determination logic Briana Oursler
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Briana Oursler @ 2020-04-01 18:03 UTC (permalink / raw)
To: sbrivio, julia.lawall, gregkh, forest, outreachy-kernel; +Cc: Briana Oursler
Limit return statements within context of switch case to improve
readability.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/staging/vt6655/rxtx.c | 29 +++++++++--------------------
1 file changed, 9 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 37fcc42ed000..443d04c4c62f 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -259,12 +259,9 @@ s_uGetDataDuration(
else
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wRate, bNeedAck);
- if (bNeedAck) {
+ if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
- } else {
- return pDevice->uSIFS + uNextPktTime;
- }
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -282,12 +279,9 @@ s_uGetDataDuration(
else
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wRate, bNeedAck);
- if (bNeedAck) {
+ if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
- } else {
- return pDevice->uSIFS + uNextPktTime;
- }
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -323,12 +317,9 @@ s_uGetDataDuration(
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
}
- if (bNeedAck) {
+ if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
- } else {
- return pDevice->uSIFS + uNextPktTime;
- }
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -363,12 +354,10 @@ s_uGetDataDuration(
else
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
}
- if (bNeedAck) {
+
+ if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
- } else {
- return pDevice->uSIFS + uNextPktTime;
- }
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Patch v3 2/4] Staging: vt6655: Move rate determination logic.
2020-04-01 18:03 [Patch v3 0/4] Staging: vt6655: Simplify statement logic Briana Oursler
2020-04-01 18:03 ` [Patch v3 1/4] Staging: vt6655: Limit return statements Briana Oursler
@ 2020-04-01 18:03 ` Briana Oursler
2020-04-01 18:03 ` [Patch v3 3/4] Staging: vt6655: Eliminate nested if else Briana Oursler
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Briana Oursler @ 2020-04-01 18:03 UTC (permalink / raw)
To: sbrivio, julia.lawall, gregkh, forest, outreachy-kernel; +Cc: Briana Oursler
Factor rate setting logic out of nested if-else statement to prevent
code duplication.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/staging/vt6655/rxtx.c | 30 ++++++++++--------------------
1 file changed, 10 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 443d04c4c62f..0fbbe81bc2ca 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -294,23 +294,18 @@ s_uGetDataDuration(
return 0;
}
} else { /* First Frag or Mid Frag */
- if (byFBOption == AUTO_FB_0) {
- if (wRate < RATE_18M)
- wRate = RATE_18M;
- else if (wRate > RATE_54M)
- wRate = RATE_54M;
+ if (wRate < RATE_18M)
+ wRate = RATE_18M;
+ else if (wRate > RATE_54M)
+ wRate = RATE_54M;
+ if (byFBOption == AUTO_FB_0) {
if (uFragIdx == (uMACfragNum - 2))
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
else
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
} else { /* (byFBOption == AUTO_FB_1) */
- if (wRate < RATE_18M)
- wRate = RATE_18M;
- else if (wRate > RATE_54M)
- wRate = RATE_54M;
-
if (uFragIdx == (uMACfragNum - 2))
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
else
@@ -332,23 +327,18 @@ s_uGetDataDuration(
return 0;
}
} else { /* First Frag or Mid Frag */
- if (byFBOption == AUTO_FB_0) {
- if (wRate < RATE_18M)
- wRate = RATE_18M;
- else if (wRate > RATE_54M)
- wRate = RATE_54M;
+ if (wRate < RATE_18M)
+ wRate = RATE_18M;
+ else if (wRate > RATE_54M)
+ wRate = RATE_54M;
+ if (byFBOption == AUTO_FB_0) {
if (uFragIdx == (uMACfragNum - 2))
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt0[FB_RATE1][wRate-RATE_18M], bNeedAck);
else
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt0[FB_RATE1][wRate-RATE_18M], bNeedAck);
} else { /* (byFBOption == AUTO_FB_1) */
- if (wRate < RATE_18M)
- wRate = RATE_18M;
- else if (wRate > RATE_54M)
- wRate = RATE_54M;
-
if (uFragIdx == (uMACfragNum - 2))
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
else
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Patch v3 3/4] Staging: vt6655: Eliminate nested if else
2020-04-01 18:03 [Patch v3 0/4] Staging: vt6655: Simplify statement logic Briana Oursler
2020-04-01 18:03 ` [Patch v3 1/4] Staging: vt6655: Limit return statements Briana Oursler
2020-04-01 18:03 ` [Patch v3 2/4] Staging: vt6655: Move rate determination logic Briana Oursler
@ 2020-04-01 18:03 ` Briana Oursler
2020-04-01 18:03 ` [Patch v3 4/4] Staging: vt6655: Format long lines Briana Oursler
2020-04-01 21:10 ` [Outreachy kernel] [Patch v3 0/4] Staging: vt6655: Simplify statement logic Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: Briana Oursler @ 2020-04-01 18:03 UTC (permalink / raw)
To: sbrivio, julia.lawall, gregkh, forest, outreachy-kernel; +Cc: Briana Oursler
Eliminate nested if else statement, reduce code duplication, and
shorten long lines by creating a new variable, len, to determine
function input needed for s_uGetTxRsvTime.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
drivers/staging/vt6655/rxtx.c | 43 ++++++++++++-----------------------
1 file changed, 14 insertions(+), 29 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 0fbbe81bc2ca..0548c48385b9 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -239,11 +239,16 @@ s_uGetDataDuration(
)
{
bool bLastFrag = false;
- unsigned int uAckTime = 0, uNextPktTime = 0;
+ unsigned int uAckTime = 0, uNextPktTime = 0, len;
if (uFragIdx == (uMACfragNum - 1))
bLastFrag = true;
+ if (uFragIdx == (uMACfragNum - 2))
+ len = cbLastFragmentSize;
+ else
+ len = cbFrameLength;
+
switch (byDurType) {
case DATADUR_B: /* DATADUR_B */
if (((uMACfragNum == 1)) || bLastFrag) {/* Non Frag or Last Frag */
@@ -254,10 +259,7 @@ s_uGetDataDuration(
return 0;
}
} else {/* First Frag or Mid Frag */
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wRate, bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wRate, bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wRate, bNeedAck);
if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
@@ -274,10 +276,7 @@ s_uGetDataDuration(
return 0;
}
} else {/* First Frag or Mid Frag */
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wRate, bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wRate, bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wRate, bNeedAck);
if (bNeedAck)
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
@@ -300,16 +299,9 @@ s_uGetDataDuration(
wRate = RATE_54M;
if (byFBOption == AUTO_FB_0) {
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
-
- } else { /* (byFBOption == AUTO_FB_1) */
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ } else {
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
}
if (bNeedAck)
@@ -333,16 +325,9 @@ s_uGetDataDuration(
wRate = RATE_54M;
if (byFBOption == AUTO_FB_0) {
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt0[FB_RATE1][wRate-RATE_18M], bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt0[FB_RATE1][wRate-RATE_18M], bNeedAck);
-
- } else { /* (byFBOption == AUTO_FB_1) */
- if (uFragIdx == (uMACfragNum - 2))
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbLastFragmentSize, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
- else
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ } else {
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
}
if (bNeedAck)
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Patch v3 4/4] Staging: vt6655: Format long lines.
2020-04-01 18:03 [Patch v3 0/4] Staging: vt6655: Simplify statement logic Briana Oursler
` (2 preceding siblings ...)
2020-04-01 18:03 ` [Patch v3 3/4] Staging: vt6655: Eliminate nested if else Briana Oursler
@ 2020-04-01 18:03 ` Briana Oursler
2020-04-01 21:10 ` [Outreachy kernel] [Patch v3 0/4] Staging: vt6655: Simplify statement logic Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: Briana Oursler @ 2020-04-01 18:03 UTC (permalink / raw)
To: sbrivio, julia.lawall, gregkh, forest, outreachy-kernel; +Cc: Briana Oursler
Add whitespace around '-' operator and wrap long lines. Issue found by
checkpatch.pl.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
---
Changes v3:
- Add curly brackets around if-statement.
drivers/staging/vt6655/rxtx.c | 51 ++++++++++++++++++++++++++---------
1 file changed, 38 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 0548c48385b9..f142dd26f111 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -261,8 +261,11 @@ s_uGetDataDuration(
} else {/* First Frag or Mid Frag */
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wRate, bNeedAck);
- if (bNeedAck)
- uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
+ if (bNeedAck) {
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopCCKBasicRate);
+ }
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -276,10 +279,14 @@ s_uGetDataDuration(
return 0;
}
} else {/* First Frag or Mid Frag */
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wRate, bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len,
+ wRate, bNeedAck);
- if (bNeedAck)
- uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
+ if (bNeedAck) {
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
+ }
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -299,13 +306,22 @@ s_uGetDataDuration(
wRate = RATE_54M;
if (byFBOption == AUTO_FB_0) {
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice,
+ byPktType, len,
+ wFB_Opt0[FB_RATE0][wRate - RATE_18M],
+ bNeedAck);
} else {
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice,
+ byPktType, len,
+ wFB_Opt1[FB_RATE0][wRate - RATE_18M],
+ bNeedAck);
}
- if (bNeedAck)
- uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
+ if (bNeedAck) {
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
+ }
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -325,13 +341,22 @@ s_uGetDataDuration(
wRate = RATE_54M;
if (byFBOption == AUTO_FB_0) {
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt0[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice,
+ byPktType, len,
+ wFB_Opt0[FB_RATE0][wRate - RATE_18M],
+ bNeedAck);
} else {
- uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wFB_Opt1[FB_RATE0][wRate-RATE_18M], bNeedAck);
+ uNextPktTime = s_uGetTxRsvTime(pDevice,
+ byPktType, len,
+ wFB_Opt1[FB_RATE0][wRate - RATE_18M],
+ bNeedAck);
}
- if (bNeedAck)
- uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
+ if (bNeedAck) {
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
+ }
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
--
2.24.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [Outreachy kernel] [Patch v3 0/4] Staging: vt6655: Simplify statement logic.
2020-04-01 18:03 [Patch v3 0/4] Staging: vt6655: Simplify statement logic Briana Oursler
` (3 preceding siblings ...)
2020-04-01 18:03 ` [Patch v3 4/4] Staging: vt6655: Format long lines Briana Oursler
@ 2020-04-01 21:10 ` Stefano Brivio
4 siblings, 0 replies; 6+ messages in thread
From: Stefano Brivio @ 2020-04-01 21:10 UTC (permalink / raw)
To: Briana Oursler; +Cc: julia.lawall, gregkh, forest, outreachy-kernel
On Wed, 1 Apr 2020 11:03:38 -0700
Briana Oursler <briana.oursler@gmail.com> wrote:
> Reduce code duplication and complexity within switch case statement. The
> goal is to improve the readability of the statement so that the outcome
> of the statement is more clear. Checkpatch identified long lines and
> other formatting issues within some nested logic. The last commit in this
> patch series addresses checkpatch warnings that were not fixed by other
> means within the series. Some long lines were left under the rationale
> that removing them rendered the output more confusing.
>
> Briana Oursler (4):
> Staging: vt6655: Limit return statements.
> Staging: vt6655: Move rate determination logic.
> Staging: vt6655: Eliminate nested if else
> Staging: vt6655: Format long lines.
>
> Changes in v3:
> - Patch 1/4 - 3/4:
> - Substance unchanged, add Reviewed-by tag from Stefano
> Brivio.
> - Patch 4/4:
> - Add curly brackets per recommendation of Stefano
> Brivio.
For the series,
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply [flat|nested] 6+ messages in thread