* [PATCH 0/4] staging: vt6655: Simplify statement logic.
@ 2020-03-30 4:47 Briana Oursler
2020-03-30 4:47 ` [PATCH 1/4] Staging: vt6655: Limit return statements Briana Oursler
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Briana Oursler @ 2020-03-30 4:47 UTC (permalink / raw)
To: gregkh, forest, julia.lawall, outreachy-kernel; +Cc: Briana Oursler
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: Raise rate determination logic.
Staging: vt6655: Create new variable len.
Staging: vt6655: Format spaces.
drivers/staging/vt6655/rxtx.c | 118 +++++++++++++++++-----------------
1 file changed, 58 insertions(+), 60 deletions(-)
--
2.24.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] Staging: vt6655: Limit return statements.
2020-03-30 4:47 [PATCH 0/4] staging: vt6655: Simplify statement logic Briana Oursler
@ 2020-03-30 4:47 ` Briana Oursler
2020-03-30 18:01 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 2/4] Staging: vt6655: Raise rate determination logic Briana Oursler
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Briana Oursler @ 2020-03-30 4:47 UTC (permalink / raw)
To: gregkh, forest, julia.lawall, 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>
---
drivers/staging/vt6655/rxtx.c | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 37fcc42ed000..f9e2087d2242 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -261,10 +261,11 @@ s_uGetDataDuration(
if (bNeedAck) {
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
} else {
- return pDevice->uSIFS + uNextPktTime;
+ uAckTime = 0;
}
+
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -284,10 +285,11 @@ s_uGetDataDuration(
if (bNeedAck) {
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
} else {
- return pDevice->uSIFS + uNextPktTime;
+ uAckTime = 0;
}
+
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -325,10 +327,10 @@ s_uGetDataDuration(
if (bNeedAck) {
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
} else {
- return pDevice->uSIFS + uNextPktTime;
+ uAckTime = 0;
}
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -365,10 +367,10 @@ s_uGetDataDuration(
}
if (bNeedAck) {
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
- return pDevice->uSIFS + uAckTime + uNextPktTime;
} else {
- return pDevice->uSIFS + uNextPktTime;
+ uAckTime = 0;
}
+ return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
--
2.24.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] Staging: vt6655: Raise rate determination logic.
2020-03-30 4:47 [PATCH 0/4] staging: vt6655: Simplify statement logic Briana Oursler
2020-03-30 4:47 ` [PATCH 1/4] Staging: vt6655: Limit return statements Briana Oursler
@ 2020-03-30 4:48 ` Briana Oursler
2020-03-30 18:02 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 3/4] Staging: vt6655: Create new variable len Briana Oursler
2020-03-30 4:48 ` [PATCH 4/4] Staging: vt6655: Format spaces Briana Oursler
3 siblings, 1 reply; 9+ messages in thread
From: Briana Oursler @ 2020-03-30 4:48 UTC (permalink / raw)
To: gregkh, forest, julia.lawall, outreachy-kernel; +Cc: Briana Oursler
Raise rate setting logic out of nested if-else statement to prevent code
duplication.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
---
drivers/staging/vt6655/rxtx.c | 26 ++++++--------------------
1 file changed, 6 insertions(+), 20 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index f9e2087d2242..4be867b95d9d 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -302,23 +302,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
@@ -344,27 +339,18 @@ s_uGetDataDuration(
}
} 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 (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
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
}
+
if (bNeedAck) {
uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
} else {
--
2.24.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] Staging: vt6655: Create new variable len.
2020-03-30 4:47 [PATCH 0/4] staging: vt6655: Simplify statement logic Briana Oursler
2020-03-30 4:47 ` [PATCH 1/4] Staging: vt6655: Limit return statements Briana Oursler
2020-03-30 4:48 ` [PATCH 2/4] Staging: vt6655: Raise rate determination logic Briana Oursler
@ 2020-03-30 4:48 ` Briana Oursler
2020-03-30 18:03 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 4/4] Staging: vt6655: Format spaces Briana Oursler
3 siblings, 1 reply; 9+ messages in thread
From: Briana Oursler @ 2020-03-30 4:48 UTC (permalink / raw)
To: gregkh, forest, julia.lawall, outreachy-kernel; +Cc: Briana Oursler
Create variable len to determine function input needed for
s_uGetTxRsvTime. Use len to eliminate a nested if else statement, reduce
code duplication, and shorten long lines found by checkpatch.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
---
drivers/staging/vt6655/rxtx.c | 44 +++++++++++++----------------------
1 file changed, 16 insertions(+), 28 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 4be867b95d9d..6068cf65e3fe 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -240,10 +240,16 @@ s_uGetDataDuration(
{
bool bLastFrag = false;
unsigned int uAckTime = 0, uNextPktTime = 0;
+ unsigned int 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 +260,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);
@@ -278,10 +281,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);
@@ -308,16 +308,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) {
@@ -338,17 +331,11 @@ s_uGetDataDuration(
return 0;
}
} else { /* First Frag or Mid Frag */
+
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) {
@@ -356,6 +343,7 @@ s_uGetDataDuration(
} else {
uAckTime = 0;
}
+
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
--
2.24.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] Staging: vt6655: Format spaces.
2020-03-30 4:47 [PATCH 0/4] staging: vt6655: Simplify statement logic Briana Oursler
` (2 preceding siblings ...)
2020-03-30 4:48 ` [PATCH 3/4] Staging: vt6655: Create new variable len Briana Oursler
@ 2020-03-30 4:48 ` Briana Oursler
2020-03-30 18:02 ` [Outreachy kernel] " Stefano Brivio
3 siblings, 1 reply; 9+ messages in thread
From: Briana Oursler @ 2020-03-30 4:48 UTC (permalink / raw)
To: gregkh, forest, julia.lawall, outreachy-kernel; +Cc: Briana Oursler
Add spaces around '-' operator and format long lines. Formatting issue
found by checkpatch.
Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
---
drivers/staging/vt6655/rxtx.c | 40 +++++++++++++++++++++++++++--------
1 file changed, 31 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
index 6068cf65e3fe..9a6abe8613e6 100644
--- a/drivers/staging/vt6655/rxtx.c
+++ b/drivers/staging/vt6655/rxtx.c
@@ -263,7 +263,9 @@ s_uGetDataDuration(
uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, len, wRate, bNeedAck);
if (bNeedAck) {
- uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopCCKBasicRate);
} else {
uAckTime = 0;
}
@@ -281,10 +283,13 @@ 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);
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
} else {
uAckTime = 0;
}
@@ -308,16 +313,25 @@ 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);
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
} else {
uAckTime = 0;
}
+
return pDevice->uSIFS + uAckTime + uNextPktTime;
}
break;
@@ -333,13 +347,21 @@ s_uGetDataDuration(
} else { /* First Frag or Mid Frag */
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);
+ uAckTime = BBuGetFrameTime(pDevice->byPreambleType,
+ byPktType, 14,
+ pDevice->byTopOFDMBasicRate);
} else {
uAckTime = 0;
}
--
2.24.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 1/4] Staging: vt6655: Limit return statements.
2020-03-30 4:47 ` [PATCH 1/4] Staging: vt6655: Limit return statements Briana Oursler
@ 2020-03-30 18:01 ` Stefano Brivio
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Brivio @ 2020-03-30 18:01 UTC (permalink / raw)
To: Briana Oursler; +Cc: gregkh, forest, julia.lawall, outreachy-kernel
On Sun, 29 Mar 2020 21:47:59 -0700
Briana Oursler <briana.oursler@gmail.com> wrote:
> Limit return statements within context of switch case to improve
> readability.
>
> Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
> ---
> drivers/staging/vt6655/rxtx.c | 18 ++++++++++--------
> 1 file changed, 10 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index 37fcc42ed000..f9e2087d2242 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -261,10 +261,11 @@ s_uGetDataDuration(
>
> if (bNeedAck) {
> uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopCCKBasicRate);
> - return pDevice->uSIFS + uAckTime + uNextPktTime;
> } else {
> - return pDevice->uSIFS + uNextPktTime;
> + uAckTime = 0;
There's no need anymore to set this to zero (it's initialised already),
hence the else clauses can go away, and also the curly brackets. I
consider these as related changes, others might see it differently (but
I guess here it's a relatively safe assumption).
> }
> +
> + return pDevice->uSIFS + uAckTime + uNextPktTime;
> }
> break;
>
> @@ -284,10 +285,11 @@ s_uGetDataDuration(
>
> if (bNeedAck) {
> uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
> - return pDevice->uSIFS + uAckTime + uNextPktTime;
> } else {
> - return pDevice->uSIFS + uNextPktTime;
> + uAckTime = 0;
> }
> +
> + return pDevice->uSIFS + uAckTime + uNextPktTime;
> }
> break;
>
> @@ -325,10 +327,10 @@ s_uGetDataDuration(
>
> if (bNeedAck) {
> uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
> - return pDevice->uSIFS + uAckTime + uNextPktTime;
> } else {
> - return pDevice->uSIFS + uNextPktTime;
> + uAckTime = 0;
> }
> + return pDevice->uSIFS + uAckTime + uNextPktTime;
> }
> break;
>
> @@ -365,10 +367,10 @@ s_uGetDataDuration(
> }
> if (bNeedAck) {
> uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
> - return pDevice->uSIFS + uAckTime + uNextPktTime;
> } else {
> - return pDevice->uSIFS + uNextPktTime;
> + uAckTime = 0;
> }
> + return pDevice->uSIFS + uAckTime + uNextPktTime;
> }
> break;
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 2/4] Staging: vt6655: Raise rate determination logic.
2020-03-30 4:48 ` [PATCH 2/4] Staging: vt6655: Raise rate determination logic Briana Oursler
@ 2020-03-30 18:02 ` Stefano Brivio
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Brivio @ 2020-03-30 18:02 UTC (permalink / raw)
To: Briana Oursler; +Cc: gregkh, forest, julia.lawall, outreachy-kernel
On Sun, 29 Mar 2020 21:48:00 -0700
Briana Oursler <briana.oursler@gmail.com> wrote:
> Raise rate setting logic out of nested if-else statement to prevent code
Again, not a native English speaker, but it's the first time I meet
this usage of "to raise". I would have rather said "move", or "factor
out".
> duplication.
>
> Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
> ---
> drivers/staging/vt6655/rxtx.c | 26 ++++++--------------------
> 1 file changed, 6 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index f9e2087d2242..4be867b95d9d 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -302,23 +302,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
> @@ -344,27 +339,18 @@ s_uGetDataDuration(
> }
> } 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 (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
> uNextPktTime = s_uGetTxRsvTime(pDevice, byPktType, cbFrameLength, wFB_Opt1[FB_RATE1][wRate-RATE_18M], bNeedAck);
> }
> +
This newline is unrelated.
> if (bNeedAck) {
> uAckTime = BBuGetFrameTime(pDevice->byPreambleType, byPktType, 14, pDevice->byTopOFDMBasicRate);
> } else {
--
Stefano
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 4/4] Staging: vt6655: Format spaces.
2020-03-30 4:48 ` [PATCH 4/4] Staging: vt6655: Format spaces Briana Oursler
@ 2020-03-30 18:02 ` Stefano Brivio
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Brivio @ 2020-03-30 18:02 UTC (permalink / raw)
To: Briana Oursler; +Cc: gregkh, forest, julia.lawall, outreachy-kernel
On Sun, 29 Mar 2020 21:48:02 -0700
Briana Oursler <briana.oursler@gmail.com> wrote:
> Add spaces around '-' operator and format long lines. Formatting issue
> found by checkpatch.
Long lines are wrapped, really, and they are not "spaces" as you
reported in the subject.
--
Stefano
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Outreachy kernel] [PATCH 3/4] Staging: vt6655: Create new variable len.
2020-03-30 4:48 ` [PATCH 3/4] Staging: vt6655: Create new variable len Briana Oursler
@ 2020-03-30 18:03 ` Stefano Brivio
0 siblings, 0 replies; 9+ messages in thread
From: Stefano Brivio @ 2020-03-30 18:03 UTC (permalink / raw)
To: Briana Oursler; +Cc: gregkh, forest, julia.lawall, outreachy-kernel
On Sun, 29 Mar 2020 21:48:01 -0700
Briana Oursler <briana.oursler@gmail.com> wrote:
> Create variable len to determine function input needed for
> s_uGetTxRsvTime. Use len to eliminate a nested if else statement, reduce
> code duplication, and shorten long lines found by checkpatch.
The commit description looks okayish to me, except perhaps that the
solution (creating a new variable) shouldn't be stated first. The
commit title is not very representative of the change, though.
>
> Signed-off-by: Briana Oursler <briana.oursler@gmail.com>
> ---
> drivers/staging/vt6655/rxtx.c | 44 +++++++++++++----------------------
> 1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/staging/vt6655/rxtx.c b/drivers/staging/vt6655/rxtx.c
> index 4be867b95d9d..6068cf65e3fe 100644
> --- a/drivers/staging/vt6655/rxtx.c
> +++ b/drivers/staging/vt6655/rxtx.c
> @@ -240,10 +240,16 @@ s_uGetDataDuration(
> {
> bool bLastFrag = false;
> unsigned int uAckTime = 0, uNextPktTime = 0;
> + unsigned int len;
The other two are declared on one line, you could add it there.
>
> 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 +260,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);
> @@ -278,10 +281,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);
> @@ -308,16 +308,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) {
> @@ -338,17 +331,11 @@ s_uGetDataDuration(
> return 0;
> }
> } else { /* First Frag or Mid Frag */
> +
Unrelated change.
> 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) {
> @@ -356,6 +343,7 @@ s_uGetDataDuration(
> } else {
> uAckTime = 0;
> }
> +
Unrelated change.
> return pDevice->uSIFS + uAckTime + uNextPktTime;
> }
> break;
--
Stefano
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-03-30 18:04 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-30 4:47 [PATCH 0/4] staging: vt6655: Simplify statement logic Briana Oursler
2020-03-30 4:47 ` [PATCH 1/4] Staging: vt6655: Limit return statements Briana Oursler
2020-03-30 18:01 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 2/4] Staging: vt6655: Raise rate determination logic Briana Oursler
2020-03-30 18:02 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 3/4] Staging: vt6655: Create new variable len Briana Oursler
2020-03-30 18:03 ` [Outreachy kernel] " Stefano Brivio
2020-03-30 4:48 ` [PATCH 4/4] Staging: vt6655: Format spaces Briana Oursler
2020-03-30 18:02 ` [Outreachy kernel] " Stefano Brivio
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.