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

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

I will do that.

Thanks/Ray

> -----Original Message-----
> From: Carsey, Jaben
> Sent: Thursday, March 16, 2017 6:28 AM
> To: Jeff Westfahl <jeff.westfahl@ni.com>
> Cc: edk2-devel@lists.01.org; Ni, Ruiyu <ruiyu.ni@intel.com>; Carsey, Jaben
> <jaben.carsey@intel.com>
> Subject: RE: [PATCH v2] ShellPkg/HandleParsingLib: Correct format specifier
> for LoadedImage
> 
> 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.un
> > >> +++ i
> > >> @@ -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
> > >
> > >