From: "Ni, Ruiyu" <ruiyu.ni@intel.com>
To: "Carsey, Jaben" <jaben.carsey@intel.com>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Chen, Chen A" <chen.a.chen@intel.com>
Subject: Re: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues
Date: Tue, 10 Jan 2017 02:24:34 +0000 [thread overview]
Message-ID: <734D49CCEBEEF84792F5B80ED585239D5B8799B9@SHSMSX104.ccr.corp.intel.com> (raw)
In-Reply-To: <CB6E33457884FA40993F35157061515C54B3DDDD@FMSMSX103.amr.corp.intel.com>
DisplayDriverModelHandle() always return SUCCESS so we change the return type to VOID.
Thanks/Ray
> -----Original Message-----
> From: Carsey, Jaben
> Sent: Monday, January 9, 2017 11:37 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Chen, Chen A <chen.a.chen@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: RE: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues
>
> One question inline.
>
> > -----Original Message-----
> > From: Ni, Ruiyu
> > Sent: Monday, January 9, 2017 1:31 AM
> > To: edk2-devel@lists.01.org
> > Cc: Chen, Chen A <chen.a.chen@intel.com>; Carsey, Jaben
> > <jaben.carsey@intel.com>
> > Subject: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues
> > Importance: High
> >
> > From: Chen A Chen <chen.a.chen@intel.com>
> >
> > The change doesn't impact the functionality.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Cc: Jaben Carsey <jaben.carsey@intel.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> > Signed-off-by: Chen A Chen <chen.a.chen@intel.com>
> > ---
> > ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c | 119
> > ++++++++++----------
> > --
> > 1 file changed, 54 insertions(+), 65 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c
> > b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c
> > index 0ac49e1..2773842 100644
> > --- a/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c
> > +++ b/ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c
> > @@ -642,11 +642,8 @@ DisplayDriverModelHandle (
> > @param[in] DriverInfo TRUE to show all info about the handle.
> > @param[in] Multiple TRUE indicates more than will be output,
> > FALSE for a single one.
> > -
> > - @retval SHELL_SUCCESS The operation was successful.
> > - @retval SHELL_INVALID_PARAMETER ProtocolName was NULL or invalid.
> > **/
> > -SHELL_STATUS
> > +VOID
>
> Is there a reason to change the function to VOID and not add a check for the
> return from the DisplayDriverModelHandle function?
>
> > DoDhByHandle(
> > IN CONST EFI_HANDLE TheHandle,
> > IN CONST BOOLEAN Verbose,
> > @@ -656,10 +653,8 @@ DoDhByHandle(
> > IN CONST BOOLEAN Multiple
> > )
> > {
> > - CHAR16 *ProtocolInfoString;
> > - SHELL_STATUS ShellStatus;
> > + CHAR16 *ProtocolInfoString;
> >
> > - ShellStatus = SHELL_SUCCESS;
> > ProtocolInfoString = NULL;
> >
> > if (!Sfo) {
> > @@ -672,7 +667,8 @@ DoDhByHandle(
> > STRING_TOKEN (STR_DH_OUTPUT),
> > gShellDriver1HiiHandle,
> > ConvertHandleToHandleIndex(TheHandle),
> > - ProtocolInfoString==NULL?L"":ProtocolInfoString);
> > + ProtocolInfoString==NULL?L"":ProtocolInfoString
> > + );
> > } else {
> > ProtocolInfoString = GetProtocolInfoString(TheHandle, Language,
> > L"\r\n", Verbose, TRUE);
> > ShellPrintHiiEx(
> > @@ -683,7 +679,8 @@ DoDhByHandle(
> > gShellDriver1HiiHandle,
> > ConvertHandleToHandleIndex(TheHandle),
> > TheHandle,
> > - ProtocolInfoString==NULL?L"":ProtocolInfoString);
> > + ProtocolInfoString==NULL?L"":ProtocolInfoString
> > + );
> > }
> >
> > if (DriverInfo) {
> > @@ -702,16 +699,13 @@ DoDhByHandle(
> > L"ControllerName",
> > ConvertHandleToHandleIndex(TheHandle),
> > L"DevPath",
> > - ProtocolInfoString==NULL?L"":ProtocolInfoString);
> > -
> > -
> > + ProtocolInfoString==NULL?L"":ProtocolInfoString
> > + );
> > }
> >
> > -
> > if (ProtocolInfoString != NULL) {
> > FreePool(ProtocolInfoString);
> > }
> > - return (ShellStatus);
> > }
> >
> > /**
> > @@ -723,8 +717,8 @@ DoDhByHandle(
> > @param[in] Language Language string per UEFI specification.
> > @param[in] DriverInfo TRUE to show all info about the handle.
> >
> > - @retval SHELL_SUCCESS The operation was successful.
> > - @retval SHELL_INVALID_PARAMETER ProtocolName was NULL or invalid.
> > + @retval SHELL_SUCCESS The operation was successful.
> > + @retval SHELL_ABORTED The operation was aborted.
> > **/
> > SHELL_STATUS
> > DoDhForHandleList(
> > @@ -740,15 +734,8 @@ DoDhForHandleList(
> >
> > ShellStatus = SHELL_SUCCESS;
> >
> > - for (HandleWalker = HandleList ; HandleWalker != NULL &&
> > *HandleWalker != NULL && ShellStatus == SHELL_SUCCESS;
> HandleWalker++) {
> > - ShellStatus = DoDhByHandle(
> > - *HandleWalker,
> > - Verbose,
> > - Sfo,
> > - Language,
> > - DriverInfo,
> > - TRUE
> > - );
> > + for ( HandleWalker = HandleList; HandleWalker != NULL &&
> > + *HandleWalker !=
> > NULL; HandleWalker++ ) {
> > + DoDhByHandle (*HandleWalker, Verbose, Sfo, Language, DriverInfo,
> > + TRUE);
> > if (ShellGetExecutionBreakFlag ()) {
> > ShellStatus = SHELL_ABORTED;
> > break;
> > @@ -862,10 +849,10 @@ ShellCommandRunDh (
> > SHELL_STATUS ShellStatus;
> > CHAR8 *Language;
> > CONST CHAR16 *Lang;
> > - CONST CHAR16 *Temp2;
> > - BOOLEAN SfoMode;
> > - BOOLEAN FlagD;
> > - BOOLEAN Verbose;
> > + CONST CHAR16 *RawValue;
> > + BOOLEAN SfoFlag;
> > + BOOLEAN DriverFlag;
> > + BOOLEAN VerboseFlag;
> > UINT64 Intermediate;
> >
> > ShellStatus = SHELL_SUCCESS;
> > @@ -900,30 +887,32 @@ ShellCommandRunDh (
> > return (SHELL_INVALID_PARAMETER);
> > }
> >
> > - Lang = ShellCommandLineGetValue(Package, L"-l");
> > - if (Lang != NULL) {
> > - Language = AllocateZeroPool(StrSize(Lang));
> > - AsciiSPrint(Language, StrSize(Lang), "%S", Lang);
> > - } else if (!ShellCommandLineGetFlag(Package, L"-l")){
> > + if (ShellCommandLineGetFlag(Package, L"-l")) {
> > + Lang = ShellCommandLineGetValue(Package, L"-l");
> > + if (Lang != NULL) {
> > + Language = AllocateZeroPool(StrSize(Lang));
> > + AsciiSPrint(Language, StrSize(Lang), "%S", Lang);
> > + } else {
> > + ASSERT(Language == NULL);
> > + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_NO_VALUE),
> > gShellDriver1HiiHandle, L"dh", L"-l");
> > + ShellCommandLineFreeVarList(Package);
> > + return (SHELL_INVALID_PARAMETER);
> > + }
> > + } else {
> > Language = AllocateZeroPool(10);
> > AsciiSPrint(Language, 10, "en-us");
> > - } else {
> > - ASSERT(Language == NULL);
> > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE),
> > gShellDriver1HiiHandle, L"dh", L"-l");
> > - ShellCommandLineFreeVarList (Package);
> > - return (SHELL_INVALID_PARAMETER);
> > }
> >
> > - SfoMode = ShellCommandLineGetFlag(Package, L"-sfo");
> > - FlagD = ShellCommandLineGetFlag(Package, L"-d");
> > - Verbose = (BOOLEAN)(ShellCommandLineGetFlag(Package, L"-v") ||
> > ShellCommandLineGetFlag(Package, L"-verbose"));
> > + SfoFlag = ShellCommandLineGetFlag (Package, L"-sfo");
> > + DriverFlag = ShellCommandLineGetFlag (Package, L"-d");
> > + VerboseFlag = (BOOLEAN)(ShellCommandLineGetFlag (Package, L"-v")
> > + ||
> > ShellCommandLineGetFlag (Package, L"-verbose"));
> >
> > - if (ShellCommandLineGetFlag(Package, L"-p")) {
> > - if (ShellCommandLineGetCount(Package) > 1) {
> > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_TOO_MANY),
> > gShellDriver1HiiHandle, L"dh");
> > + if (ShellCommandLineGetFlag (Package, L"-p")) {
> > + if (ShellCommandLineGetCount (Package) > 1) {
> > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > + (STR_GEN_TOO_MANY),
> > gShellDriver1HiiHandle, L"dh");
> > ShellStatus = SHELL_INVALID_PARAMETER;
> > } else if (ShellCommandLineGetValue(Package, L"-p") == NULL) {
> > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE),
> > gShellDriver1HiiHandle, L"dh", L"-p");
> > + ShellPrintHiiEx (-1, -1, NULL, STRING_TOKEN
> > + (STR_GEN_NO_VALUE),
> > gShellDriver1HiiHandle, L"dh", L"-p");
> > ShellStatus = SHELL_INVALID_PARAMETER;
> > } else {
> > //
> > @@ -931,41 +920,41 @@ ShellCommandRunDh (
> > //
> > ShellStatus = DoDhByProtocol(
> > ShellCommandLineGetValue(Package, L"-p"),
> > - Verbose,
> > - SfoMode,
> > - Lang==NULL?NULL:Language,
> > - FlagD
> > - );
> > + VerboseFlag,
> > + SfoFlag,
> > + Language,
> > + DriverFlag
> > + );
> > }
> > } else {
> > - Temp2 = ShellCommandLineGetRawValue(Package, 1);
> > - if (Temp2 == NULL) {
> > + RawValue = ShellCommandLineGetRawValue(Package, 1);
> > + if (RawValue == NULL) {
> > //
> > // Print everything
> > //
> > ShellStatus = DoDhForAll(
> > - SfoMode,
> > - Verbose,
> > - Lang==NULL?NULL:Language,
> > - FlagD
> > + SfoFlag,
> > + VerboseFlag,
> > + Language,
> > + DriverFlag
> > );
> > } else {
> > - Status = ShellConvertStringToUint64(Temp2, &Intermediate, TRUE,
> FALSE);
> > + Status = ShellConvertStringToUint64(RawValue, &Intermediate,
> > + TRUE,
> > FALSE);
> > if (EFI_ERROR(Status) ||
> > ConvertHandleIndexToHandle((UINTN)Intermediate) == NULL) {
> > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_INV_HANDLE),
> > gShellDriver1HiiHandle, L"dh", Temp2);
> > + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN
> > + (STR_GEN_INV_HANDLE),
> > gShellDriver1HiiHandle, L"dh", RawValue);
> > ShellStatus = SHELL_INVALID_PARAMETER;
> > } else {
> > //
> > // print 1 handle
> > //
> > - ShellStatus = DoDhByHandle(
> > + DoDhByHandle(
> > ConvertHandleIndexToHandle((UINTN)Intermediate),
> > - Verbose,
> > - SfoMode,
> > - Lang==NULL?NULL:Language,
> > - FlagD,
> > + VerboseFlag,
> > + SfoFlag,
> > + Language,
> > + DriverFlag,
> > FALSE
> > - );
> > + );
> > }
> > }
> > }
> > --
> > 2.9.0.windows.1
next prev parent reply other threads:[~2017-01-10 2:24 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-01-09 9:30 [PATCH 0/5] Change "dh" to support dump from GUID and "decode" parameter Ruiyu Ni
2017-01-09 9:30 ` [PATCH 1/5] ShellPkg/HandleParsingLib: Rename global variables Ruiyu Ni
2017-01-09 9:30 ` [PATCH 2/5] ShellPkg/HandleParsingLib: Return NULL name for unknown GUID Ruiyu Ni
2017-01-09 9:30 ` [PATCH 3/5] ShellPkg/HandleParsingLib: Add new API GetAllMappingGuids Ruiyu Ni
2017-01-09 9:30 ` [PATCH 4/5] ShellPkg/Dh: Fix coding style issues Ruiyu Ni
2017-01-09 15:36 ` Carsey, Jaben
2017-01-10 2:24 ` Ni, Ruiyu [this message]
2017-01-10 16:20 ` Carsey, Jaben
2017-01-09 9:30 ` [PATCH 5/5] ShellPkg/dh: Support dump from GUID and "decode" parameter Ruiyu Ni
2017-01-09 15:40 ` [PATCH 0/5] Change "dh" to support " Carsey, Jaben
2017-01-10 2:23 ` Ni, Ruiyu
2017-01-10 21:43 ` Carsey, Jaben
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=734D49CCEBEEF84792F5B80ED585239D5B8799B9@SHSMSX104.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox