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

[Jeff Westfahl <jeff.westfahl@ni.com>]

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
>
>