From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 2C87D211E00F5 for ; Wed, 20 Mar 2019 09:57:59 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 79304C04FFEE; Wed, 20 Mar 2019 16:57:58 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-169.rdu2.redhat.com [10.10.120.169]) by smtp.corp.redhat.com (Postfix) with ESMTP id D5F0160BE5; Wed, 20 Mar 2019 16:57:56 +0000 (UTC) To: Zhichao Gao , edk2-devel@lists.01.org Cc: Jordan Justen , Michael Turner , Bret Barkelew , Liming Gao References: <20190319152549.16104-1-zhichao.gao@intel.com> <20190319152549.16104-10-zhichao.gao@intel.com> From: Laszlo Ersek Message-ID: <9b493f8e-0712-9e2f-c548-016bf3135959@redhat.com> Date: Wed, 20 Mar 2019 17:57:55 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20190319152549.16104-10-zhichao.gao@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 20 Mar 2019 16:57:58 +0000 (UTC) Subject: Re: [PATCH V3 09/17] OvmfPkg/PlatformDebugLibIoPort: 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: Wed, 20 Mar 2019 16:57:59 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 03/19/19 16:25, Zhichao Gao wrote: > 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 > Cc: Jordan Justen > Cc: Laszlo Ersek > Cc: Ard Biesheuvel > Cc: Liming Gao > Cc: Sean Brogan > Cc: Michael Turner > Cc: Bret Barkelew > --- > OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c | 106 +++++++++++++++++++++- > 1 file changed, 101 insertions(+), 5 deletions(-) > > diff --git a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c > index 36cde54976..cda35faf66 100644 > --- a/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c > +++ b/OvmfPkg/Library/PlatformDebugLibIoPort/DebugLib.c > @@ -2,7 +2,7 @@ > Base Debug library instance for QEMU debug port. > It uses PrintLib to send debug messages to a fixed I/O port. > > - Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
> + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> Copyright (c) 2012, Red Hat, Inc.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > @@ -29,6 +29,12 @@ > // > #define MAX_DEBUG_MESSAGE_LENGTH 0x100 > > +// > +// 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. > > @@ -51,9 +57,41 @@ 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 > + ) > { > CHAR8 Buffer[MAX_DEBUG_MESSAGE_LENGTH]; > - VA_LIST Marker; > UINTN Length; > > // > @@ -72,9 +110,11 @@ DebugPrint ( > // > // Convert the DEBUG() message to an ASCII String > // > - VA_START (Marker, Format); > - Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, Marker); > - VA_END (Marker); > + if (BaseListMarker == NULL) { > + Length = AsciiVSPrint (Buffer, sizeof (Buffer), Format, VaListMarker); > + } else { > + Length = AsciiBSPrint (Buffer, sizeof (Buffer), Format, BaseListMarker); > + } > > // > // Send the print string to the debug I/O port > @@ -83,6 +123,62 @@ 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. > I couldn't find the will to review the comments, but the code looks sane to me. Acked-by: Laszlo Ersek In the future, for any given person CC'd on at least one patch in the series, please make sure the person is also CC'd on the cover letter. Version 3 of this patch is very different from version 2, but I had to construct the reason myself retro-actively (look up the v3 blurb in my list folder, read the summary, go back to v2, find Andrew's comments). This could have been helped at least a little if I had been CC'd on the v3 blurb. Thanks Laszlo