From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4864:20::d43; helo=mail-io1-xd43.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-io1-xd43.google.com (mail-io1-xd43.google.com [IPv6:2607:f8b0:4864:20::d43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 29CD7211EABB9 for ; Tue, 26 Mar 2019 05:43:43 -0700 (PDT) Received: by mail-io1-xd43.google.com with SMTP id e13so10614692ioq.6 for ; Tue, 26 Mar 2019 05:43:43 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=h5DjZ8oUg59AYN1KfFqEWMfWmZ0caF/HfUUxRPl1wUA=; b=uDWTFipN+Rp29GqErDUgzjlhwZKvT0i0TARtEqycxtCGxQCeworsSzj29Gu4fdgpeT hErYqFg+BaghWpNGlLfA1P2UAkRJzSSe3zGNnIB0cCuwcjXFwNVDAME0BCJodAxm5clH 47inqnJJKcWp6XMltrEgJFctPRGuyyPCQMxz6ymx6cvLBprGhX37SDn2ZDYUXIzpDB3g +olh+FL0vccn4yYruottxUwjsVQI2HUVM1IcC0945pp/XnB4cII3eaV+Rxenzt1j0uzD BZ39PISS7sK9n9Cxk3xty+kbP2F2awG8Xj8ARZ79pGuq5DtE+9yZrjRGUq/slFe97CNO SVeA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=h5DjZ8oUg59AYN1KfFqEWMfWmZ0caF/HfUUxRPl1wUA=; b=Xpvu+iF1F2CjkE6lRgO15qfWIYCbiSB+P49u26qITFXsxUsDU/wygQdhg4dStgQd90 RKcYu7zIONGydFeQrqG7jVszWpqnGDR1glAlDEGtMHeQC1k7pSumNCd8lU+HpJifBImz Ozqahz2whVMgiQE7k0bMerIwGBwbNx3daePQkvKFtzLyj5HO6esOSOXHbg5kIC2qatDH xboc9LZR46vhCOQr28HCMycMkKLJrHVdOadPfmv2KYU9xsZ26X4oEDg3PDBL+MXgwRSr oDfe6QJjC7j7ZPROvsi0TZTY6W6NAXhJNAMrB5Gv1MmNnd2bgsnBBJnBYJRrdfdBiHpa CgXQ== X-Gm-Message-State: APjAAAX5TKECmuY5vLA7c/ezETLEyYqkOTa4lyt9ViWNHY2NQLQf/zky zN5xgoMIthHHhY2qMkzZMTSmEc9pjjvHWJ4ZNwxgCA== X-Google-Smtp-Source: APXvYqzgqNI0V7FgEQ2HFmUPzKnkF845EY2g0qM/cgkiIQ7FPLfVkLdpWMKJWzd4GcWL7A88D7JEYck+tkhHtIE+44g= X-Received: by 2002:a5e:9b17:: with SMTP id j23mr22082570iok.60.1553604221733; Tue, 26 Mar 2019 05:43:41 -0700 (PDT) MIME-Version: 1.0 References: <20190321140459.18304-1-zhichao.gao@intel.com> <20190321140459.18304-9-zhichao.gao@intel.com> <20190326121906.uke2slmodkqvwvgy@bivouac.eciton.net> In-Reply-To: <20190326121906.uke2slmodkqvwvgy@bivouac.eciton.net> From: Ard Biesheuvel Date: Tue, 26 Mar 2019 13:43:30 +0100 Message-ID: To: Leif Lindholm , "Cohen, Eugene" Cc: Zhichao Gao , "edk2-devel@lists.01.org" , Liming Gao , Sean Brogan , Michael Turner , Bret Barkelew Subject: Re: [PATCH V4 08/17] ArmPkg/SemiHostingDebugLib: 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: Tue, 26 Mar 2019 12:43:44 -0000 Content-Type: text/plain; charset="UTF-8" On Tue, 26 Mar 2019 at 13:19, Leif Lindholm wrote: > > Hi Zhichao, > > Apologies for delay in responding, due to holiday and stuff. > > On the whole this looks fine (one comment below), but I don't actually > have any platform on which to test this. > > Ard: maybe it's time to retire this component? > I don't have a problem with that. Adding Eugene, perhaps he has an opinion as well. > On Thu, Mar 21, 2019 at 10:04:50PM +0800, 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: Leif Lindholm > > Cc: Ard Biesheuvel > > Cc: Liming Gao > > Cc: Sean Brogan > > Cc: Michael Turner > > Cc: Bret Barkelew > > --- > > ArmPkg/Library/SemiHostingDebugLib/DebugLib.c | 106 ++++++++++++++++++++++++-- > > 1 file changed, 101 insertions(+), 5 deletions(-) > > > > diff --git a/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c b/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c > > index ec03edb774..a368dd43b8 100644 > > --- a/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c > > +++ b/ArmPkg/Library/SemiHostingDebugLib/DebugLib.c > > @@ -1,7 +1,7 @@ > > /** @file > > UEFI Debug Library that uses PrintLib to send messages to STDERR. > > > > - Copyright (c) 2006 - 2007, Intel Corporation. All rights reserved.
> > + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> > Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
> > This program and the accompanying materials > > are licensed and made available under the terms and conditions of the BSD License > > @@ -27,6 +27,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; > > I would prefer if this was marked STATIC. > > If you feel strongly otherwise, leave it as is. > Either way: > Reviewed-by: Leif Lindholm > > / > Leif > > > + > > /** > > > > Prints a debug message to the debug output device if the specified error level is enabled. > > @@ -48,9 +54,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 AsciiBuffer[MAX_DEBUG_MESSAGE_LENGTH]; > > - VA_LIST Marker; > > > > // > > // If Format is NULL, then ASSERT(). > > @@ -67,14 +105,72 @@ DebugPrint ( > > // > > // Convert the DEBUG() message to a Unicode String > > // > > - VA_START (Marker, Format); > > - AsciiVSPrint (AsciiBuffer, sizeof (AsciiBuffer), Format, Marker); > > - VA_END (Marker); > > + if (BaseListMarker == NULL) { > > + AsciiVSPrint (AsciiBuffer, sizeof (AsciiBuffer), Format, VaListMarker); > > + } else { > > + AsciiBSPrint (AsciiBuffer, sizeof (AsciiBuffer), Format, BaseListMarker); > > + } > > > > SemihostWriteString (AsciiBuffer); > > } > > > > > > +/** > > + 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. > > -- > > 2.16.2.windows.1 > >