From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id EFD2D81AD6 for ; Mon, 9 Jan 2017 07:37:58 -0800 (PST) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga101.jf.intel.com with ESMTP; 09 Jan 2017 07:37:58 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,340,1477983600"; d="scan'208";a="47116819" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga004.jf.intel.com with ESMTP; 09 Jan 2017 07:37:58 -0800 Received: from fmsmsx112.amr.corp.intel.com (10.18.116.6) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.248.2; Mon, 9 Jan 2017 07:36:51 -0800 Received: from fmsmsx103.amr.corp.intel.com ([169.254.2.47]) by FMSMSX112.amr.corp.intel.com ([10.18.116.6]) with mapi id 14.03.0248.002; Mon, 9 Jan 2017 07:36:51 -0800 From: "Carsey, Jaben" To: "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: "Chen, Chen A" , "Carsey, Jaben" Thread-Topic: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues Thread-Index: AQHSalsdlef3BkcunE6izesxtJuTbKEwRnAQ Date: Mon, 9 Jan 2017 15:36:50 +0000 Message-ID: References: <20170109093052.140504-1-ruiyu.ni@intel.com> <20170109093052.140504-5-ruiyu.ni@intel.com> In-Reply-To: <20170109093052.140504-5-ruiyu.ni@intel.com> Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.1.200.106] MIME-Version: 1.0 Subject: Re: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 09 Jan 2017 15:37:59 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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 ; Carsey, Jaben > > Subject: [PATCH 4/5] ShellPkg/Dh: Fix coding style issues > Importance: High >=20 > From: Chen A Chen >=20 > The change doesn't impact the functionality. >=20 > Contributed-under: TianoCore Contribution Agreement 1.0 > Cc: Jaben Carsey > Cc: Ruiyu Ni > Signed-off-by: Ruiyu Ni > Signed-off-by: Chen A Chen > --- > ShellPkg/Library/UefiShellDriver1CommandsLib/Dh.c | 119 ++++++++++------= ---- > -- > 1 file changed, 54 insertions(+), 65 deletions(-) >=20 > 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 th= e 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; >=20 > - ShellStatus =3D SHELL_SUCCESS; > ProtocolInfoString =3D NULL; >=20 > if (!Sfo) { > @@ -672,7 +667,8 @@ DoDhByHandle( > STRING_TOKEN (STR_DH_OUTPUT), > gShellDriver1HiiHandle, > ConvertHandleToHandleIndex(TheHandle), > - ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString); > + ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString > + ); > } else { > ProtocolInfoString =3D GetProtocolInfoString(TheHandle, Language, = L"\r\n", > Verbose, TRUE); > ShellPrintHiiEx( > @@ -683,7 +679,8 @@ DoDhByHandle( > gShellDriver1HiiHandle, > ConvertHandleToHandleIndex(TheHandle), > TheHandle, > - ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString); > + ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString > + ); > } >=20 > if (DriverInfo) { > @@ -702,16 +699,13 @@ DoDhByHandle( > L"ControllerName", > ConvertHandleToHandleIndex(TheHandle), > L"DevPath", > - ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString); > - > - > + ProtocolInfoString=3D=3DNULL?L"":ProtocolInfoString > + ); > } >=20 > - > if (ProtocolInfoString !=3D NULL) { > FreePool(ProtocolInfoString); > } > - return (ShellStatus); > } >=20 > /** > @@ -723,8 +717,8 @@ DoDhByHandle( > @param[in] Language Language string per UEFI specification. > @param[in] DriverInfo TRUE to show all info about the handle. >=20 > - @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( >=20 > ShellStatus =3D SHELL_SUCCESS; >=20 > - for (HandleWalker =3D HandleList ; HandleWalker !=3D NULL && *HandleWa= lker !=3D > NULL && ShellStatus =3D=3D SHELL_SUCCESS; HandleWalker++) { > - ShellStatus =3D DoDhByHandle( > - *HandleWalker, > - Verbose, > - Sfo, > - Language, > - DriverInfo, > - TRUE > - ); > + for ( HandleWalker =3D HandleList; HandleWalker !=3D NULL && *HandleWa= lker !=3D > NULL; HandleWalker++ ) { > + DoDhByHandle (*HandleWalker, Verbose, Sfo, Language, DriverInfo, TRU= E); > if (ShellGetExecutionBreakFlag ()) { > ShellStatus =3D 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; >=20 > ShellStatus =3D SHELL_SUCCESS; > @@ -900,30 +887,32 @@ ShellCommandRunDh ( > return (SHELL_INVALID_PARAMETER); > } >=20 > - Lang =3D ShellCommandLineGetValue(Package, L"-l"); > - if (Lang !=3D NULL) { > - Language =3D AllocateZeroPool(StrSize(Lang)); > - AsciiSPrint(Language, StrSize(Lang), "%S", Lang); > - } else if (!ShellCommandLineGetFlag(Package, L"-l")){ > + if (ShellCommandLineGetFlag(Package, L"-l")) { > + Lang =3D ShellCommandLineGetValue(Package, L"-l"); > + if (Lang !=3D NULL) { > + Language =3D AllocateZeroPool(StrSize(Lang)); > + AsciiSPrint(Language, StrSize(Lang), "%S", Lang); > + } else { > + ASSERT(Language =3D=3D NULL); > + ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN(STR_GEN_NO_VALUE), > gShellDriver1HiiHandle, L"dh", L"-l"); > + ShellCommandLineFreeVarList(Package); > + return (SHELL_INVALID_PARAMETER); > + } > + } else { > Language =3D AllocateZeroPool(10); > AsciiSPrint(Language, 10, "en-us"); > - } else { > - ASSERT(Language =3D=3D NULL); > - ShellPrintHiiEx(-1, -1, NULL, STRING_TOKEN (STR_GEN_NO_VALUE), > gShellDriver1HiiHandle, L"dh", L"-l"); > - ShellCommandLineFreeVarList (Package); > - return (SHELL_INVALID_PARAMETER); > } >=20 > - SfoMode =3D ShellCommandLineGetFlag(Package, L"-sfo"); > - FlagD =3D ShellCommandLineGetFlag(Package, L"-d"); > - Verbose =3D (BOOLEAN)(ShellCommandLineGetFlag(Package, L"-v") || > ShellCommandLineGetFlag(Package, L"-verbose")); > + SfoFlag =3D ShellCommandLineGetFlag (Package, L"-sfo"); > + DriverFlag =3D ShellCommandLineGetFlag (Package, L"-d"); > + VerboseFlag =3D (BOOLEAN)(ShellCommandLineGetFlag (Package, L"-v") |= | > ShellCommandLineGetFlag (Package, L"-verbose")); >=20 > - 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 =3D SHELL_INVALID_PARAMETER; > } else if (ShellCommandLineGetValue(Package, L"-p") =3D=3D 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 =3D SHELL_INVALID_PARAMETER; > } else { > // > @@ -931,41 +920,41 @@ ShellCommandRunDh ( > // > ShellStatus =3D DoDhByProtocol( > ShellCommandLineGetValue(Package, L"-p"), > - Verbose, > - SfoMode, > - Lang=3D=3DNULL?NULL:Language, > - FlagD > - ); > + VerboseFlag, > + SfoFlag, > + Language, > + DriverFlag > + ); > } > } else { > - Temp2 =3D ShellCommandLineGetRawValue(Package, 1); > - if (Temp2 =3D=3D NULL) { > + RawValue =3D ShellCommandLineGetRawValue(Package, 1); > + if (RawValue =3D=3D NULL) { > // > // Print everything > // > ShellStatus =3D DoDhForAll( > - SfoMode, > - Verbose, > - Lang=3D=3DNULL?NULL:Language, > - FlagD > + SfoFlag, > + VerboseFlag, > + Language, > + DriverFlag > ); > } else { > - Status =3D ShellConvertStringToUint64(Temp2, &Intermediate, TRUE= , FALSE); > + Status =3D ShellConvertStringToUint64(RawValue, &Intermediate, T= RUE, > FALSE); > if (EFI_ERROR(Status) || > ConvertHandleIndexToHandle((UINTN)Intermediate) =3D=3D 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 =3D SHELL_INVALID_PARAMETER; > } else { > // > // print 1 handle > // > - ShellStatus =3D DoDhByHandle( > + DoDhByHandle( > ConvertHandleIndexToHandle((UINTN)Intermediate), > - Verbose, > - SfoMode, > - Lang=3D=3DNULL?NULL:Language, > - FlagD, > + VerboseFlag, > + SfoFlag, > + Language, > + DriverFlag, > FALSE > - ); > + ); > } > } > } > -- > 2.9.0.windows.1