From: "Heinrich Schuchardt" <xypron.glpk@gmx.de>
To: xianhui liu <xianhuix.liu@intel.com>, devel@edk2.groups.io
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>,
Eric Jin <eric.jin@intel.com>
Subject: Re: [edk2-test][Patch] uefi-sct/SctPkg: Add MediaPresentSupported check
Date: Tue, 24 Sep 2019 21:05:56 +0200 [thread overview]
Message-ID: <1ec6d2d9-561b-c04e-327e-20b82001929f@gmx.de> (raw)
In-Reply-To: <20190924085052.8484-1-xianhuix.liu@intel.com>
On 9/24/19 10:50 AM, xianhui liu wrote:
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2061
> check MediaPresent while MediaPresentSupported is TRUE
> sync change from EFI to IHV SimpleNetworkBBTestFunction
Thanks for addressing this issue.
>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
%s/Cc:/Reported-by:/
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Eric Jin <eric.jin@intel.com>
>
> Signed-off-by: xianhui liu <xianhuix.liu@intel.com>
> ---
> .../BlackBoxTest/SimpleNetworkBBTestFunction.c | 64 +++++++++++-----------
> 1 file changed, 33 insertions(+), 31 deletions(-)
>
> diff --git a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestFunction.c b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestFunction.c
> index b4c7b5ee..8559e894 100644
> --- a/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestFunction.c
> +++ b/uefi-sct/SctPkg/TestCase/UEFI/IHV/Protocol/SimpleNetwork/BlackBoxTest/SimpleNetworkBBTestFunction.c
> @@ -1888,38 +1888,40 @@ BBTestGetStatusFunctionTest (
> Status = SnpInterface->GetStatus (SnpInterface, &InterruptStatus, &TxBuf);
> Status1 = SnpInterface->GetStatus (SnpInterface, &InterruptStatus, &TxBuf);
>
> - if (SnpInterface->Mode->MediaPresent == FALSE) {
> - if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS) && (InterruptStatus == 0)) {
> - AssertionType = EFI_TEST_ASSERTION_PASSED;
> - } else {
> - AssertionType = EFI_TEST_ASSERTION_FAILED;
> - }
> - } else {
> - if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS)) {
> - AssertionType = EFI_TEST_ASSERTION_PASSED;
> - if (InterruptStatus &
> - ~( EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT |
> - EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT |
> - EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT |
> - EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT)) {
> - AssertionType = EFI_TEST_ASSERTION_FAILED;
> - }
> - } else {
> - AssertionType = EFI_TEST_ASSERTION_FAILED;
> - }
> + if (SnpInterface->Mode-> MediaPresentSupported == TRUE) {
> + if (SnpInterface->Mode->MediaPresent == FALSE) {
If MediaPresentSupported == FALSE shouldn't we assume that Media is present.
So isn't a single 'if' enough:
if (SnpInterface->Mode-> MediaPresentSupported == TRUE &&
SnpInterface->Mode->MediaPresent == FALSE) {
Best regards
Heinrich Schuchardt
> + if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS) && (InterruptStatus == 0)) {
> + AssertionType = EFI_TEST_ASSERTION_PASSED;
> + } else {
> + AssertionType = EFI_TEST_ASSERTION_FAILED;
> + }
> + } else {
> + if ((Status1 == EFI_SUCCESS) && (Status == EFI_SUCCESS)) {
> + AssertionType = EFI_TEST_ASSERTION_PASSED;
> + if (InterruptStatus &
> + ~( EFI_SIMPLE_NETWORK_RECEIVE_INTERRUPT |
> + EFI_SIMPLE_NETWORK_TRANSMIT_INTERRUPT |
> + EFI_SIMPLE_NETWORK_COMMAND_INTERRUPT |
> + EFI_SIMPLE_NETWORK_SOFTWARE_INTERRUPT)) {
> + AssertionType = EFI_TEST_ASSERTION_FAILED;
> + }
> + } else {
> + AssertionType = EFI_TEST_ASSERTION_FAILED;
> + }
> + }
> + StandardLib->RecordAssertion (
> + StandardLib,
> + AssertionType,
> + gSimpleNetworkBBTestFunctionAssertionGuid022,
> + L"EFI_SIMPLE_NETWORK_PROTOCOL.GetStatus - Invoke GetStatus() and verify interface correctness within test case",
> + L"%a:%d:Status - %r, Status1 - %r, InterruptStatus - %d",
> + __FILE__,
> + (UINTN)__LINE__,
> + Status,
> + Status1,
> + InterruptStatus
> + );
> }
> - StandardLib->RecordAssertion (
> - StandardLib,
> - AssertionType,
> - gSimpleNetworkBBTestFunctionAssertionGuid022,
> - L"EFI_SIMPLE_NETWORK_PROTOCOL.GetStatus - Invoke GetStatus() and verify interface correctness within test case",
> - L"%a:%d:Status - %r, Status1 - %r, InterruptStatus - %d",
> - __FILE__,
> - (UINTN)__LINE__,
> - Status,
> - Status1,
> - InterruptStatus
> - );
>
> //
> // Restore SNP State
>
next prev parent reply other threads:[~2019-09-24 19:06 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-24 8:50 [edk2-test][Patch] uefi-sct/SctPkg: Add MediaPresentSupported check xianhui liu
2019-09-24 19:05 ` Heinrich Schuchardt [this message]
2019-09-25 2:18 ` [edk2-devel] " xianhui liu
[not found] ` <15C78CE54FBDEE07.4626@groups.io>
2019-09-27 2:01 ` xianhui liu
2019-09-27 6:00 ` Heinrich Schuchardt
2019-09-27 6:36 ` Eric Jin
[not found] ` <15C83824ED461154.3061@groups.io>
2019-09-29 3:37 ` Eric Jin
-- strict thread matches above, loose matches on Subject: below --
2019-09-23 1:42 xianhuix.liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1ec6d2d9-561b-c04e-327e-20b82001929f@gmx.de \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox