On 09/26/19 04:53, Gao, Zhichao wrote:
>
>
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
>> Laszlo Ersek
>> Sent: Wednesday, September 18, 2019 3:50 AM
>> To: edk2-devel-groups-io <devel@edk2.groups.io>
>> Cc: Carsey, Jaben <jaben.car...@intel.com>; Ni, Ray <ray...@intel.com>;
>> Gao, Zhichao <zhichao....@intel.com>
>> Subject: [edk2-devel] [PATCH 30/35] ShellPkg: stop taking EFI_HANDLE in
>> place of SHELL_FILE_HANDLE
>>
>> The TouchFileByHandle() and IsDirectoryEmpty() functions are passed
>> SHELL_FILE_HANDLE parameters, and they use those parameters correctly.
>> However, their parameter lists say EFI_HANDLE.
>>
>> Spell out the right type in the parameter lists.
>>
>> In practice, this change is a no-op (because, quite regrettably, both
>> EFI_HANDLE and SHELL_FILE_HANDLE are specified to be typedefs of
>> (VOID*)).
>>
>> Cc: Jaben Carsey <jaben.car...@intel.com>
>> Cc: Ray Ni <ray...@intel.com>
>> Cc: Zhichao Gao <zhichao....@intel.com>
>> Signed-off-by: Laszlo Ersek <ler...@redhat.com>
>> ---
>>
>> Notes:
>> tested: rm, touch
>>
>> ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c | 2 +-
>> ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c | 2 +-
>> 2 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
>> b/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
>> index 3a1196f1529e..59f7eec376f2 100644
>> --- a/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
>> +++ b/ShellPkg/Library/UefiShellLevel2CommandsLib/Rm.c
>> @@ -24,7 +24,7 @@ STATIC CONST SHELL_PARAM_ITEM ParamList[] = { **/
>> BOOLEAN IsDirectoryEmpty (
>> - IN EFI_HANDLE FileHandle
>> + IN SHELL_FILE_HANDLE FileHandle
>
> Here may be EFI_FILE_HANDLE.
Yes, it *may*, but doesn't *have* to.
We have the following call tree:
CascadeDelete()
IsDirectoryEmpty()
FileHandleFindFirstFile()
FileHandleFindNextFile()
With this patch, we have:
"Node->Handle"
|
[SHELL_FILE_HANDLE]
|
v
CascadeDelete()
|
[SHELL_FILE_HANDLE]
|
v
IsDirectoryEmpty()
|
[EFI_FILE_HANDLE]
|
v
FileHandleFindFirstFile()
FileHandleFindNextFile()
with your proposal, we would have:
"Node->Handle"
|
[SHELL_FILE_HANDLE]
|
v
CascadeDelete()
|
[EFI_FILE_HANDLE]
|
v
IsDirectoryEmpty()
|
[EFI_FILE_HANDLE]
|
v
FileHandleFindFirstFile()
FileHandleFindNextFile()
In both cases, we depend on SHELL_FILE_HANDLE being equivalent to
EFI_FILE_HANDLE. In the end, both types are:
(struct _EFI_FILE_PROTOCOL *)
In both cases, we go from SHELL_FILE_HANDLE to EFI_FILE_HANDLE, and
exploit that they are identical; the difference is only *where* we
exploit that.
- In this patch, we exploit the identity in IsDirectoryEmpty(): we take
a SHELL_FILE_HANDLE, and give it to functions that take EFI_FILE_HANDLE.
- In your proposal, we would exploit the exact same identity, just in
CascadeDelete(): "Node->Handle" is a SHELL_FILE_HANDLE (see
EFI_SHELL_FILE_INFO.Handle), and we'd pass it to a function (namely
IsDirectoryEmpty()) taking an EFI_FILE_HANDLE.
Given that your proposal wouldn't change our dependence on the
SHELL_FILE_HANDLE===EFI_FILE_HANDLE identity, I prefer to stick with the
current patch.
Thanks
Laszlo
>
>> )
>> {
>> EFI_STATUS Status;
>> diff --git a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
>> b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
>> index 0f00344c815e..a215f5774c69 100644
>> --- a/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
>> +++ b/ShellPkg/Library/UefiShellLevel3CommandsLib/Touch.c
>> @@ -21,7 +21,7 @@
>> **/
>> EFI_STATUS
>> TouchFileByHandle (
>> - IN EFI_HANDLE Handle
>> + IN SHELL_FILE_HANDLE Handle
>
> Here is OK.
>
> Thanks,
> Zhichao
>
>> )
>> {
>> EFI_STATUS Status;
>> --
>> 2.19.1.3.g30247aa5d201
>>
>>
>>
>> -=-=-=-=-=-=
>> Groups.io Links: You receive all messages sent to this group.
>>
>> View/Reply Online (#47417): https://edk2.groups.io/g/devel/message/47417
>> Mute This Topic: https://groups.io/mt/34180233/1768756
>> Group Owner: devel+ow...@edk2.groups.io
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub
>> [zhichao....@intel.com]
>> -=-=-=-=-=-=
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#48092): https://edk2.groups.io/g/devel/message/48092
Mute This Topic: https://groups.io/mt/34180233/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
