I have mixed the section with IntelFrameworkModulePkg.
Seems the truncating behavior can work fine. And the comment of this section
should be updated.
Thanks,
Zhichao
> -----Original Message-----
> From: Wu, Hao A
> Sent: Thursday, March 21, 2019 3:06 PM
> To: Gao, Liming <liming....@intel.com>; Gao, Zhichao
> <zhichao....@intel.com>; edk2-devel@lists.01.org
> Cc: Wang, Jian J <jian.j.w...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng,
> Star <star.z...@intel.com>; Sean Brogan <sean.bro...@microsoft.com>;
> Michael Turner <michael.tur...@microsoft.com>; Bret Barkelew
> <bret.barke...@microsoft.com>
> Subject: RE: [PATCH V3 13/17]
> MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs
>
> > -----Original Message-----
> > From: Gao, Liming
> > Sent: Thursday, March 21, 2019 2:58 PM
> > To: Gao, Zhichao; edk2-devel@lists.01.org
> > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Sean Brogan; Michael
> > Turner; Bret Barkelew
> > Subject: RE: [PATCH V3 13/17]
> > MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs
> >
> > Zhichao:
> > Why do below change? Seemly, this change is not related to add new
> > APIs. In fact, current log is added as the purpose by commit
> > 137ed15511e2045a7333e33ae7f1e873ce1961dd.
>
> I would suggest to add another separate commit to refine the below
> comments to state the message truncating behavior here:
>
> //
> // If the TotalSize is larger than the maximum record size, then return
> ^^^^^^^^^^^
> //
> if (TotalSize > sizeof (Buffer)) {
> TotalSize = sizeof (Buffer);
> }
>
> Best Regards,
> Hao Wu
>
> >
> > if (TotalSize > sizeof (Buffer)) {
> > TotalSize = sizeof (Buffer); ==> return;
> > }
> >
> > Thanks
> > Liming
> > >-----Original Message-----
> > >From: Gao, Zhichao
> > >Sent: Tuesday, March 19, 2019 11:26 PM
> > >To: edk2-devel@lists.01.org
> > >Cc: Wang, Jian J <jian.j.w...@intel.com>; Wu, Hao A
> > ><hao.a...@intel.com>; Ni, Ray <ray...@intel.com>; Zeng, Star
> > ><star.z...@intel.com>; Gao, Liming <liming....@intel.com>; Sean
> > >Brogan <sean.bro...@microsoft.com>; Michael Turner
> > ><michael.tur...@microsoft.com>; Bret Barkelew
> > ><bret.barke...@microsoft.com>
> > >Subject: [PATCH V3 13/17]
> > >MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs
> > >
> > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1395
> > >
> > >Add new APIs' implementation (DebugVPrint, DebugBPrint) in the
> > >DebugLib instance. These APIs would expose print routines with VaList
> > >parameter and BaseList parameter.
> > >
> > >Contributed-under: TianoCore Contribution Agreement 1.1
> > >Signed-off-by: Zhichao Gao <zhichao....@intel.com>
> > >Cc: Jian J Wang <jian.j.w...@intel.com>
> > >Cc: Hao Wu <hao.a...@intel.com>
> > >Cc: Ray Ni <ray...@intel.com>
> > >Cc: Star Zeng <star.z...@intel.com>
> > >Cc: Liming Gao <liming....@intel.com>
> > >Cc: Sean Brogan <sean.bro...@microsoft.com>
> > >Cc: Michael Turner <michael.tur...@microsoft.com>
> > >Cc: Bret Barkelew <bret.barke...@microsoft.com>
> > >---
> > > .../PeiDxeDebugLibReportStatusCode/DebugLib.c | 144
> > >++++++++++++++++++---
> > > 1 file changed, 128 insertions(+), 16 deletions(-)
> > >
> > >diff --git
> > >a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
> > >b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
> > >index 6f0f416273..f1d31cb619 100644
> > >---
> a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
> > >+++
> > >b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c
> > >@@ -4,7 +4,7 @@
> > > Note that if the debug message length is larger than the maximum
> > allowable
> > > record length, then the debug message will be ignored directly.
> > >
> > >- Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > >reserved.<BR>
> > >+ Copyright (c) 2006 - 2019, Intel Corporation. All rights
> > >+ reserved.<BR>
> > > This program and the accompanying materials
> > > are licensed and made available under the terms and conditions of
> > >the BSD License
> > > which accompanies this distribution. The full text of the license
> > >may be found at @@ -27,6 +27,12 @@ #include <Library/PcdLib.h>
> > >#include <Library/DebugPrintErrorLevelLib.h>
> > >
> > >+//
> > >+// VA_LIST can not initialize to NULL for all compiler, so we use
> > >+this to // indicate a null VA_LIST //
> > >+VA_LIST mVaListNull;
> > >+
> > > /**
> > > Prints a debug message to the debug output device if the specified
> > >error level is enabled.
> > >
> > >@@ -52,12 +58,43 @@ DebugPrint (
> > > IN CONST CHAR8 *Format,
> > > ...
> > > )
> > >+{
> > >+ VA_LIST Marker;
> > >+
> > >+ VA_START (Marker, Format);
> > >+ DebugVPrint (ErrorLevel, Format, Marker);
> > >+ VA_END (Marker);
> > >+}
> > >+
> > >+/**
> > >+ Prints a debug message to the debug output device if the specified
> > >+ error level is enabled base on Null-terminated format string and a
> > >+ VA_LIST argument list or a BASE_LIST argument list.
> > >+
> > >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib
> > >+ function GetDebugPrintErrorLevel (), then print the message
> > >+ specified by Format
> > >and
> > >+ the associated variable argument list to the debug output device.
> > >+
> > >+ If Format is NULL, then ASSERT().
> > >+
> > >+ @param ErrorLevel The error level of the debug message.
> > >+ @param Format Format string for the debug message to print.
> > >+ @param VaListMarker VA_LIST marker for the variable argument list.
> > >+ @param BaseListMarker BASE_LIST marker for the variable argument
> list.
> > >+
> > >+**/
> > >+VOID
> > >+DebugPrintMarker (
> > >+ IN UINTN ErrorLevel,
> > >+ IN CONST CHAR8 *Format,
> > >+ IN VA_LIST VaListMarker,
> > >+ IN BASE_LIST BaseListMarker
> > >+ )
> > > {
> > > UINT64 Buffer[(EFI_STATUS_CODE_DATA_MAX_SIZE / sizeof
> (UINT64))
> > >+ 1];
> > > EFI_DEBUG_INFO *DebugInfo;
> > > UINTN TotalSize;
> > >- VA_LIST VaListMarker;
> > >- BASE_LIST BaseListMarker;
> > >+ BASE_LIST BaseListMarkerPointer;
> > > CHAR8 *FormatString;
> > > BOOLEAN Long;
> > >
> > >@@ -96,7 +133,7 @@ DebugPrint (
> > > // If the TotalSize is larger than the maximum record size, then return
> > > //
> > > if (TotalSize > sizeof (Buffer)) {
> > >- TotalSize = sizeof (Buffer);
> > >+ return;
> > > }
> > >
> > > //
> > >@@ -109,7 +146,7 @@ DebugPrint (
> > > //
> > > DebugInfo = (EFI_DEBUG_INFO *)(Buffer) + 1;
> > > DebugInfo->ErrorLevel = (UINT32)ErrorLevel;
> > >- BaseListMarker = (BASE_LIST)(DebugInfo + 1);
> > >+ BaseListMarkerPointer = (BASE_LIST)(DebugInfo + 1);
> > > FormatString = (CHAR8 *)((UINT64 *)(DebugInfo + 1) + 12);
> > >
> > > //
> > >@@ -125,7 +162,6 @@ DebugPrint (
> > > // of format in DEBUG string, which is followed by the DEBUG format
> string.
> > > // Here we will process the variable arguments and pack them in this
> area.
> > > //
> > >- VA_START (VaListMarker, Format);
> > >
> > > //
> > > // Use the actual format string.
> > >@@ -167,7 +203,11 @@ DebugPrint (
> > > // '*' in format field means the precision of the field is
> > > specified by
> > > // a UINTN argument in the argument list.
> > > //
> > >- BASE_ARG (BaseListMarker, UINTN) = VA_ARG (VaListMarker,
> UINTN);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, UINTN) = VA_ARG
> > >+ (VaListMarker,
> > >UINTN);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, UINTN) = BASE_ARG
> > >(BaseListMarker, UINTN);
> > >+ }
> > > continue;
> > > }
> > > if (*Format == '\0') {
> > >@@ -192,16 +232,36 @@ DebugPrint (
> > > }
> > > if (*Format == 'p' || *Format == 'X' || *Format == 'x' ||
> > >*Format == 'd' || *Format == 'u') {
> > > if (Long) {
> > >- BASE_ARG (BaseListMarker, INT64) = VA_ARG (VaListMarker, INT64);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, INT64) = VA_ARG
> > >+ (VaListMarker,
> > >INT64);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, INT64) = BASE_ARG
> > >(BaseListMarker, INT64);
> > >+ }
> > > } else {
> > >- BASE_ARG (BaseListMarker, int) = VA_ARG (VaListMarker, int);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, int) = VA_ARG (VaListMarker,
> int);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, int) = BASE_ARG
> > >+ (BaseListMarker,
> > >int);
> > >+ }
> > > }
> > > } else if (*Format == 's' || *Format == 'S' || *Format == 'a' ||
> > > *Format ==
> > 'g'
> > >|| *Format == 't') {
> > >- BASE_ARG (BaseListMarker, VOID *) = VA_ARG (VaListMarker, VOID
> *);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, VOID *) = VA_ARG
> > >+ (VaListMarker,
> > >VOID *);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, VOID *) = BASE_ARG
> > >(BaseListMarker, VOID *);
> > >+ }
> > > } else if (*Format == 'c') {
> > >- BASE_ARG (BaseListMarker, UINTN) = VA_ARG (VaListMarker,
> UINTN);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, UINTN) = VA_ARG
> > >+ (VaListMarker,
> > >UINTN);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, UINTN) = BASE_ARG
> > >(BaseListMarker, UINTN);
> > >+ }
> > > } else if (*Format == 'r') {
> > >- BASE_ARG (BaseListMarker, RETURN_STATUS) = VA_ARG
> (VaListMarker,
> > >RETURN_STATUS);
> > >+ if (BaseListMarker == NULL) {
> > >+ BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) = VA_ARG
> > >(VaListMarker, RETURN_STATUS);
> > >+ } else {
> > >+ BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) = BASE_ARG
> > >(BaseListMarker, RETURN_STATUS);
> > >+ }
> > > }
> > >
> > > //
> > >@@ -209,17 +269,15 @@ DebugPrint (
> > > // This indicates that the DEBUG() macro is passing in more
> > >argument than can be handled by
> > > // the EFI_DEBUG_INFO record
> > > //
> > >- ASSERT ((CHAR8 *)BaseListMarker <= FormatString);
> > >+ ASSERT ((CHAR8 *)BaseListMarkerPointer <= FormatString);
> > >
> > > //
> > > // If the converted BASE_LIST is larger than the 12 * sizeof
> > >(UINT64) allocated bytes, then return
> > > //
> > >- if ((CHAR8 *)BaseListMarker > FormatString) {
> > >- VA_END (VaListMarker);
> > >+ if ((CHAR8 *)BaseListMarkerPointer > FormatString) {
> > > return;
> > > }
> > > }
> > >- VA_END (VaListMarker);
> > >
> > > //
> > > // Send the DebugInfo record
> > >@@ -235,6 +293,60 @@ DebugPrint (
> > > );
> > > }
> > >
> > >+/**
> > >+ Prints a debug message to the debug output device if the specified
> > >+ error level is enabled.
> > >+
> > >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib
> > >+ function GetDebugPrintErrorLevel (), then print the message
> > >+ specified by Format
> > >and
> > >+ the associated variable argument list to the debug output device.
> > >+
> > >+ If Format is NULL, then ASSERT().
> > >+
> > >+ @param ErrorLevel The error level of the debug message.
> > >+ @param Format Format string for the debug message to print.
> > >+ @param VaListMarker VA_LIST marker for the variable argument list.
> > >+
> > >+**/
> > >+VOID
> > >+EFIAPI
> > >+DebugVPrint (
> > >+ IN UINTN ErrorLevel,
> > >+ IN CONST CHAR8 *Format,
> > >+ IN VA_LIST VaListMarker
> > >+ )
> > >+{
> > >+ DebugPrintMarker (ErrorLevel, Format, VaListMarker, NULL); }
> > >+
> > >+/**
> > >+ Prints a debug message to the debug output device if the specified
> > >+ error level is enabled.
> > >+ This function use BASE_LIST which would provide a more compatible
> > >+ service than VA_LIST.
> > >+
> > >+ If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib
> > >+ function GetDebugPrintErrorLevel (), then print the message
> > >+ specified by Format
> > >and
> > >+ the associated variable argument list to the debug output device.
> > >+
> > >+ If Format is NULL, then ASSERT().
> > >+
> > >+ @param ErrorLevel The error level of the debug message.
> > >+ @param Format Format string for the debug message to print.
> > >+ @param BaseListMarker BASE_LIST marker for the variable argument
> list.
> > >+
> > >+**/
> > >+VOID
> > >+EFIAPI
> > >+DebugBPrint (
> > >+ IN UINTN ErrorLevel,
> > >+ IN CONST CHAR8 *Format,
> > >+ IN BASE_LIST BaseListMarker
> > >+ )
> > >+{
> > >+ DebugPrintMarker (ErrorLevel, Format, mVaListNull,
> > >+BaseListMarker); }
> > >+
> > > /**
> > > Prints an assert message containing a filename, line number, and
> > description.
> > > This may be followed by a breakpoint or a dead loop.
> > >--
> > >2.16.2.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
