Thanks. I will send updated code change for the same library in IntelFrameworkModulePkg.
> -----Original Message----- > From: Gao, Liming [mailto:[email protected]] > Sent: Friday, December 04, 2015 3:32 AM > To: Anbazhagan, Baraneedharan; Kinney, Michael D; [email protected] > Subject: RE: MdeModulePkg: DebugAssert enhancement > > Reviewed-by: Liming Gao <[email protected]> > > I would like to help commit your serial patches if no other comments. > > Thanks > Liming > -----Original Message----- > From: Anbazhagan, Baraneedharan [mailto:[email protected]] > Sent: Friday, December 04, 2015 1:13 AM > To: Kinney, Michael D; Gao, Liming; [email protected] > 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:[email protected]] > > Sent: Wednesday, December 02, 2015 8:22 PM > > To: Gao, Liming; Anbazhagan, Baraneedharan; [email protected]; > > 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:[email protected]] On Behalf > > > Of Gao, Liming > > > Sent: Wednesday, December 2, 2015 6:07 PM > > > To: Anbazhagan, Baraneedharan <[email protected]>; > > > [email protected] > > > 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:[email protected]] On Behalf > > > Of Anbazhagan, Baraneedharan > > > Sent: Wednesday, December 02, 2015 11:42 PM > > > To: [email protected] > > > 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 <[email protected]> > > > --- > > > .../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/PeiDxeDebugLib > > > Re > > > portStatusCode.inf > > > b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLib > > > Re > > > portStatusCode.inf > > > index 5544667..50f60c7 100644 > > > --- > > > a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLib > > > Re > > > portStatusCode.inf > > > +++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebu > > > +++ gL > > > +++ ibReportStatusCode.inf > > > @@ -43,6 +43,7 @@ > > > BaseMemoryLib > > > BaseLib > > > DebugPrintErrorLevelLib > > > + PrintLib > > > > > > [Pcd] > > > gEfiMdePkgTokenSpaceGuid.PcdDebugClearMemoryValue ## > > > SOMETIMES_CONSUMES > > > -- > > > 2.6.3.windows.1 > > > > > > _______________________________________________ > > > edk2-devel mailing list > > > [email protected] > > > https://lists.01.org/mailman/listinfo/edk2-devel > > > _______________________________________________ > > > edk2-devel mailing list > > > [email protected] > > > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
