From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.136; helo=mga12.intel.com; envelope-from=zhichao.gao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id F36922194EB70 for ; Thu, 21 Mar 2019 00:29:22 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Mar 2019 00:29:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,252,1549958400"; d="scan'208";a="154343845" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga004.fm.intel.com with ESMTP; 21 Mar 2019 00:29:21 -0700 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 00:29:21 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 00:29:06 -0700 Received: from shsmsx101.ccr.corp.intel.com ([169.254.1.158]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.80]) with mapi id 14.03.0415.000; Thu, 21 Mar 2019 15:29:04 +0800 From: "Gao, Zhichao" To: "Wu, Hao A" , "Gao, Liming" , "edk2-devel@lists.01.org" CC: "Wang, Jian J" , "Ni, Ray" , "Zeng, Star" , Sean Brogan , Michael Turner , Bret Barkelew Thread-Topic: [PATCH V3 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs Thread-Index: AQHU37NlMB+/5Zyl6keD+uJmD6eo26YVI5uAgACLqzA= Date: Thu, 21 Mar 2019 07:29:03 +0000 Message-ID: <3CE959C139B4C44DBEA1810E3AA6F9000B7B0B47@SHSMSX101.ccr.corp.intel.com> References: <20190319152549.16104-1-zhichao.gao@intel.com> <20190319152549.16104-14-zhichao.gao@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E408772@SHSMSX104.ccr.corp.intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTE4NTBlNTEtNmM5MC00M2JlLTkxMzItMGU1MTVjZmI4ODBhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiRUNLWXFCNGdzdmRxT2t1WmI5Z0xpYXZTMndHcnNqOGNKVVFGUTJZSVJ2aTZ4QmtHSm5ZcjFTSkRHYXhQXC9ObjIifQ== x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V3 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Mar 2019 07:29:23 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable I have mixed the section with IntelFrameworkModulePkg. Seems the truncating behavior can work fine. And the comment of this sectio= n should be updated. Thanks, Zhichao > -----Original Message----- > From: Wu, Hao A > Sent: Thursday, March 21, 2019 3:06 PM > To: Gao, Liming ; Gao, Zhichao > ; edk2-devel@lists.01.org > Cc: Wang, Jian J ; Ni, Ray ; Zen= g, > Star ; Sean Brogan ; > Michael Turner ; Bret Barkelew > > Subject: RE: [PATCH V3 13/17] > MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs >=20 > > -----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. >=20 > I would suggest to add another separate commit to refine the below > comments to state the message truncating behavior here: >=20 > // > // If the TotalSize is larger than the maximum record size, then return > ^^^^^^^^^^^ > // > if (TotalSize > sizeof (Buffer)) { > TotalSize =3D sizeof (Buffer); > } >=20 > Best Regards, > Hao Wu >=20 > > > > if (TotalSize > sizeof (Buffer)) { > > TotalSize =3D sizeof (Buffer); =3D=3D> 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 ; Wu, Hao A > > >; Ni, Ray ; Zeng, Star > > >; Gao, Liming ; Sean > > >Brogan ; Michael Turner > > >; Bret Barkelew > > > > > >Subject: [PATCH V3 13/17] > > >MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs > > > > > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1395 > > > > > >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 > > >Cc: Jian J Wang > > >Cc: Hao Wu > > >Cc: Ray Ni > > >Cc: Star Zeng > > >Cc: Liming Gao > > >Cc: Sean Brogan > > >Cc: Michael Turner > > >Cc: Bret Barkelew > > >--- > > > .../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.
> > >+ Copyright (c) 2006 - 2019, Intel Corporation. All rights > > >+ reserved.
> > > 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 > > >#include > > > > > >+// > > >+// 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 prin= t. > > >+ @param VaListMarker VA_LIST marker for the variable argument li= st. > > >+ @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 re= turn > > > // > > > if (TotalSize > sizeof (Buffer)) { > > >- TotalSize =3D sizeof (Buffer); > > >+ return; > > > } > > > > > > // > > >@@ -109,7 +146,7 @@ DebugPrint ( > > > // > > > DebugInfo =3D (EFI_DEBUG_INFO *)(Buffer) + 1; > > > DebugInfo->ErrorLevel =3D (UINT32)ErrorLevel; > > >- BaseListMarker =3D (BASE_LIST)(DebugInfo + 1); > > >+ BaseListMarkerPointer =3D (BASE_LIST)(DebugInfo + 1); > > > FormatString =3D (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 thi= s > 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 sp= ecified by > > > // a UINTN argument in the argument list. > > > // > > >- BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, > UINTN); > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D VA_ARG > > >+ (VaListMarker, > > >UINTN); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG > > >(BaseListMarker, UINTN); > > >+ } > > > continue; > > > } > > > if (*Format =3D=3D '\0') { > > >@@ -192,16 +232,36 @@ DebugPrint ( > > > } > > > if (*Format =3D=3D 'p' || *Format =3D=3D 'X' || *Format =3D=3D 'x= ' || > > >*Format =3D=3D 'd' || *Format =3D=3D 'u') { > > > if (Long) { > > >- BASE_ARG (BaseListMarker, INT64) =3D VA_ARG (VaListMarker, IN= T64); > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, INT64) =3D VA_ARG > > >+ (VaListMarker, > > >INT64); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, INT64) =3D BASE_ARG > > >(BaseListMarker, INT64); > > >+ } > > > } else { > > >- BASE_ARG (BaseListMarker, int) =3D VA_ARG (VaListMarker, int)= ; > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, int) =3D VA_ARG (VaListMar= ker, > int); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, int) =3D BASE_ARG > > >+ (BaseListMarker, > > >int); > > >+ } > > > } > > > } else if (*Format =3D=3D 's' || *Format =3D=3D 'S' || *Format = =3D=3D 'a' || > > > *Format =3D=3D > > 'g' > > >|| *Format =3D=3D 't') { > > >- BASE_ARG (BaseListMarker, VOID *) =3D VA_ARG (VaListMarker, VOI= D > *); > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, VOID *) =3D VA_ARG > > >+ (VaListMarker, > > >VOID *); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, VOID *) =3D BASE_ARG > > >(BaseListMarker, VOID *); > > >+ } > > > } else if (*Format =3D=3D 'c') { > > >- BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, > UINTN); > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D VA_ARG > > >+ (VaListMarker, > > >UINTN); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG > > >(BaseListMarker, UINTN); > > >+ } > > > } else if (*Format =3D=3D 'r') { > > >- BASE_ARG (BaseListMarker, RETURN_STATUS) =3D VA_ARG > (VaListMarker, > > >RETURN_STATUS); > > >+ if (BaseListMarker =3D=3D NULL) { > > >+ BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) =3D VA_ARG > > >(VaListMarker, RETURN_STATUS); > > >+ } else { > > >+ BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) =3D 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 <=3D FormatString); > > >+ ASSERT ((CHAR8 *)BaseListMarkerPointer <=3D 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 prin= t. > > >+ @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