Reviewed-by: Liming Gao <liming....@intel.com> I would like to help commit your serial patches if no other comments.
Thanks Liming -----Original Message----- From: Anbazhagan, Baraneedharan [mailto:anbazha...@hp.com] Sent: Friday, December 04, 2015 1:13 AM To: Kinney, Michael D; Gao, Liming; edk2-devel@lists.01.org Subject: RE: MdeModulePkg: DebugAssert enhancement Updated code based on your suggestion to use copy mem and ModuleNameSize calculation. diff --git a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c index f1d9827..c3010df 100644 --- a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c +++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c @@ -261,6 +261,7 @@ DebugAssert ( UINTN HeaderSize; UINTN TotalSize; CHAR8 *Temp; + UINTN ModuleNameSize; UINTN FileNameSize; UINTN DescriptionSize; @@ -268,31 +269,40 @@ DebugAssert ( // Get string size // HeaderSize = sizeof (EFI_DEBUG_ASSERT_DATA); + // + // Compute string size of module name enclosed by [] // + ModuleNameSize = 2 + AsciiStrSize (gEfiCallerBaseName); FileNameSize = AsciiStrSize (FileName); DescriptionSize = AsciiStrSize (Description); // // Make sure it will all fit in the passed in buffer. // - if (HeaderSize + FileNameSize + DescriptionSize > sizeof (Buffer)) { + if (HeaderSize + ModuleNameSize + FileNameSize + DescriptionSize > + sizeof (Buffer)) { // - // FileName + Description is too long to be filled into buffer. + // remove module name if it's too long to be filled into buffer // - if (HeaderSize + FileNameSize < sizeof (Buffer)) { - // - // Description has enough buffer to be truncated. - // - DescriptionSize = sizeof (Buffer) - HeaderSize - FileNameSize; - } else { + ModuleNameSize = 0; + if (HeaderSize + FileNameSize + DescriptionSize > sizeof (Buffer)) + { // - // FileName is too long to be filled into buffer. - // FileName will be truncated. Reserved one byte for Description NULL terminator. + // FileName + Description is too long to be filled into buffer. // - DescriptionSize = 1; - FileNameSize = sizeof (Buffer) - HeaderSize - DescriptionSize; + if (HeaderSize + FileNameSize < sizeof (Buffer)) { + // + // Description has enough buffer to be truncated. + // + DescriptionSize = sizeof (Buffer) - HeaderSize - FileNameSize; + } else { + // + // FileName is too long to be filled into buffer. + // FileName will be truncated. Reserved one byte for Description NULL terminator. + // + DescriptionSize = 1; + FileNameSize = sizeof (Buffer) - HeaderSize - DescriptionSize; + } } } - // // Fill in EFI_DEBUG_ASSERT_DATA // @@ -300,12 +310,23 @@ DebugAssert ( AssertData->LineNumber = (UINT32)LineNumber; TotalSize = sizeof (EFI_DEBUG_ASSERT_DATA); + Temp = (CHAR8 *)(AssertData + 1); + + // + // Copy Ascii [ModuleName]. + // + if (ModuleNameSize != 0) { + CopyMem(Temp, "[", 1); + CopyMem(Temp + 1, gEfiCallerBaseName, ModuleNameSize - 3); + CopyMem(Temp + ModuleNameSize - 2, "] ", 2); } + // // Copy Ascii FileName including NULL terminator. // - Temp = CopyMem (AssertData + 1, FileName, FileNameSize); + Temp = CopyMem (Temp + ModuleNameSize, FileName, FileNameSize); Temp[FileNameSize - 1] = 0; - TotalSize += FileNameSize; + TotalSize += (ModuleNameSize + FileNameSize); // // Copy Ascii Description include NULL terminator. -- 2.6.3.windows.1 > -----Original Message----- > From: Kinney, Michael D [mailto:michael.d.kin...@intel.com] > Sent: Wednesday, December 02, 2015 8:22 PM > To: Gao, Liming; Anbazhagan, Baraneedharan; edk2-devel@lists.01.org; Kinney, > Michael D > Subject: RE: MdeModulePkg: DebugAssert enhancement > > Baranee, > > I also think the following line could be changed for smaller code gen from: > > + ModuleNameSize = AsciiStrLen("[") + AsciiStrLen (gEfiCallerBaseName) + > AsciiStrLen("] "); > > To: > > + // > + // Compute string size of module name enclosed by [] // > + ModuleNameSize = 2 + AsciiStrSize (gEfiCallerBaseName); > > Mike > > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Gao, Liming > > Sent: Wednesday, December 2, 2015 6:07 PM > > To: Anbazhagan, Baraneedharan <anbazha...@hp.com>; > > edk2-devel@lists.01.org > > Subject: Re: [edk2] MdeModulePkg: DebugAssert enhancement > > > > Baranee: > > Could you use CopyMem() to fill gEfiCallerBaseName instead of > > AsciiSPrint()? This library instance is for size optimization. So, it > > uses > > CopyMem() to fill FileName and Description. > > > > Thanks > > Liming > > -----Original Message----- > > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of > > Anbazhagan, Baraneedharan > > Sent: Wednesday, December 02, 2015 11:42 PM > > To: edk2-devel@lists.01.org > > Subject: [edk2] MdeModulePkg: DebugAssert enhancement > > > > If the assert happens in a library, then it's hard to determine which > > module using > that library is generating that assert. > > Use gEfiCallerBaseName in DebugAssert to display the module name. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Baraneedharan Anbazhagan <anbazha...@hp.com> > > --- > > .../PeiDxeDebugLibReportStatusCode/DebugLib.c | 58 > > ++++++++++++++-------- > > .../PeiDxeDebugLibReportStatusCode.inf | 1 + > > 2 files changed, 39 insertions(+), 20 deletions(-) > > > > diff --git > > a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > > b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > > index f1d9827..af369ea 100644 > > --- a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > > +++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > > @@ -22,6 +22,7 @@ > > > > #include <Library/DebugLib.h> > > #include <Library/BaseLib.h> > > +#include <Library/PrintLib.h> > > #include <Library/BaseMemoryLib.h> > > #include <Library/ReportStatusCodeLib.h> #include <Library/PcdLib.h> @@ - > 261,6 +262,7 @@ DebugAssert ( > > UINTN HeaderSize; > > UINTN TotalSize; > > CHAR8 *Temp; > > + UINTN ModuleNameSize; > > UINTN FileNameSize; > > UINTN DescriptionSize; > > > > @@ -268,31 +270,37 @@ DebugAssert ( > > // Get string size > > // > > HeaderSize = sizeof (EFI_DEBUG_ASSERT_DATA); > > + ModuleNameSize = AsciiStrLen("[") + AsciiStrLen (gEfiCallerBaseName) + > AsciiStrLen("] "); > > FileNameSize = AsciiStrSize (FileName); > > DescriptionSize = AsciiStrSize (Description); > > > > // > > // Make sure it will all fit in the passed in buffer. > > // > > - if (HeaderSize + FileNameSize + DescriptionSize > sizeof (Buffer)) > > { > > + if (HeaderSize + ModuleNameSize + FileNameSize + DescriptionSize > > > + sizeof (Buffer)) { > > // > > - // FileName + Description is too long to be filled into buffer. > > + // remove module name if it's too long to be filled into buffer > > // > > - if (HeaderSize + FileNameSize < sizeof (Buffer)) { > > + ModuleNameSize = 0; > > + if (HeaderSize + FileNameSize + DescriptionSize > sizeof > > + (Buffer)) { > > // > > - // Description has enough buffer to be truncated. > > + // FileName + Description is too long to be filled into buffer. > > // > > - DescriptionSize = sizeof (Buffer) - HeaderSize - FileNameSize; > > - } else { > > - // > > - // FileName is too long to be filled into buffer. > > - // FileName will be truncated. Reserved one byte for Description NULL > terminator. > > - // > > - DescriptionSize = 1; > > - FileNameSize = sizeof (Buffer) - HeaderSize - DescriptionSize; > > + if (HeaderSize + FileNameSize < sizeof (Buffer)) { > > + // > > + // Description has enough buffer to be truncated. > > + // > > + DescriptionSize = sizeof (Buffer) - HeaderSize - FileNameSize; > > + } else { > > + // > > + // FileName is too long to be filled into buffer. > > + // FileName will be truncated. Reserved one byte for Description > > NULL > terminator. > > + // > > + DescriptionSize = 1; > > + FileNameSize = sizeof (Buffer) - HeaderSize - DescriptionSize; > > + } > > } > > } > > - > > // > > // Fill in EFI_DEBUG_ASSERT_DATA > > // > > @@ -300,17 +308,27 @@ DebugAssert ( > > AssertData->LineNumber = (UINT32)LineNumber; > > TotalSize = sizeof (EFI_DEBUG_ASSERT_DATA); > > > > - // > > - // Copy Ascii FileName including NULL terminator. > > - // > > - Temp = CopyMem (AssertData + 1, FileName, FileNameSize); > > - Temp[FileNameSize - 1] = 0; > > - TotalSize += FileNameSize; > > + Temp = (CHAR8 *)(AssertData + 1); > > + if (ModuleNameSize != 0) { > > + // > > + // Copy Ascii ModuleName & FileName including NULL terminator > > + // > > + AsciiSPrint(Temp, ModuleNameSize + FileNameSize, "[%a] %a", > gEfiCallerBaseName, FileName); > > + TotalSize += (ModuleNameSize + FileNameSize); } else { > > + // > > + // Copy Ascii FileName including NULL terminator. > > + // > > + Temp = CopyMem (Temp, FileName, FileNameSize); > > + Temp[FileNameSize - 1] = 0; > > + TotalSize += FileNameSize; > > + } > > + > > > > // > > // Copy Ascii Description include NULL terminator. > > // > > - Temp = CopyMem (Temp + FileNameSize, Description, DescriptionSize); > > + Temp = CopyMem (Temp + ModuleNameSize + FileNameSize, Description, > > + DescriptionSize); > > Temp[DescriptionSize - 1] = 0; > > TotalSize += DescriptionSize; > > > > diff --git > > a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibRe > > portStatusCode.inf > > b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibRe > > portStatusCode.inf > > index 5544667..50f60c7 100644 > > --- > > a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibRe > > portStatusCode.inf > > +++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugL > > +++ ibReportStatusCode.inf > > @@ -43,6 +43,7 @@ > > BaseMemoryLib > > BaseLib > > DebugPrintErrorLevelLib > > + PrintLib > > > > [Pcd] > > gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## > > SOMETIMES_CONSUMES > > -- > > 2.6.3.windows.1 > > > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.01.org > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
