From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 F2159203564BE for ; Tue, 28 Nov 2017 05:06:38 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 9F73585545; Tue, 28 Nov 2017 13:11:01 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-99.rdu2.redhat.com [10.10.120.99]) by smtp.corp.redhat.com (Postfix) with ESMTP id 25CD65447A; Tue, 28 Nov 2017 13:10:59 +0000 (UTC) To: Ruiyu Ni , edk2-devel@lists.01.org Cc: Anthony Perard , Jordan Justen , Ard Biesheuvel References: <20171128083812.59164-1-ruiyu.ni@intel.com> <20171128083812.59164-7-ruiyu.ni@intel.com> From: Laszlo Ersek Message-ID: Date: Tue, 28 Nov 2017 14:10:58 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 MIME-Version: 1.0 In-Reply-To: <20171128083812.59164-7-ruiyu.ni@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Tue, 28 Nov 2017 13:11:01 +0000 (UTC) Subject: Re: [PATCH 6/6] OvmfPkg: Add tftp dynamic command X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 28 Nov 2017 13:06:39 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Anthony Perard > Cc: Julien Grall > --- > 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 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 { > > 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.
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> # > # 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 { > > 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.
> +# Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP
> # > # 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 { > > 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 >