Re: [PATCH 6/6] OvmfPkg: Add tftp dynamic command

[Laszlo Ersek <lersek@redhat.com>]

Hello Ray,

I have two comments:

On 11/28/17 09:38, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Anthony Perard <anthony.perard@citrix.com>
> Cc: Julien Grall <julien.grall@linaro.org>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc    | 1 +
>  OvmfPkg/OvmfPkgIa32.fdf    | 3 ++-
>  OvmfPkg/OvmfPkgIa32X64.dsc | 1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf | 3 ++-
>  OvmfPkg/OvmfPkgX64.dsc     | 1 +
>  OvmfPkg/OvmfPkgX64.fdf     | 1 +
>  6 files changed, 8 insertions(+), 2 deletions(-)

(1) Please add the following to the commit message:

"The TFTP command was converted from a NULL class library instance to a
dynamic shell command in commit 0961002352e9. This patch complements
commit f9bc2f876326, which only removed the old library, but didn't add
the new dynamic command."


(2) You forgot to update the Intel (C) notice in "OvmfPkg/OvmfPkgX64.fdf".

(The DSC files already have "2017" as the last year, and the
OvmfPkgIa32.fdf and OvmfPkgIa32X64.fdf files are updated in this patch,
but the "OvmfPkgX64.fdf" file was missed.)


With those changes:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 19fa0b4c8d..1acaa279c7 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -783,6 +783,7 @@ [Components]
>  !endif
>  
>  !ifndef $(USE_OLD_SHELL)
> +  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>    ShellPkg/Application/Shell/Shell.inf {
>      <LibraryClasses>
>        ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 06a439f8cb..ba980834d7 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Open Virtual Machine Firmware: FDF
>  #
> -#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -285,6 +285,7 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  
>  !ifndef $(USE_OLD_SHELL)
> +INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>  INF  ShellPkg/Application/Shell/Shell.inf
>  !else
>  INF  RuleOverride = BINARY EdkShellBinPkg/FullShell/FullShell.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index e1555dbfa8..eac53947c1 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -792,6 +792,7 @@ [Components.X64]
>  !endif
>  
>  !ifndef $(USE_OLD_SHELL)
> +  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>    ShellPkg/Application/Shell/Shell.inf {
>      <LibraryClasses>
>        ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index ced4c5639f..72ac82e76b 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -1,7 +1,7 @@
>  ## @file
>  #  Open Virtual Machine Firmware: FDF
>  #
> -#  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.<BR>
>  #  (C) Copyright 2016 Hewlett Packard Enterprise Development LP<BR>
>  #
>  #  This program and the accompanying materials
> @@ -286,6 +286,7 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  
>  !ifndef $(USE_OLD_SHELL)
> +INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>  INF  ShellPkg/Application/Shell/Shell.inf
>  !else
>  INF  RuleOverride = BINARY USE = X64 EdkShellBinPkg/FullShell/FullShell.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 83d63e55d7..f4a34e7f14 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -790,6 +790,7 @@ [Components]
>  !endif
>  
>  !ifndef $(USE_OLD_SHELL)
> +  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>    ShellPkg/Application/Shell/Shell.inf {
>      <LibraryClasses>
>        ShellCommandLib|ShellPkg/Library/UefiShellCommandLib/UefiShellCommandLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 62dd58f6e4..b9887e4386 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -286,6 +286,7 @@ [FV.DXEFV]
>  INF  MdeModulePkg/Universal/Disk/UdfDxe/UdfDxe.inf
>  
>  !ifndef $(USE_OLD_SHELL)
> +INF  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
>  INF  ShellPkg/Application/Shell/Shell.inf
>  !else
>  INF  RuleOverride = BINARY EdkShellBinPkg/FullShell/FullShell.inf
>