From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2607:f8b0:4001:c0b::243; helo=mail-it0-x243.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-it0-x243.google.com (mail-it0-x243.google.com [IPv6:2607:f8b0:4001:c0b::243]) (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 C77A1221EA0A5 for ; Thu, 7 Dec 2017 07:21:51 -0800 (PST) Received: by mail-it0-x243.google.com with SMTP id f190so15128721ita.5 for ; Thu, 07 Dec 2017 07:26:24 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=KaD+ubbE1UvkKklMFmCKgnbw/p/XyJfjfOTruQ7pW0w=; b=I+7uJ6WLfmjD1XFPmo63+bSX2l1ejfN0mgscijUR44hFesTCcxB/FIDsF5QsWJrCVb DyD+xEfcWivgzjQgLMInVvmXmB7c3Dc4DeyVIysuWJOd3Ef9LEitApCcwsMm7f9ee2eo agbHC5ZefE73tomuho6T1k7+9NXhfX8tnjQuA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=KaD+ubbE1UvkKklMFmCKgnbw/p/XyJfjfOTruQ7pW0w=; b=ou4f3wEV3JHfBPC0JvA4MacPJ/Qh/OpRfuA4FYN7TF7lTjBni+LeKLihS24OOyOQku 4IanG6XZc+o5nCtfPhuirqpY7sUlHM1vFvaUkahcxap/G1hLGvf/Qfgu0bcM3j1JoMLb Gif725WZjYxoddgIMBw34IHirEVjzbsVf5sAmpLiGud1btOZ9wJChCmw2CdXFI2kilCz gvjsDBPmVwclwkPh36rG0EWmGtVKR10bl7tb5bsk91iuw10RjoPsPAHztEfeorONuEyD h9hryquM0VnzdTxEOnGSv4zQb7iQo4k/beViFIsqvnUEFElMn6UHETol7O7+HmHhRWP6 UMig== X-Gm-Message-State: AJaThX4Ff3cNYSeVoAtj0T45P9f/HhxbygH54ZwpLkwOGqMCoDBtSt7j IUx2YPYzcRzkV2Nn0ecgwLlyEKXFNwwkOA3dLgTr1Py2 X-Google-Smtp-Source: AGs4zMaJVgRYRN237A6xdqTyE2r4EXF8tMZFCoYo3m64gx6Tak73SaBndbpmNfwwC0RkAIFtkLZhHwQerT2Q5qnRIK8= X-Received: by 10.107.59.85 with SMTP id i82mr37210408ioa.253.1512660384127; Thu, 07 Dec 2017 07:26:24 -0800 (PST) MIME-Version: 1.0 Received: by 10.107.104.16 with HTTP; Thu, 7 Dec 2017 07:26:23 -0800 (PST) In-Reply-To: <20171207151208.25648-1-ard.biesheuvel@linaro.org> References: <20171207151208.25648-1-ard.biesheuvel@linaro.org> From: Ard Biesheuvel Date: Thu, 7 Dec 2017 15:26:23 +0000 Message-ID: To: "edk2-devel@lists.01.org" Cc: Leif Lindholm , "Gao, Liming" , Evan Lloyd , "Kinney, Michael D" , Alexei Fedorov , Ard Biesheuvel Subject: Re: [PATCH] MdePkg/DebugLib; swap if conditions in ASSERT_[EFI|RETURN]_ERROR X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 07 Dec 2017 15:21:52 -0000 Content-Type: text/plain; charset="UTF-8" On 7 December 2017 at 15:12, Ard Biesheuvel wrote: > ASSERT_EFI_ERROR () is currently defined as > > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) \ > do { \ > if (DebugAssertEnabled ()) { \ > if (EFI_ERROR (StatusParameter)) { \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); \ > } \ > } \ > } while (FALSE) > #else > #define ASSERT_EFI_ERROR(StatusParameter) > #endif > > This is suboptimal, given that the DebugAssertEnabled () call in the > outer if must be executed unconditionally, since the compiler does not > know that it does not have any side effects. Instead, let's swap the > two ifs, and only call DebugAssertEnabled () if StatusParameter contains > an error value to begin with. Do the same for ASSERT_RETURN_ERROR > as well. > I just noticed we could do the same for ASSERT () as well. > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Ard Biesheuvel > Reported-by: Alexei Fedorov > --- > MdePkg/Include/Library/DebugLib.h | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h > index 3a910e6a208b..8369c378e79c 100644 > --- a/MdePkg/Include/Library/DebugLib.h > +++ b/MdePkg/Include/Library/DebugLib.h > @@ -337,8 +337,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_EFI_ERROR(StatusParameter) \ > do { \ > - if (DebugAssertEnabled ()) { \ > - if (EFI_ERROR (StatusParameter)) { \ > + if (EFI_ERROR (StatusParameter)) { \ > + if (DebugAssertEnabled ()) { \ > DEBUG ((EFI_D_ERROR, "\nASSERT_EFI_ERROR (Status = %r)\n", StatusParameter)); \ > _ASSERT (!EFI_ERROR (StatusParameter)); \ > } \ > @@ -363,8 +363,8 @@ DebugPrintLevelEnabled ( > #if !defined(MDEPKG_NDEBUG) > #define ASSERT_RETURN_ERROR(StatusParameter) \ > do { \ > - if (DebugAssertEnabled ()) { \ > - if (RETURN_ERROR (StatusParameter)) { \ > + if (RETURN_ERROR (StatusParameter)) { \ > + if (DebugAssertEnabled ()) { \ > DEBUG ((DEBUG_ERROR, "\nASSERT_RETURN_ERROR (Status = %r)\n", \ > StatusParameter)); \ > _ASSERT (!RETURN_ERROR (StatusParameter)); \ > -- > 2.11.0 >