From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 1F5801A1E3A for ; Mon, 24 Oct 2016 13:59:38 -0700 (PDT) Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 78A9580086; Mon, 24 Oct 2016 20:59:36 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-107.phx2.redhat.com [10.3.116.107]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u9OKxZLH021767; Mon, 24 Oct 2016 16:59:35 -0400 References: <20161021212737.15974-1-lersek@redhat.com> <20161021212737.15974-2-lersek@redhat.com> Cc: edk2-devel-01 To: Michael D Kinney , Liming Gao From: Laszlo Ersek Message-ID: <567fc440-ce97-548d-e645-6ae84dfbf488@redhat.com> Date: Mon, 24 Oct 2016 22:59:34 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161021212737.15974-2-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.24 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Mon, 24 Oct 2016 20:59:36 +0000 (UTC) Subject: Re: [PATCH 01/19] MdePkg/DebugLib.h: add ASSERT_RETURN_ERROR() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 24 Oct 2016 20:59:40 -0000 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Mike, Liming, On 10/21/16 23:27, Laszlo Ersek wrote: > ASSERT_EFI_ERROR() cannot be used in BASE type modules because > - the replacement text calls EFI_ERROR(), > - EFI_ERROR() is defined in "MdePkg/Include/Uefi/UefiBaseType.h", > - the inclusion of "UefiBaseType.h" is not required for BASE type modules. > > While > > ASSERT (!RETURN_ERROR (StatusParameter)) > > would be a functional statement in BASE type modules, it would be less > convenient and less informative: ASSERT_EFI_ERROR() prints the actual > StatusParameter. > > Hence add ASSERT_RETURN_ERROR(), paralleling ASSERT_EFI_ERROR(). Copy the > original macro definition and update it as follows: > - replace EFI with RETURN, > - wrap overlong lines in the comment block and in the code, > - EFI_D_ERROR is deprecated, so employ DEBUG_ERROR instead. > > Cc: Liming Gao > Cc: Michael D Kinney > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=166 > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek > --- > > Notes: > OvmfPkg/SmbiosVersionLib, modified in one of the upcoming patches, is > one such BASE module. > > MdePkg/Include/Library/DebugLib.h | 27 ++++++++++++++++++++ > 1 file changed, 27 insertions(+) > > diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h > index 81904325703f..3a910e6a208b 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -348,6 +348,33 @@ DebugPrintLevelEnabled ( > #define ASSERT_EFI_ERROR(StatusParameter) > #endif > > +/** > + Macro that calls DebugAssert() if a RETURN_STATUS evaluates to an error code. > + > + If MDEPKG_NDEBUG is not defined and the DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED > + bit of PcdDebugProperyMask is set, then this macro evaluates the > + RETURN_STATUS value specified by StatusParameter. If StatusParameter is an > + error code, then DebugAssert() is called passing in the source filename, > + source line number, and StatusParameter. > + > + @param StatusParameter RETURN_STATUS value to evaluate. > + > +**/ > +#if !defined(MDEPKG_NDEBUG) > + #define ASSERT_RETURN_ERROR(StatusParameter) \ > + do { \ > + if (DebugAssertEnabled ()) { \ > + if (RETURN_ERROR (StatusParameter)) { \ > + DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \ > + StatusParameter)); \ > + _ASSERT (!RETURN_ERROR (StatusParameter)); \ > + } \ > + } \ > + } while (FALSE) > +#else > + #define ASSERT_RETURN_ERROR(StatusParameter) > +#endif > + > /** > Macro that calls DebugAssert() if a protocol is already installed in the > handle database. > can I please get a maintainer review for this patch? The rest of the series is ready to go, but it depends on this patch. Thanks! Laszlo