Re: [PATCH 0/3] ShellPkg/HandleParsingLib: Show LoadedImageProtocol info

["Ni, Ruiyu" <ruiyu.ni@intel.com>]

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