Discussion:
[edk2] [PATCH] ShellPkg\Application\Shell: Refine the code style.
Qiu Shumin
2015-06-19 08:43:15 UTC
Permalink
BOOLEAN variable should not use explicit comparisons to TRUE or FALSE. To follow EDKII code style.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
---
ShellPkg/Application/Shell/ConsoleLogger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ConsoleLogger.c b/ShellPkg/Application/Shell/ConsoleLogger.c
index 22abf55..9b64303 100644
--- a/ShellPkg/Application/Shell/ConsoleLogger.c
+++ b/ShellPkg/Application/Shell/ConsoleLogger.c
@@ -416,7 +416,7 @@ ConsoleLoggerReset (
//
if (!EFI_ERROR (Status)) {
ConsoleLoggerResetBuffers(ConsoleInfo);
- if (ExtendedVerification == TRUE) {
+ if (ExtendedVerification) {
ConsoleInfo->OriginalStartRow = 0;
ConsoleInfo->CurrentStartRow = 0;
}
--
1.9.5.msysgit.1



------------------------------------------------------------------------------
Daryl McDaniel
2015-06-19 13:26:04 UTC
Permalink
I think you misinterpreted the Coding Standard.
It says that while comparisons must be explicit, BOOLEAN variables are the
exception and don't have to be compared to TRUE or FALSE.
It is OPTIONAL whether or not the BOOLEAN variable is explicitly compared.

Explicit comparisons are always the most clear.
It is the developer's choice whether, or not, to compare BOOLEAN variables
explicitly.

So, while the change is superfluous, I don't see anything wrong with it.

Reviewed-by: Daryl McDaniel <***@intel.com>

Daryl

-----Original Message-----
From: Qiu Shumin [mailto:***@intel.com]
Sent: Friday, June 19, 2015 1:43 AM
To: edk2-***@lists.sourceforge.net
Subject: [edk2] [PATCH] ShellPkg\Application\Shell: Refine the code style.

BOOLEAN variable should not use explicit comparisons to TRUE or FALSE. To
follow EDKII code style.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qiu Shumin <***@intel.com>
---
ShellPkg/Application/Shell/ConsoleLogger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ShellPkg/Application/Shell/ConsoleLogger.c
b/ShellPkg/Application/Shell/ConsoleLogger.c
index 22abf55..9b64303 100644
--- a/ShellPkg/Application/Shell/ConsoleLogger.c
+++ b/ShellPkg/Application/Shell/ConsoleLogger.c
@@ -416,7 +416,7 @@ ConsoleLoggerReset (
//
if (!EFI_ERROR (Status)) {
ConsoleLoggerResetBuffers(ConsoleInfo);
- if (ExtendedVerification == TRUE) {
+ if (ExtendedVerification) {
ConsoleInfo->OriginalStartRow = 0;
ConsoleInfo->CurrentStartRow = 0;
}
--
1.9.5.msysgit.1



----------------------------------------------------------------------------
--
_______________________________________________
edk2-devel mailing list
edk2-***@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel


------------------------------------------------------------------------------
Scott Duplichan
2015-06-19 19:11:47 UTC
Permalink
Daryl McDaniel [mailto:edk2-***@mc2research.org] wrote:

]Sent: Friday, June 19, 2015 08:26 AM
]To: edk2-***@lists.sourceforge.net
]Subject: Re: [edk2] [PATCH] ShellPkg\Application\Shell: Refine the code style.
]
]I think you misinterpreted the Coding Standard.
]It says that while comparisons must be explicit, BOOLEAN variables are the
]exception and don't have to be compared to TRUE or FALSE.
]It is OPTIONAL whether or not the BOOLEAN variable is explicitly compared.

What you say is consistent with the existing EDK2 code, where many examples
of both forms can be found. On the other hand, table 10 on page 48 of the
coding document lists "If (Done == TRUE)"** under the heading Incorrect, and
lists "if (Done)" under the heading Correct.

** Sad part is that the entire column labeled 'Incorrect' is not even valid
C code due to the capitalization of 'if'.
Thanks,
Scott

]Explicit comparisons are always the most clear.
]It is the developer's choice whether, or not, to compare BOOLEAN variables
]explicitly.
]
]So, while the change is superfluous, I don't see anything wrong with it.
]
]Reviewed-by: Daryl McDaniel <***@intel.com>
]
]Daryl
]
]-----Original Message-----
]From: Qiu Shumin [mailto:***@intel.com]
]Sent: Friday, June 19, 2015 1:43 AM
]To: edk2-***@lists.sourceforge.net
]Subject: [edk2] [PATCH] ShellPkg\Application\Shell: Refine the code style.
]
]BOOLEAN variable should not use explicit comparisons to TRUE or FALSE. To
]follow EDKII code style.
]
]Contributed-under: TianoCore Contribution Agreement 1.0
]Signed-off-by: Qiu Shumin <***@intel.com>
]---
] ShellPkg/Application/Shell/ConsoleLogger.c | 2 +-
] 1 file changed, 1 insertion(+), 1 deletion(-)
]
]diff --git a/ShellPkg/Application/Shell/ConsoleLogger.c
]b/ShellPkg/Application/Shell/ConsoleLogger.c
]index 22abf55..9b64303 100644
]--- a/ShellPkg/Application/Shell/ConsoleLogger.c
]+++ b/ShellPkg/Application/Shell/ConsoleLogger.c
]@@ -416,7 +416,7 @@ ConsoleLoggerReset (
] //
] if (!EFI_ERROR (Status)) {
] ConsoleLoggerResetBuffers(ConsoleInfo);
]- if (ExtendedVerification == TRUE) {
]+ if (ExtendedVerification) {
] ConsoleInfo->OriginalStartRow = 0;
] ConsoleInfo->CurrentStartRow = 0;
] }
]--
]1.9.5.msysgit.1



------------------------------------------------------------------------------
Carsey, Jaben
2015-06-19 14:56:52 UTC
Permalink
Post by Daryl McDaniel
-----Original Message-----
From: Qiu, Shumin
Sent: Friday, June 19, 2015 1:43 AM
Cc: Carsey, Jaben
Subject: [PATCH] ShellPkg\Application\Shell: Refine the code style.
Importance: High
BOOLEAN variable should not use explicit comparisons to TRUE or FALSE. To
follow EDKII code style.
Contributed-under: TianoCore Contribution Agreement 1.0
---
ShellPkg/Application/Shell/ConsoleLogger.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/ShellPkg/Application/Shell/ConsoleLogger.c
b/ShellPkg/Application/Shell/ConsoleLogger.c
index 22abf55..9b64303 100644
--- a/ShellPkg/Application/Shell/ConsoleLogger.c
+++ b/ShellPkg/Application/Shell/ConsoleLogger.c
@@ -416,7 +416,7 @@ ConsoleLoggerReset (
//
if (!EFI_ERROR (Status)) {
ConsoleLoggerResetBuffers(ConsoleInfo);
- if (ExtendedVerification == TRUE) {
+ if (ExtendedVerification) {
ConsoleInfo->OriginalStartRow = 0;
ConsoleInfo->CurrentStartRow = 0;
}
--
1.9.5.msysgit.1
------------------------------------------------------------------------------
Loading...