Re: [PATCH 2/3] ArmPkg/PlatformBootManagerLib: refer to Shell FILE_GUID directly

[Leif Lindholm <leif.lindholm@linaro.org>]

On Mon, Mar 20, 2017 at 08:35:44PM +0000, Ard Biesheuvel wrote:
> Instead of indirecting the reference to the Shell binary via a PCD
> that is defined in IntelFrameworkModulePkg, and which invariably
> gets set to the same value by all users of this library, move the
> reference into the code, and drop the reference to the PCD entirely.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c               | 7 +++++--
>  ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 -
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index cc5a4d1ff9b3..d479c28775fb 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -33,6 +33,9 @@
>  
>  #define DP_NODE_LEN(Type) { (UINT8)sizeof (Type), (UINT8)(sizeof (Type) >> 8) }
>  
> +STATIC CONST EFI_GUID mUefiShellFileGuid = {
> +  0x7C04A583, 0x9E3E, 0x4f1c, { 0xAD, 0x65, 0xE0, 0x52, 0x68, 0xD0, 0xB4, 0xD1 }
> +};

Surely this ought to be defined in a shared header file rather than
replicated across the tree? (And yes, used in QuarkPlatformPkg as
well.)

Otherwise, looks like a good change.

/
    Leif

>  
>  #pragma pack (1)
>  typedef struct {
> @@ -327,7 +330,7 @@ AddOutput (
>  STATIC
>  VOID
>  PlatformRegisterFvBootOption (
> -  EFI_GUID                         *FileGuid,
> +  CONST EFI_GUID                   *FileGuid,
>    CHAR16                           *Description,
>    UINT32                           Attributes
>    )
> @@ -540,7 +543,7 @@ PlatformBootManagerAfterConsole (
>    // Register UEFI Shell
>    //
>    PlatformRegisterFvBootOption (
> -    PcdGetPtr (PcdShellFile), L"UEFI Shell", LOAD_OPTION_ACTIVE
> +    &mUefiShellFileGuid, L"UEFI Shell", LOAD_OPTION_ACTIVE
>      );
>  }
>  
> diff --git a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 8ec4f1dea6c4..8ac3b3799674 100644
> --- a/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/ArmPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -59,7 +59,6 @@ [FeaturePcd]
>  
>  [FixedPcd]
>    gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdLogoFile
> -  gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity
> -- 
> 2.7.4
>