From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 A9B70210DA145 for ; Sun, 5 Aug 2018 19:03:29 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Aug 2018 19:03:29 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.51,450,1526367600"; d="scan'208";a="72959789" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.4]) ([10.239.9.4]) by orsmga003.jf.intel.com with ESMTP; 05 Aug 2018 19:03:26 -0700 To: Laszlo Ersek , edk2-devel-01 Cc: Jaben Carsey References: <20180803121537.32123-1-lersek@redhat.com> <20180803121537.32123-7-lersek@redhat.com> From: "Ni, Ruiyu" Message-ID: <67724fcc-8e5d-694f-1e0e-df7025129239@Intel.com> Date: Mon, 6 Aug 2018 10:04:04 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20180803121537.32123-7-lersek@redhat.com> Subject: Re: [PATCH v2 6/7] ShellPkg/UefiShellLib: drop DeviceHandle param of ShellOpenFileByDevicePath() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Aug 2018 02:03:29 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 8/3/2018 8:15 PM, Laszlo Ersek wrote: > The ShellOpenFileByDevicePath() API promises to set the DeviceHandle > output parameter to the handle of the filesystem identified by the > FilePath input parameter. However, this doesn't actually happen when the > UEFI Shell 2.0 method is used (which is basically "always" nowadays). > > Accordingly, the only caller of ShellOpenFileByDevicePath(), namely > ShellOpenFileByName(), defines a (dummy) local DeviceHandle variable just > so it can call ShellOpenFileByDevicePath(). > > Remove the useless output parameter. > > Cc: Jaben Carsey > Cc: Ruiyu Ni > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Laszlo Ersek > Reviewed-by: Jaben Carsey > --- > > Notes: > v2: > - pick up Jaben's R-b > > ShellPkg/Library/UefiShellLib/UefiShellLib.inf | 2 +- > ShellPkg/Include/Library/ShellLib.h | 2 -- > ShellPkg/Library/UefiShellLib/UefiShellLib.c | 11 ++++------- > 3 files changed, 5 insertions(+), 10 deletions(-) > > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf > index 0df632378fe6..38d9a4b81f5f 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.inf > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.inf > @@ -19,7 +19,7 @@ [Defines] > BASE_NAME = UefiShellLib > FILE_GUID = 449D0F00-2148-4a43-9836-F10B3980ECF5 > MODULE_TYPE = UEFI_DRIVER > - VERSION_STRING = 1.1 > + VERSION_STRING = 1.2 > LIBRARY_CLASS = ShellLib|UEFI_APPLICATION UEFI_DRIVER DXE_RUNTIME_DRIVER DXE_DRIVER > CONSTRUCTOR = ShellLibConstructor > DESTRUCTOR = ShellLibDestructor > diff --git a/ShellPkg/Include/Library/ShellLib.h b/ShellPkg/Include/Library/ShellLib.h > index e360a67ac751..92fddc50f5dd 100644 > --- a/ShellPkg/Include/Library/ShellLib.h > +++ b/ShellPkg/Include/Library/ShellLib.h > @@ -89,7 +89,6 @@ ShellSetFileInfo ( > > @param[in, out] FilePath On input, the device path to the file. On output, > the remaining device path. > - @param[out] DeviceHandle Pointer to the system device handle. > @param[out] FileHandle Pointer to the file handle. > @param[in] OpenMode The mode to open the file with. > @param[in] Attributes The file's file attributes. > @@ -115,7 +114,6 @@ EFI_STATUS > EFIAPI > ShellOpenFileByDevicePath( > IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath, > - OUT EFI_HANDLE *DeviceHandle, > OUT SHELL_FILE_HANDLE *FileHandle, > IN UINT64 OpenMode, > IN UINT64 Attributes > diff --git a/ShellPkg/Library/UefiShellLib/UefiShellLib.c b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > index 3c24ba1742bf..18c3be4a8bc7 100644 > --- a/ShellPkg/Library/UefiShellLib/UefiShellLib.c > +++ b/ShellPkg/Library/UefiShellLib/UefiShellLib.c > @@ -472,7 +472,6 @@ ShellSetFileInfo ( > > @param FilePath on input the device path to the file. On output > the remaining device path. > - @param DeviceHandle pointer to the system device handle. > @param FileHandle pointer to the file handle. > @param OpenMode the mode to open the file with. > @param Attributes the file's file attributes. > @@ -498,7 +497,6 @@ EFI_STATUS > EFIAPI > ShellOpenFileByDevicePath( > IN OUT EFI_DEVICE_PATH_PROTOCOL **FilePath, > - OUT EFI_HANDLE *DeviceHandle, > OUT SHELL_FILE_HANDLE *FileHandle, > IN UINT64 OpenMode, > IN UINT64 Attributes > @@ -511,8 +509,9 @@ ShellOpenFileByDevicePath( > EFI_FILE_PROTOCOL *Handle2; > CHAR16 *FnafPathName; > UINTN PathLen; > + EFI_HANDLE DeviceHandle; > > - if (FilePath == NULL || FileHandle == NULL || DeviceHandle == NULL) { > + if (FilePath == NULL || FileHandle == NULL) { > return (EFI_INVALID_PARAMETER); > } > > @@ -538,11 +537,11 @@ ShellOpenFileByDevicePath( > // > Status = gBS->LocateDevicePath (&gEfiSimpleFileSystemProtocolGuid, > FilePath, > - DeviceHandle); > + &DeviceHandle); > if (EFI_ERROR (Status)) { > return Status; > } > - Status = gBS->OpenProtocol(*DeviceHandle, > + Status = gBS->OpenProtocol(DeviceHandle, > &gEfiSimpleFileSystemProtocolGuid, > (VOID**)&EfiSimpleFileSystemProtocol, > gImageHandle, > @@ -690,7 +689,6 @@ ShellOpenFileByName( > IN UINT64 Attributes > ) > { > - EFI_HANDLE DeviceHandle; > EFI_DEVICE_PATH_PROTOCOL *FilePath; > EFI_STATUS Status; > EFI_FILE_INFO *FileInfo; > @@ -774,7 +772,6 @@ ShellOpenFileByName( > FilePath = mEfiShellEnvironment2->NameToPath ((CHAR16*)FileName); > if (FilePath != NULL) { > return (ShellOpenFileByDevicePath(&FilePath, > - &DeviceHandle, > FileHandle, > OpenMode, > Attributes)); > Reviewed-by: Ruiyu Ni -- Thanks, Ray