* [PATCH 1/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text
2017-05-04 21:53 [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Jeff Westfahl
@ 2017-05-04 21:53 ` Jeff Westfahl
2017-05-04 21:53 ` [PATCH 2/3] ShellPkg/HandleParsingLib: Open LoadedImageProtocol first Jeff Westfahl
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Westfahl @ 2017-05-04 21:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Westfahl, Ruiyu Ni, Jaben Carsey
This patch adds support for displaying a text representation of the file
path associated with a LoadedImageProtocol. This is a behavior that was
present in the old shell but has been lost in the new shell.
For example, using 'dh -v' in the old shell:
FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
FilePath......: \EFI\BOOT\BOOTX64.EFI
vs. the new shell:
FilePath......: 3A539018
FilePath......: 3A728718
This seems like useful information for the shell to display.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
---
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 6 +++++-
ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni | 2 +-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index da1d92f..2db8a3a 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -169,6 +169,7 @@ LoadedImageProtocolDumpInformation(
EFI_STATUS Status;
CHAR16 *RetVal;
CHAR16 *Temp;
+ CHAR16 *FilePath;
CHAR16 *CodeType;
CHAR16 *DataType;
@@ -197,6 +198,8 @@ LoadedImageProtocolDumpInformation(
return NULL;
}
+ FilePath = ConvertDevicePathToText(LoadedImage->FilePath, TRUE, TRUE);
+
DataType = ConvertMemoryType(LoadedImage->ImageDataType);
CodeType = ConvertMemoryType(LoadedImage->ImageCodeType);
@@ -207,7 +210,7 @@ LoadedImageProtocolDumpInformation(
LoadedImage->ParentHandle,
LoadedImage->SystemTable,
LoadedImage->DeviceHandle,
- LoadedImage->FilePath,
+ FilePath,
LoadedImage->LoadOptionsSize,
LoadedImage->LoadOptions,
LoadedImage->ImageBase,
@@ -219,6 +222,7 @@ LoadedImageProtocolDumpInformation(
SHELL_FREE_NON_NULL(Temp);
+ SHELL_FREE_NON_NULL(FilePath);
SHELL_FREE_NON_NULL(CodeType);
SHELL_FREE_NON_NULL(DataType);
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
index 273a420..7b3711d 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
@@ -352,7 +352,7 @@
" ParentHandle..: %%H%x%%N\r\n"
" SystemTable...: %%H%x%%N\r\n"
" DeviceHandle..: %%H%x%%N\r\n"
- " FilePath......: %%H%x%%N\r\n"
+ " FilePath......: %%H%s%%N\r\n"
" OptionsSize...: %%H%x%%N\r\n"
" LoadOptions...: %%H%x%%N\r\n"
" ImageBase.....: %%H%x%%N\r\n"
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/3] ShellPkg/HandleParsingLib: Open LoadedImageProtocol first
2017-05-04 21:53 [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Jeff Westfahl
2017-05-04 21:53 ` [PATCH 1/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text Jeff Westfahl
@ 2017-05-04 21:53 ` Jeff Westfahl
2017-05-04 21:53 ` [PATCH 3/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name Jeff Westfahl
2017-05-05 2:59 ` [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Ni, Ruiyu
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Westfahl @ 2017-05-04 21:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Westfahl, Ruiyu Ni, Jaben Carsey
This patch changes the order of operations to make sure we can open the
LoadedImageProtocol before getting the format string. This should not
affect functionality, and makes the next patch easier to review.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
---
.../Library/UefiHandleParsingLib/UefiHandleParsingLib.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 2db8a3a..c96f6dd 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -177,13 +177,6 @@ LoadedImageProtocolDumpInformation(
return (CatSPrint(NULL, L"LoadedImage"));
}
- HandleParsingHiiInit();
-
- Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), NULL);
- if (Temp == NULL) {
- return NULL;
- }
-
Status = gBS->OpenProtocol (
TheHandle,
&gEfiLoadedImageProtocolGuid,
@@ -194,7 +187,13 @@ LoadedImageProtocolDumpInformation(
);
if (EFI_ERROR (Status)) {
- SHELL_FREE_NON_NULL (Temp);
+ return NULL;
+ }
+
+ HandleParsingHiiInit();
+
+ Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), NULL);
+ if (Temp == NULL) {
return NULL;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 3/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name
2017-05-04 21:53 [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Jeff Westfahl
2017-05-04 21:53 ` [PATCH 1/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text Jeff Westfahl
2017-05-04 21:53 ` [PATCH 2/3] ShellPkg/HandleParsingLib: Open LoadedImageProtocol first Jeff Westfahl
@ 2017-05-04 21:53 ` Jeff Westfahl
2017-05-05 2:59 ` [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Ni, Ruiyu
3 siblings, 0 replies; 7+ messages in thread
From: Jeff Westfahl @ 2017-05-04 21:53 UTC (permalink / raw)
To: edk2-devel; +Cc: Jeff Westfahl, Ruiyu Ni, Jaben Carsey
This patch adds support for showing the file name associated with a
LoadedImageProtocol file path. This is a behavior that was present in
the old shell but has been lost in the new shell.
For example, using 'dh -v' in the old shell:
Handle D3 (3A552218)
Image (3A54C918) File:MicrocodeUpdate
ParentHandle..: 3A666398
vs. the new shell:
D3: 3A552218
LoadedImage
Revision......: 0x00001000
ParentHandle..: 3A666398
Here's what the output of 'dh -v' looks like after this patch:
D3: 3A552218
LoadedImage
Name..........: MicrocodeUpdate
Revision......: 0x00001000
ParentHandle..: 3A666398
This seems like useful information for the shell to display.
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Jaben Carsey <jaben.carsey@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Westfahl <jeff.westfahl@ni.com>
---
.../UefiHandleParsingLib/UefiHandleParsingLib.c | 90 +++++++++++++++++++++-
.../UefiHandleParsingLib/UefiHandleParsingLib.uni | 2 +
2 files changed, 91 insertions(+), 1 deletion(-)
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index c96f6dd..74934f8 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -16,12 +16,85 @@
#include "UefiHandleParsingLib.h"
#include "IndustryStandard/Acpi10.h"
+#include "Pi/PiFirmwareFile.h"
+#include "Pi/PiFirmwareVolume.h"
+#include "Protocol/FirmwareVolume2.h"
EFI_HANDLE mHandleParsingHiiHandle = NULL;
HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
GUID_INFO_BLOCK *mGuidList;
UINTN mGuidListCount;
/**
+ Function to find the file name associated with a LoadedImageProtocol.
+
+ @param[in] LoadedImage An instance of LoadedImageProtocol.
+
+ @retval A string representation of the file name associated
+ with LoadedImage, or NULL if no name can be found.
+**/
+CHAR16*
+FindLoadedImageFileName (
+ IN EFI_LOADED_IMAGE_PROTOCOL *LoadedImage
+ )
+{
+ EFI_GUID *NameGuid;
+ EFI_STATUS Status;
+ EFI_FIRMWARE_VOLUME2_PROTOCOL *Fv;
+ VOID *Buffer;
+ UINTN BufferSize;
+ UINT32 AuthenticationStatus;
+
+ //
+ // Only subtype MEDIA_PIWG_FW_FILE_DP of type MEDIA_DEVICE_PATH is supported.
+ //
+ if ( (LoadedImage == NULL)
+ || (LoadedImage->FilePath == NULL)
+ || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
+ || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
+ ) {
+ return (NULL);
+ }
+
+ NameGuid = EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)LoadedImage->FilePath);
+
+ if (NameGuid == NULL) {
+ return (NULL);
+ }
+
+ //
+ // Get the FirmwareVolume2Protocol of the device handle that this image was
+ // loaded from.
+ //
+ Status = gBS->HandleProtocol(
+ LoadedImage->DeviceHandle,
+ &gEfiFirmwareVolume2ProtocolGuid,
+ (VOID**)&Fv);
+
+ //
+ // FirmwareVolume2Protocol is PI, and is not required to be available.
+ //
+ if (EFI_ERROR(Status)) {
+ return (NULL);
+ }
+
+ //
+ // Read the user interface section of the image.
+ //
+ Buffer = NULL;
+ Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE, 0, &Buffer, &BufferSize, &AuthenticationStatus);
+
+ if (EFI_ERROR(Status)) {
+ return (NULL);
+ }
+
+ //
+ // ReadSection returns just the section data, without any section header. For
+ // a user interface section, the only data is the file name.
+ //
+ return (Buffer);
+}
+
+/**
Function to translate the EFI_MEMORY_TYPE into a string.
@param[in] Memory The memory type.
@@ -169,6 +242,7 @@ LoadedImageProtocolDumpInformation(
EFI_STATUS Status;
CHAR16 *RetVal;
CHAR16 *Temp;
+ CHAR16 *FileName;
CHAR16 *FilePath;
CHAR16 *CodeType;
CHAR16 *DataType;
@@ -192,6 +266,20 @@ LoadedImageProtocolDumpInformation(
HandleParsingHiiInit();
+ FileName = FindLoadedImageFileName(LoadedImage);
+
+ RetVal = NULL;
+ if (FileName) {
+ Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
+
+ if (Temp != NULL) {
+ RetVal = CatSPrint(NULL, Temp, FileName);
+ }
+
+ SHELL_FREE_NON_NULL(Temp);
+ SHELL_FREE_NON_NULL(FileName);
+ }
+
Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_MAIN), NULL);
if (Temp == NULL) {
return NULL;
@@ -203,7 +291,7 @@ LoadedImageProtocolDumpInformation(
CodeType = ConvertMemoryType(LoadedImage->ImageCodeType);
RetVal = CatSPrint(
- NULL,
+ RetVal,
Temp,
LoadedImage->Revision,
LoadedImage->ParentHandle,
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
index 7b3711d..e07c9a1 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
@@ -348,6 +348,8 @@
+#string STR_LI_DUMP_NAME #language en-US " Name..........: %%H%s%%N\r\n"
+
#string STR_LI_DUMP_MAIN #language en-US " Revision......: %%H0x%08x%%N\r\n"
" ParentHandle..: %%H%x%%N\r\n"
" SystemTable...: %%H%x%%N\r\n"
--
2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info
2017-05-04 21:53 [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Jeff Westfahl
` (2 preceding siblings ...)
2017-05-04 21:53 ` [PATCH 3/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name Jeff Westfahl
@ 2017-05-05 2:59 ` Ni, Ruiyu
2017-05-05 12:25 ` Jeff Westfahl
3 siblings, 1 reply; 7+ messages in thread
From: Ni, Ruiyu @ 2017-05-05 2:59 UTC (permalink / raw)
To: Jeff Westfahl, edk2-devel@lists.01.org; +Cc: Carsey, Jaben
Jeff,
Firstly great thanks for filling the gap between EDK shell and UEFI shell.
There are many behavior differences between the two, and EDK Shell does carry much more information that
are very helpful for developer debugging.
Before I check in your patch, could you please kindly review whether the additional change is ok to you.
One change is to fix the ECC failure: "python BaseTools\Source\Python\Ecc\Ecc.py -t ShellPkg\Library\UefiHandleParsingLib -s"
ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): [3002]Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=) Predicate Expression: FileName
ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): [3003]A comparison of any pointer to zero must be done via the NULL type Predicate Expression: FileName
====patch content===
.../UefiHandleParsingLib/UefiHandleParsingLib.c | 39 ++++++++--------------
1 file changed, 14 insertions(+), 25 deletions(-)
diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
index 74934f8617..d3ee068eba 100644
--- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
+++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
@@ -16,14 +16,14 @@
#include "UefiHandleParsingLib.h"
#include "IndustryStandard/Acpi10.h"
-#include "Pi/PiFirmwareFile.h"
-#include "Pi/PiFirmwareVolume.h"
-#include "Protocol/FirmwareVolume2.h"
+#include <PiDxe.h>
+#include <Protocol/FirmwareVolume2.h>
EFI_HANDLE mHandleParsingHiiHandle = NULL;
HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
GUID_INFO_BLOCK *mGuidList;
UINTN mGuidListCount;
+
/**
Function to find the file name associated with a LoadedImageProtocol.
@@ -44,37 +44,26 @@ FindLoadedImageFileName (
UINTN BufferSize;
UINT32 AuthenticationStatus;
- //
- // Only subtype MEDIA_PIWG_FW_FILE_DP of type MEDIA_DEVICE_PATH is supported.
- //
- if ( (LoadedImage == NULL)
- || (LoadedImage->FilePath == NULL)
- || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
- || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
- ) {
- return (NULL);
+ if ((LoadedImage == NULL) || (LoadedImage->FilePath == NULL)) {
+ return NULL;
}
NameGuid = EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)LoadedImage->FilePath);
if (NameGuid == NULL) {
- return (NULL);
+ return NULL;
}
//
- // Get the FirmwareVolume2Protocol of the device handle that this image was
- // loaded from.
+ // Get the FirmwareVolume2Protocol of the device handle that this image was loaded from.
//
- Status = gBS->HandleProtocol(
- LoadedImage->DeviceHandle,
- &gEfiFirmwareVolume2ProtocolGuid,
- (VOID**)&Fv);
+ Status = gBS->HandleProtocol (LoadedImage->DeviceHandle, &gEfiFirmwareVolume2ProtocolGuid, (VOID**) &Fv);
//
// FirmwareVolume2Protocol is PI, and is not required to be available.
//
- if (EFI_ERROR(Status)) {
- return (NULL);
+ if (EFI_ERROR (Status)) {
+ return NULL;
}
//
@@ -83,15 +72,15 @@ FindLoadedImageFileName (
Buffer = NULL;
Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE, 0, &Buffer, &BufferSize, &AuthenticationStatus);
- if (EFI_ERROR(Status)) {
- return (NULL);
+ if (EFI_ERROR (Status)) {
+ return NULL;
}
//
// ReadSection returns just the section data, without any section header. For
// a user interface section, the only data is the file name.
//
- return (Buffer);
+ return Buffer;
}
/**
@@ -269,7 +258,7 @@ LoadedImageProtocolDumpInformation(
FileName = FindLoadedImageFileName(LoadedImage);
RetVal = NULL;
- if (FileName) {
+ if (FileName != NULL) {
Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
if (Temp != NULL) {
Thanks/Ray
> -----Original Message-----
> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
> Sent: Friday, May 5, 2017 5:53 AM
> To: edk2-devel@lists.01.org
> Cc: Jeff Westfahl <jeff.westfahl@ni.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
> Carsey, Jaben <jaben.carsey@intel.com>
> Subject: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol
> info
>
> In the old shell, 'dh -v' showed some useful information about the file path
> associated with a LoadedImageProtocol that is no longer shown in the modern
> shell.
>
> For example, with the old shell:
>
> Handle D3 (3A552218)
> Image (3A54C918) File:MicrocodeUpdate
> ParentHandle..: 3A666398
> SystemTable...: 3D2A8F18
> DeviceHandle..: 3B1C8098
> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> ImageBase.....: 3D650000 - 3D655540
> ImageSize.....: 5540
> CodeType......: RT_code
> DataType......: RT_data
>
> compared to the new shell:
>
> D3: 3A552218
> LoadedImage
> Revision......: 0x00001000
> ParentHandle..: 3A666398
> SystemTable...: 3D2A8F18
> DeviceHandle..: 3B1C8098
> FilePath......: 3A552298
> OptionsSize...: 0
> LoadOptions...: 0
> ImageBase.....: 3D650000
> ImageSize.....: 5540
> CodeType......: EfiRuntimeServicesCode
> DataType......: EfiRuntimeServicesData
> Unload........: 0
>
> Here is the output for the same handle with this series applied:
>
> D3: 3A552218
> LoadedImage
> Name..........: MicrocodeUpdate
> Revision......: 0x00001000
> ParentHandle..: 3A666398
> SystemTable...: 3D2A8F18
> DeviceHandle..: 3B1C8098
> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> OptionsSize...: 0
> LoadOptions...: 0
> ImageBase.....: 3D650000
> ImageSize.....: 5540
> CodeType......: EfiRuntimeServicesCode
> DataType......: EfiRuntimeServicesData
> Unload........: 0
>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Jaben Carsey <jaben.carsey@intel.com>
>
> Jeff Westfahl (3):
> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text
> ShellPkg/HandleParsingLib: Open LoadedImageProtocol first
> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name
>
> .../UefiHandleParsingLib/UefiHandleParsingLib.c | 111
> +++++++++++++++++++--
> .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +-
> 2 files changed, 104 insertions(+), 11 deletions(-)
>
> --
> 2.7.4
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info
2017-05-05 2:59 ` [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info Ni, Ruiyu
@ 2017-05-05 12:25 ` Jeff Westfahl
2017-05-05 15:49 ` Carsey, Jaben
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Westfahl @ 2017-05-05 12:25 UTC (permalink / raw)
To: Ni, Ruiyu; +Cc: Jeff Westfahl, edk2-devel@lists.01.org, Carsey, Jaben
Hi Ray,
Thank you for pointing out the ECC tool. These changes look good to me.
Reviewed-by: Jeff Westfahl <jeff.westfahl@ni.com>
On Fri, 5 May 2017, Ni, Ruiyu wrote:
> Jeff,
> Firstly great thanks for filling the gap between EDK shell and UEFI shell.
> There are many behavior differences between the two, and EDK Shell does carry much more information that
> are very helpful for developer debugging.
>
> Before I check in your patch, could you please kindly review whether the additional change is ok to you.
> One change is to fix the ECC failure: "python BaseTools\Source\Python\Ecc\Ecc.py -t ShellPkg\Library\UefiHandleParsingLib -s"
>
> ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): [3002]Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=) Predicate Expression: FileName
> ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272): [3003]A comparison of any pointer to zero must be done via the NULL type Predicate Expression: FileName
>
> ====patch content===
> .../UefiHandleParsingLib/UefiHandleParsingLib.c | 39 ++++++++--------------
> 1 file changed, 14 insertions(+), 25 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> index 74934f8617..d3ee068eba 100644
> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> @@ -16,14 +16,14 @@
>
> #include "UefiHandleParsingLib.h"
> #include "IndustryStandard/Acpi10.h"
> -#include "Pi/PiFirmwareFile.h"
> -#include "Pi/PiFirmwareVolume.h"
> -#include "Protocol/FirmwareVolume2.h"
> +#include <PiDxe.h>
> +#include <Protocol/FirmwareVolume2.h>
>
> EFI_HANDLE mHandleParsingHiiHandle = NULL;
> HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
> GUID_INFO_BLOCK *mGuidList;
> UINTN mGuidListCount;
> +
> /**
> Function to find the file name associated with a LoadedImageProtocol.
>
> @@ -44,37 +44,26 @@ FindLoadedImageFileName (
> UINTN BufferSize;
> UINT32 AuthenticationStatus;
>
> - //
> - // Only subtype MEDIA_PIWG_FW_FILE_DP of type MEDIA_DEVICE_PATH is supported.
> - //
> - if ( (LoadedImage == NULL)
> - || (LoadedImage->FilePath == NULL)
> - || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
> - || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
> - ) {
> - return (NULL);
> + if ((LoadedImage == NULL) || (LoadedImage->FilePath == NULL)) {
> + return NULL;
> }
>
> NameGuid = EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_DEVICE_PATH *)LoadedImage->FilePath);
>
> if (NameGuid == NULL) {
> - return (NULL);
> + return NULL;
> }
>
> //
> - // Get the FirmwareVolume2Protocol of the device handle that this image was
> - // loaded from.
> + // Get the FirmwareVolume2Protocol of the device handle that this image was loaded from.
> //
> - Status = gBS->HandleProtocol(
> - LoadedImage->DeviceHandle,
> - &gEfiFirmwareVolume2ProtocolGuid,
> - (VOID**)&Fv);
> + Status = gBS->HandleProtocol (LoadedImage->DeviceHandle, &gEfiFirmwareVolume2ProtocolGuid, (VOID**) &Fv);
>
> //
> // FirmwareVolume2Protocol is PI, and is not required to be available.
> //
> - if (EFI_ERROR(Status)) {
> - return (NULL);
> + if (EFI_ERROR (Status)) {
> + return NULL;
> }
>
> //
> @@ -83,15 +72,15 @@ FindLoadedImageFileName (
> Buffer = NULL;
> Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE, 0, &Buffer, &BufferSize, &AuthenticationStatus);
>
> - if (EFI_ERROR(Status)) {
> - return (NULL);
> + if (EFI_ERROR (Status)) {
> + return NULL;
> }
>
> //
> // ReadSection returns just the section data, without any section header. For
> // a user interface section, the only data is the file name.
> //
> - return (Buffer);
> + return Buffer;
> }
>
> /**
> @@ -269,7 +258,7 @@ LoadedImageProtocolDumpInformation(
> FileName = FindLoadedImageFileName(LoadedImage);
>
> RetVal = NULL;
> - if (FileName) {
> + if (FileName != NULL) {
> Temp = HiiGetString(mHandleParsingHiiHandle, STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
>
> if (Temp != NULL) {
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
>> Sent: Friday, May 5, 2017 5:53 AM
>> To: edk2-devel@lists.01.org
>> Cc: Jeff Westfahl <jeff.westfahl@ni.com>; Ni, Ruiyu <ruiyu.ni@intel.com>;
>> Carsey, Jaben <jaben.carsey@intel.com>
>> Subject: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol
>> info
>>
>> In the old shell, 'dh -v' showed some useful information about the file path
>> associated with a LoadedImageProtocol that is no longer shown in the modern
>> shell.
>>
>> For example, with the old shell:
>>
>> Handle D3 (3A552218)
>> Image (3A54C918) File:MicrocodeUpdate
>> ParentHandle..: 3A666398
>> SystemTable...: 3D2A8F18
>> DeviceHandle..: 3B1C8098
>> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
>> ImageBase.....: 3D650000 - 3D655540
>> ImageSize.....: 5540
>> CodeType......: RT_code
>> DataType......: RT_data
>>
>> compared to the new shell:
>>
>> D3: 3A552218
>> LoadedImage
>> Revision......: 0x00001000
>> ParentHandle..: 3A666398
>> SystemTable...: 3D2A8F18
>> DeviceHandle..: 3B1C8098
>> FilePath......: 3A552298
>> OptionsSize...: 0
>> LoadOptions...: 0
>> ImageBase.....: 3D650000
>> ImageSize.....: 5540
>> CodeType......: EfiRuntimeServicesCode
>> DataType......: EfiRuntimeServicesData
>> Unload........: 0
>>
>> Here is the output for the same handle with this series applied:
>>
>> D3: 3A552218
>> LoadedImage
>> Name..........: MicrocodeUpdate
>> Revision......: 0x00001000
>> ParentHandle..: 3A666398
>> SystemTable...: 3D2A8F18
>> DeviceHandle..: 3B1C8098
>> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
>> OptionsSize...: 0
>> LoadOptions...: 0
>> ImageBase.....: 3D650000
>> ImageSize.....: 5540
>> CodeType......: EfiRuntimeServicesCode
>> DataType......: EfiRuntimeServicesData
>> Unload........: 0
>>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Jaben Carsey <jaben.carsey@intel.com>
>>
>> Jeff Westfahl (3):
>> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text
>> ShellPkg/HandleParsingLib: Open LoadedImageProtocol first
>> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name
>>
>> .../UefiHandleParsingLib/UefiHandleParsingLib.c | 111
>> +++++++++++++++++++--
>> .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +-
>> 2 files changed, 104 insertions(+), 11 deletions(-)
>>
>> --
>> 2.7.4
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info
2017-05-05 12:25 ` Jeff Westfahl
@ 2017-05-05 15:49 ` Carsey, Jaben
0 siblings, 0 replies; 7+ messages in thread
From: Carsey, Jaben @ 2017-05-05 15:49 UTC (permalink / raw)
To: Jeff Westfahl, Ni, Ruiyu; +Cc: edk2-devel@lists.01.org
I agree with Ray's commentary. Good positive changes to UEFI Shell!
For the series.
Reviewed-by: Jaben Carsey <jaben.carsey@intel.com>
> -----Original Message-----
> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
> Sent: Friday, May 05, 2017 5:25 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>
> Cc: Jeff Westfahl <jeff.westfahl@ni.com>; edk2-devel@lists.01.org; Carsey,
> Jaben <jaben.carsey@intel.com>
> Subject: RE: [PATCH 0/3] ShellPkg/HandleParsingLib: Show
> LoadedImageProtocol info
> Importance: High
>
> Hi Ray,
>
> Thank you for pointing out the ECC tool. These changes look good to me.
>
> Reviewed-by: Jeff Westfahl <jeff.westfahl@ni.com>
>
> On Fri, 5 May 2017, Ni, Ruiyu wrote:
>
> > Jeff,
> > Firstly great thanks for filling the gap between EDK shell and UEFI shell.
> > There are many behavior differences between the two, and EDK Shell
> does carry much more information that
> > are very helpful for developer debugging.
> >
> > Before I check in your patch, could you please kindly review whether the
> additional change is ok to you.
> > One change is to fix the ECC failure: "python
> BaseTools\Source\Python\Ecc\Ecc.py -t
> ShellPkg\Library\UefiHandleParsingLib -s"
> >
> > ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272):
> [3002]Non-Boolean comparisons should use a compare operator (==, !=, >, <
> >=, <=) Predicate Expression: FileName
> > ShellPkg\Library\UefiHandleParsingLib\UefiHandleParsingLib.c(272):
> [3003]A comparison of any pointer to zero must be done via the NULL type
> Predicate Expression: FileName
> >
> > ====patch content===
> > .../UefiHandleParsingLib/UefiHandleParsingLib.c | 39 ++++++++------------
> --
> > 1 file changed, 14 insertions(+), 25 deletions(-)
> >
> > diff --git a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > index 74934f8617..d3ee068eba 100644
> > --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.c
> > @@ -16,14 +16,14 @@
> >
> > #include "UefiHandleParsingLib.h"
> > #include "IndustryStandard/Acpi10.h"
> > -#include "Pi/PiFirmwareFile.h"
> > -#include "Pi/PiFirmwareVolume.h"
> > -#include "Protocol/FirmwareVolume2.h"
> > +#include <PiDxe.h>
> > +#include <Protocol/FirmwareVolume2.h>
> >
> > EFI_HANDLE mHandleParsingHiiHandle = NULL;
> > HANDLE_INDEX_LIST mHandleList = {{{NULL,NULL},0,0},0};
> > GUID_INFO_BLOCK *mGuidList;
> > UINTN mGuidListCount;
> > +
> > /**
> > Function to find the file name associated with a LoadedImageProtocol.
> >
> > @@ -44,37 +44,26 @@ FindLoadedImageFileName (
> > UINTN BufferSize;
> > UINT32 AuthenticationStatus;
> >
> > - //
> > - // Only subtype MEDIA_PIWG_FW_FILE_DP of type
> MEDIA_DEVICE_PATH is supported.
> > - //
> > - if ( (LoadedImage == NULL)
> > - || (LoadedImage->FilePath == NULL)
> > - || (LoadedImage->FilePath->Type != MEDIA_DEVICE_PATH)
> > - || (LoadedImage->FilePath->SubType != MEDIA_PIWG_FW_FILE_DP)
> > - ) {
> > - return (NULL);
> > + if ((LoadedImage == NULL) || (LoadedImage->FilePath == NULL)) {
> > + return NULL;
> > }
> >
> > NameGuid =
> EfiGetNameGuidFromFwVolDevicePathNode((MEDIA_FW_VOL_FILEPATH_
> DEVICE_PATH *)LoadedImage->FilePath);
> >
> > if (NameGuid == NULL) {
> > - return (NULL);
> > + return NULL;
> > }
> >
> > //
> > - // Get the FirmwareVolume2Protocol of the device handle that this
> image was
> > - // loaded from.
> > + // Get the FirmwareVolume2Protocol of the device handle that this
> image was loaded from.
> > //
> > - Status = gBS->HandleProtocol(
> > - LoadedImage->DeviceHandle,
> > - &gEfiFirmwareVolume2ProtocolGuid,
> > - (VOID**)&Fv);
> > + Status = gBS->HandleProtocol (LoadedImage->DeviceHandle,
> &gEfiFirmwareVolume2ProtocolGuid, (VOID**) &Fv);
> >
> > //
> > // FirmwareVolume2Protocol is PI, and is not required to be available.
> > //
> > - if (EFI_ERROR(Status)) {
> > - return (NULL);
> > + if (EFI_ERROR (Status)) {
> > + return NULL;
> > }
> >
> > //
> > @@ -83,15 +72,15 @@ FindLoadedImageFileName (
> > Buffer = NULL;
> > Status = Fv->ReadSection(Fv, NameGuid, EFI_SECTION_USER_INTERFACE,
> 0, &Buffer, &BufferSize, &AuthenticationStatus);
> >
> > - if (EFI_ERROR(Status)) {
> > - return (NULL);
> > + if (EFI_ERROR (Status)) {
> > + return NULL;
> > }
> >
> > //
> > // ReadSection returns just the section data, without any section header.
> For
> > // a user interface section, the only data is the file name.
> > //
> > - return (Buffer);
> > + return Buffer;
> > }
> >
> > /**
> > @@ -269,7 +258,7 @@ LoadedImageProtocolDumpInformation(
> > FileName = FindLoadedImageFileName(LoadedImage);
> >
> > RetVal = NULL;
> > - if (FileName) {
> > + if (FileName != NULL) {
> > Temp = HiiGetString(mHandleParsingHiiHandle,
> STRING_TOKEN(STR_LI_DUMP_NAME), NULL);
> >
> > if (Temp != NULL) {
> >
> > Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
> >> Sent: Friday, May 5, 2017 5:53 AM
> >> To: edk2-devel@lists.01.org
> >> Cc: Jeff Westfahl <jeff.westfahl@ni.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>;
> >> Carsey, Jaben <jaben.carsey@intel.com>
> >> Subject: [PATCH 0/3] ShellPkg/HandleParsingLib: Show
> LoadedImageProtocol
> >> info
> >>
> >> In the old shell, 'dh -v' showed some useful information about the file
> path
> >> associated with a LoadedImageProtocol that is no longer shown in the
> modern
> >> shell.
> >>
> >> For example, with the old shell:
> >>
> >> Handle D3 (3A552218)
> >> Image (3A54C918) File:MicrocodeUpdate
> >> ParentHandle..: 3A666398
> >> SystemTable...: 3D2A8F18
> >> DeviceHandle..: 3B1C8098
> >> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> >> ImageBase.....: 3D650000 - 3D655540
> >> ImageSize.....: 5540
> >> CodeType......: RT_code
> >> DataType......: RT_data
> >>
> >> compared to the new shell:
> >>
> >> D3: 3A552218
> >> LoadedImage
> >> Revision......: 0x00001000
> >> ParentHandle..: 3A666398
> >> SystemTable...: 3D2A8F18
> >> DeviceHandle..: 3B1C8098
> >> FilePath......: 3A552298
> >> OptionsSize...: 0
> >> LoadOptions...: 0
> >> ImageBase.....: 3D650000
> >> ImageSize.....: 5540
> >> CodeType......: EfiRuntimeServicesCode
> >> DataType......: EfiRuntimeServicesData
> >> Unload........: 0
> >>
> >> Here is the output for the same handle with this series applied:
> >>
> >> D3: 3A552218
> >> LoadedImage
> >> Name..........: MicrocodeUpdate
> >> Revision......: 0x00001000
> >> ParentHandle..: 3A666398
> >> SystemTable...: 3D2A8F18
> >> DeviceHandle..: 3B1C8098
> >> FilePath......: FvFile(F3331DE6-4A55-44E4-B767-7453F7A1A021)
> >> OptionsSize...: 0
> >> LoadOptions...: 0
> >> ImageBase.....: 3D650000
> >> ImageSize.....: 5540
> >> CodeType......: EfiRuntimeServicesCode
> >> DataType......: EfiRuntimeServicesData
> >> Unload........: 0
> >>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Jaben Carsey <jaben.carsey@intel.com>
> >>
> >> Jeff Westfahl (3):
> >> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file path as text
> >> ShellPkg/HandleParsingLib: Open LoadedImageProtocol first
> >> ShellPkg/HandleParsingLib: Show LoadedImageProtocol file name
> >>
> >> .../UefiHandleParsingLib/UefiHandleParsingLib.c | 111
> >> +++++++++++++++++++--
> >> .../UefiHandleParsingLib/UefiHandleParsingLib.uni | 4 +-
> >> 2 files changed, 104 insertions(+), 11 deletions(-)
> >>
> >> --
> >> 2.7.4
> >
> >
^ permalink raw reply [flat|nested] 7+ messages in thread