From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=2a00:1450:400c:c0c::241; helo=mail-wr0-x241.google.com; envelope-from=ard.biesheuvel@linaro.org; receiver=edk2-devel@lists.01.org Received: from mail-wr0-x241.google.com (mail-wr0-x241.google.com [IPv6:2a00:1450:400c:c0c::241]) (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 84139221EA0A1 for ; Thu, 7 Dec 2017 07:07:44 -0800 (PST) Received: by mail-wr0-x241.google.com with SMTP id v105so7852108wrc.3 for ; Thu, 07 Dec 2017 07:12:17 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=trUUvODB7anFtHhbcuJbSPibUAfEVilbU022okPJnec=; b=ZDB+NQE/qQ36dud/lRnhlli6gj7Uv+VxZNR3jaLyXwyBFR+sgBirLfa+O938iAjy6S Hn5H8II7E+fh8THBbk+LFxRhWVkZehO0wZT/rYOii2VJyo8ghx4xq9b4Z5AD756lXeAH OBhiSKXmgW1tEBfzbJ5l1bdLF6hoRzFDJRd3c= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=trUUvODB7anFtHhbcuJbSPibUAfEVilbU022okPJnec=; b=lpVQCXQandOGKLDbBbELL62JswAVweWl83qZQwM0ObOVqa6o6IgHm/UdyaWFjfYOn1 lmdzkdUKWSw7ykxQWQR0hFeoeztW4Y+LvJvEaz/pi0e6GXrfCS2+tX73Qbr9LZZ9g3Gw /NhjYSm4RdpJZKbbXIPrIea8SO6dkxIjVjPAeogfqv8lPAOOhFfzJMQH9qewSWDCOZeR afNw0Qsp+8PCtqVSrxE90crgkOm/2vvY4dvsAe1EJ1L22Itp5/pqq6BImWDs6GHG8Idz 7mfPEoWeABUvxLhD08iUIjiNdEFHPaYxij9lFqWgku6Wt4Dxzix8qTaJdz5SH7lWGsuS OSpw== X-Gm-Message-State: AJaThX559NAVfp5K54R+Clti35ZQUQy2pg1bqsBjy90dBOim97uyQW36 F//VTYjs4XdQIemEgw1lfZID9ZzKrbmaNw== X-Google-Smtp-Source: AGs4zMY1CMuuCIXYtA03TwA+yO46cQ5BrcSgV5Vq5VhvXObxF41v3dquM41DncXvB9xIRgJ9aw2FTQ== X-Received: by 10.223.136.162 with SMTP id f31mr24081219wrf.130.1512659536155; Thu, 07 Dec 2017 07:12:16 -0800 (PST) Received: from localhost.localdomain ([160.171.158.223]) by smtp.gmail.com with ESMTPSA id b45sm5894826wrb.1.2017.12.07.07.12.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 07 Dec 2017 07:12:15 -0800 (PST) From: Ard Biesheuvel To: edk2-devel@lists.01.org Cc: leif.lindholm@linaro.org, liming.gao@intel.com, evan.lloyd@arm.com, michael.d.kinney@intel.com, Alexei.Fedorov@arm.com, Ard Biesheuvel Date: Thu, 7 Dec 2017 15:12:08 +0000 Message-Id: <20171207151208.25648-1-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.11.0 Subject: [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:07:44 -0000 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. 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