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

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

On Wed, Mar 22, 2017 at 01:16:26PM +0000, Ard Biesheuvel wrote:
> On 22 March 2017 at 12:53, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > 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.
> >
> 
> Yes, it would make the most sense to add this to the [Guid] section of
> ShellPkg itself. I will propose it as a separate series, and replace
> the quark reference as well

Since I'm about to go offline - with that change implemented:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>