Re: [PATCH v2] ShellPkg/HandleParsingLib: Correct format specifier for LoadedImage

["Carsey, Jaben" <jaben.carsey@intel.com>]

I was unsure if printing the hex address of the load options was useful (useful enough?).

I know for some images it was nice to get the load options printed as a string since they are command line parameters. I guess that might be more focused on learning about the shell itself and how it was launched than other images.  Hence my question.

I am good with this patch.

Ray can you review and push it if you agree?

-Jaben

> -----Original Message-----
> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
> Sent: Wednesday, March 15, 2017 2:09 PM
> To: Carsey, Jaben <jaben.carsey@intel.com>
> Cc: Jeff Westfahl <jeff.westfahl@ni.com>; edk2-devel@lists.01.org; Ni, Ruiyu
> <ruiyu.ni@intel.com>
> Subject: RE: [PATCH v2] ShellPkg/HandleParsingLib: Correct format specifier
> for LoadedImage
> Importance: High
> 
> Jaben,
> 
> I think the output looks good with the udpated format. All of the output
> values are aligned, and it prints the hex address of the load options,
> just like it prints the hex address of the image address right below.
> 
> Regards,
> Jeff
> 
> On Wed, 15 Mar 2017, Carsey, Jaben wrote:
> 
> > Does the print call need to be updated to print this out properly?
> >
> > -Jaben
> >
> >> -----Original Message-----
> >> From: Jeff Westfahl [mailto:jeff.westfahl@ni.com]
> >> Sent: Tuesday, March 14, 2017 2:02 PM
> >> 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 v2] ShellPkg/HandleParsingLib: Correct format specifier
> for
> >> LoadedImage
> >> Importance: High
> >>
> >> The format specifier for the LoadOptions field of the LoadedImage
> protocol
> >> is "%s". However, the data in LoadOptions is often generic binary data. A
> >> format specifier of "%x" is more appropriate for this field.
> >>
> >> Using "dh -v" with format specifier "%s" on BIOS images based on EDK II
> >> source before commit 891d844 can cause a crash.
> >>
> >> 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.uni | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git
> a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> >> b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> >> index 0d51627..273a420 100644
> >> --- a/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> >> +++ b/ShellPkg/Library/UefiHandleParsingLib/UefiHandleParsingLib.uni
> >> @@ -354,7 +354,7 @@
> >>                                                    "     DeviceHandle..: %%H%x%%N\r\n"
> >>                                                    "     FilePath......: %%H%x%%N\r\n"
> >>                                                    "     OptionsSize...: %%H%x%%N\r\n"
> >> -                                                  "     LoadOptions...: %%H%s%%N\r\n"
> >> +                                                  "     LoadOptions...: %%H%x%%N\r\n"
> >>                                                    "     ImageBase.....: %%H%x%%N\r\n"
> >>                                                    "     ImageSize.....: %%H%Lx%%N\r\n"
> >>                                                    "     CodeType......: %%H%s%%N\r\n"
> >> --
> >> 2.7.4
> >
> >