From: "Eric Jin" <eric.jin@intel.com>
To: Heinrich Schuchardt <xypron.glpk@gmx.de>,
EDK II Development <devel@edk2.groups.io>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>,
Stephano Cetola <stephano.cetola@linux.intel.com>
Subject: Re: [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: SNP Statistics may be unsupported
Date: Fri, 6 Sep 2019 07:06:08 +0000 [thread overview]
Message-ID: <DA72DC7456565B47808A57108259571F6380FEFA@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20190904092554.10090-1-xypron.glpk@gmx.de>
Hi Heinrich,
2 comments embedded below. Other is ok to me.
Open a new ticket in Bugzilla is better.
Best Regards
Eric
> -----Original Message-----
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Sent: Wednesday, September 4, 2019 5:26 PM
> To: EDK II Development <devel@edk2.groups.io>
> Cc: Jin, Eric <eric.jin@intel.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Stephano Cetola
> <stephano.cetola@linux.intel.com>; Heinrich Schuchardt
> <xypron.glpk@gmx.de>
> Subject: [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: SNP Statistics may be
> unsupported
>
> If EFI_SIMPLE_NETWORK.Statistics() is not supported, it will return
> EFI_UNSUPPORTED. In this case do not expect input parameters to be
> checked for conformance.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> .../SimpleNetworkBBTestConformance.c | 112 +++++++++++-------
> 1 file changed, 72 insertions(+), 40 deletions(-)
>
> diff --git a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simpl
> eNetworkBBTestConformance.c b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simpl
> eNetworkBBTestConformance.c
> index 9d5bec18..ea4a806b 100644
> --- a/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxTest/Simpl
> eNetworkBBTestConformance.c
> +++ b/uefi-
> sct/SctPkg/TestCase/UEFI/EFI/Protocol/SimpleNetwork/BlackBoxT
> +++ est/SimpleNetworkBBTestConformance.c
> @@ -1015,10 +1015,19 @@ BBTestStatisticsConformanceTest (
> // Call Statistics() function if network interface not start. // StatusBuf[0] =
> SnpInterface->Statistics (SnpInterface, FALSE, &StatisticsSize,
> &StatisticsTable);- if ((StatusBuf[0] == EFI_NOT_STARTED) && (SnpInterface-
> >Mode->State == EfiSimpleNetworkStopped)) {- AssertionType[0] =
> EFI_TEST_ASSERTION_PASSED;+ if (StatusBuf[0] == EFI_UNSUPPORTED) {+
> StandardLib->RecordMessage(+ StandardLib,+
> EFI_VERBOSE_LEVEL_QUIET,+ L"Statistics isn't supported, Status -
> %r\n",+ StatusBuf[0]+ ); } else {- AssertionType[0] =
> EFI_TEST_ASSERTION_FAILED;+ if ((StatusBuf[0] == EFI_NOT_STARTED) &&
> (SnpInterface->Mode->State == EfiSimpleNetworkStopped)) {+
> AssertionType[0] = EFI_TEST_ASSERTION_PASSED;+ } else {+
> AssertionType[0] = EFI_TEST_ASSERTION_FAILED;+ } } //@@ -1031,10
> +1040,19 @@ BBTestStatisticsConformanceTest (
> } StatusBuf[1] = SnpInterface->Statistics (SnpInterface, FALSE,
> &StatisticsSize, &StatisticsTable);- if (StatusBuf[1] == EFI_DEVICE_ERROR) {-
> AssertionType[1] = EFI_TEST_ASSERTION_PASSED;+ if (StatusBuf[1] ==
> EFI_UNSUPPORTED) {+ StandardLib->RecordMessage(+
> StandardLib,+ EFI_VERBOSE_LEVEL_QUIET,+ L"Statistics
> isn't supported, Status - %r\n",+ StatusBuf[1]+ ); } else {-
> AssertionType[1] = EFI_TEST_ASSERTION_FAILED;+ if (StatusBuf[1] ==
> EFI_DEVICE_ERROR) {+ AssertionType[1] =
> EFI_TEST_ASSERTION_PASSED;+ } else {+ AssertionType[1] =
> EFI_TEST_ASSERTION_FAILED;+ } } @@ -1054,45 +1072,59 @@
> BBTestStatisticsConformanceTest (
> StatisticsSize = 0; StatusBuf[2] = SnpInterface->Statistics (SnpInterface,
> FALSE, &StatisticsSize, &StatisticsTable);- if (StatusBuf[2] ==
> EFI_BUFFER_TOO_SMALL || Status == EFI_UNSUPPORTED) {-
> AssertionType[2] = EFI_TEST_ASSERTION_PASSED;+ if (StatusBuf[2] ==
> EFI_UNSUPPORTED) {+ StandardLib->RecordMessage(+
> StandardLib,+ EFI_VERBOSE_LEVEL_QUIET,+ L"Statistics
> isn't supported, Status - %r\n",+ StatusBuf[2]+ ); } else {-
> AssertionType[2] = EFI_TEST_ASSERTION_FAILED;+ if (StatusBuf[2] ==
> EFI_BUFFER_TOO_SMALL || Status == EFI_UNSUPPORTED) {+
Status == EFI_UNSUPPORTED could be removed from here.
> AssertionType[2] = EFI_TEST_ASSERTION_PASSED;+ } else {+
> AssertionType[2] = EFI_TEST_ASSERTION_FAILED;+ } }
Had better move RecordAssertion as early as possible, because
SnpInterface->Initialize() may cause return early.
+ if (StatusBuf[0] !=
> EFI_UNSUPPORTED) {+ StandardLib->RecordAssertion (+
> StandardLib,+ AssertionType[0],+
> gSimpleNetworkBBTestConformanceAssertionGuid014,+
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() when
> network interface not start.",+ L"%a:%d:Status - %r",+
> __FILE__,+ (UINTN)__LINE__,+ StatusBuf[0]+ );+ }
> - StandardLib->RecordAssertion (- StandardLib,-
> AssertionType[0],-
> gSimpleNetworkBBTestConformanceAssertionGuid014,-
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() when
> network interface not start.",- L"%a:%d:Status - %r",-
> __FILE__,- (UINTN)__LINE__,- StatusBuf[0]- );- -
> StandardLib->RecordAssertion (- StandardLib,-
> AssertionType[1],-
> gSimpleNetworkBBTestConformanceAssertionGuid015,-
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() when
> network interface not initialized.",- L"%a:%d:Status - %r",-
> __FILE__,- (UINTN)__LINE__,- StatusBuf[1]- );+ if
> (StatusBuf[1] != EFI_UNSUPPORTED) {+ StandardLib->RecordAssertion (+
> StandardLib,+ AssertionType[1],+
> gSimpleNetworkBBTestConformanceAssertionGuid015,+
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() when
> network interface not initialized.",+ L"%a:%d:Status - %r",+
> __FILE__,+ (UINTN)__LINE__,+ StatusBuf[1]+ );+ }
> - StandardLib->RecordAssertion (- StandardLib,-
> AssertionType[2],-
> gSimpleNetworkBBTestConformanceAssertionGuid017,-
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() with small
> buffer.",- L"%a:%d:Status - %r",- __FILE__,-
> (UINTN)__LINE__,- StatusBuf[2]- );+ if (StatusBuf[2] !=
> EFI_UNSUPPORTED) {+ StandardLib->RecordAssertion (+
> StandardLib,+ AssertionType[2],+
> gSimpleNetworkBBTestConformanceAssertionGuid017,+
> L"EFI_SIMPLE_NETWORK_PROTOCOL.Statistics - Invoke Statistics() with small
> buffer.",+ L"%a:%d:Status - %r",+ __FILE__,+
> (UINTN)__LINE__,+ StatusBuf[2]+ );+ } // // Restore
> SNP State--
> 2.23.0.rc1
prev parent reply other threads:[~2019-09-06 7:26 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-04 9:25 [edk2-test] [PATCH 1/1] uefi-sct/SctPkg: SNP Statistics may be unsupported Heinrich Schuchardt
2019-09-06 7:06 ` Eric Jin [this message]
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=DA72DC7456565B47808A57108259571F6380FEFA@SHSMSX103.ccr.corp.intel.com \
--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