Re: [edk2-devel] [Patch v8 2/2] MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK unit test

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Sean,

I agree that tis unit test can be reorganized a bit to support
unit tests for all APIs in BaseLib.

Can you enter a BZ for this and we can work on cleaning this
up after the stable tag?

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Sean
> Sent: Wednesday, May 20, 2020 12:05 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Andrew Fish <afish@apple.com>; Ard Biesheuvel
> <ard.biesheuvel@linaro.org>; Bret Barkelew
> <bret.barkelew@microsoft.com>; Brian J . Johnson
> <brian.johnson@hpe.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Justen, Jordan L
> <jordan.l.justen@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Leif Lindholm <leif@nuviainc.com>;
> Gao, Liming <liming.gao@intel.com>; Marvin H?user
> <mhaeuser@outlook.de>; Zimmer, Vincent
> <vincent.zimmer@intel.com>; Gao, Zhichao
> <zhichao.gao@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Vitaly Cheptsov
> <vit9696@protonmail.com>
> Subject: Re: [edk2-devel] [Patch v8 2/2]
> MdePkg/Test/BaseLib: Add SAFE_STRING_CONSTRAINT_CHECK
> unit test
> 
> Mike,
> 
> I would have thought the SafeString tests would have
> gone in a different
> c file.  Base64UnitTest.c seems by its title to be
> targeted at the
> base64 encode/decode test.
> 
> Looking at this i do see that would require some
> restructuring as there
> is no BaseLibUnitTest.c file for the common test part.
> As the author of
> this test originally, I can see that i didn't set it up
> to scale to the
> entire baselib very well.  Sorry.
> 
> 
> Thanks
> Sean
> 
> 
> 
> 
> On 5/19/2020 8:01 PM, Michael D Kinney wrote:
> > Use the safe string function StrCpyS() in BaseLib to
> test the
> > SAFE_STRING_CONSTRAINT_CHECK() macro.
> >
> > Cc: Andrew Fish <afish@apple.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Cc: Bret Barkelew <bret.barkelew@microsoft.com>
> > Cc: Brian J. Johnson <brian.johnson@hpe.com>
> > Cc: Chasel Chiu <chasel.chiu@intel.com>
> > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Leif Lindholm <leif@nuviainc.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Marvin H?user <mhaeuser@outlook.de>
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Vincent Zimmer <vincent.zimmer@intel.com>
> > Cc: Zhichao Gao <zhichao.gao@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Vitaly Cheptsov <vit9696@protonmail.com>
> > Signed-off-by: Michael D Kinney
> <michael.d.kinney@intel.com>
> > ---
> >   .../UnitTest/Library/BaseLib/Base64UnitTest.c | 85
> +++++++++++++++++++
> >   1 file changed, 85 insertions(+)
> >
> > diff --git
> a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> > index 8952f9da6c..5aced69e0d 100644
> > ---
> a/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> > +++
> b/MdePkg/Test/UnitTest/Library/BaseLib/Base64UnitTest.c
> > @@ -290,6 +290,77 @@ RfcDecodeTest(
> >     return UNIT_TEST_PASSED;
> >   }
> >
> > +#define SOURCE_STRING  L"Hello"
> > +
> > +STATIC
> > +UNIT_TEST_STATUS
> > +EFIAPI
> > +SafeStringContraintCheckTest (
> > +  IN UNIT_TEST_CONTEXT  Context
> > +  )
> > +{
> > +  RETURN_STATUS  Status;
> > +  CHAR16         Destination[20];
> > +
> > +  //
> > +  // Positive test case copy source unicode string
> to destination
> > +  //
> > +  Status = StrCpyS (Destination, sizeof
> (Destination) / sizeof (CHAR16), SOURCE_STRING);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING,
> sizeof (SOURCE_STRING));
> > +
> > +  //
> > +  // Positive test case with DestMax the same as
> Source size
> > +  //
> > +  Status = StrCpyS (Destination, sizeof
> (SOURCE_STRING) / sizeof (CHAR16), SOURCE_STRING);
> > +  UT_ASSERT_NOT_EFI_ERROR (Status);
> > +  UT_ASSERT_MEM_EQUAL (Destination, SOURCE_STRING,
> sizeof (SOURCE_STRING));
> > +
> > +  //
> > +  // Negative test case with Destination NULL
> > +  //
> > +  Status = StrCpyS (NULL, sizeof (Destination) /
> sizeof (CHAR16), SOURCE_STRING);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_INVALID_PARAMETER);
> > +
> > +  //
> > +  // Negative test case with Source NULL
> > +  //
> > +  Status = StrCpyS (Destination, sizeof
> (Destination) / sizeof (CHAR16), NULL);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_INVALID_PARAMETER);
> > +
> > +  //
> > +  // Negative test case with DestMax too big
> > +  //
> > +  Status = StrCpyS (Destination, MAX_UINTN,
> SOURCE_STRING);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_INVALID_PARAMETER);
> > +
> > +  //
> > +  // Negative test case with DestMax 0
> > +  //
> > +  Status = StrCpyS (Destination, 0, SOURCE_STRING);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_INVALID_PARAMETER);
> > +
> > +  //
> > +  // Negative test case with DestMax smaller than
> Source size
> > +  //
> > +  Status = StrCpyS (Destination, 1, SOURCE_STRING);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_BUFFER_TOO_SMALL);
> > +
> > +  //
> > +  // Negative test case with DestMax smaller than
> Source size by one character
> > +  //
> > +  Status = StrCpyS (Destination, sizeof
> (SOURCE_STRING) / sizeof (CHAR16) - 1, SOURCE_STRING);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_BUFFER_TOO_SMALL);
> > +
> > +  //
> > +  // Negative test case with DestMax smaller than
> Source size
> > +  //
> > +  Status = StrCpyS (Destination, sizeof
> (Destination) / sizeof (CHAR16), Destination);
> > +  UT_ASSERT_STATUS_EQUAL (Status,
> RETURN_ACCESS_DENIED);
> > +
> > +  return UNIT_TEST_PASSED;
> > +}
> > +
> >   /**
> >     Initialze the unit test framework, suite, and
> unit tests for the
> >     Base64 conversion APIs of BaseLib and run the
> unit tests.
> > @@ -309,6 +380,7 @@ UnitTestingEntry (
> >     UNIT_TEST_FRAMEWORK_HANDLE  Fw;
> >     UNIT_TEST_SUITE_HANDLE      b64EncodeTests;
> >     UNIT_TEST_SUITE_HANDLE      b64DecodeTests;
> > +  UNIT_TEST_SUITE_HANDLE      SafeStringTests;
> >
> >     Fw = NULL;
> >
> > @@ -367,6 +439,19 @@ UnitTestingEntry (
> >     AddTestCase (b64DecodeTests, "Incorrectly placed
> padding character", "Error4", RfcDecodeTest, NULL,
> CleanUpB64TestContext, &mBasicDecodeError4);
> >     AddTestCase (b64DecodeTests, "Too small of output
> buffer", "Error5", RfcDecodeTest, NULL,
> CleanUpB64TestContext, &mBasicDecodeError5);
> >
> > +  //
> > +  // Populate the safe string Unit Test Suite.
> > +  //
> > +  Status = CreateUnitTestSuite (&SafeStringTests,
> Fw, "Safe String", "BaseLib.SafeString", NULL, NULL);
> > +  if (EFI_ERROR (Status)) {
> > +    DEBUG ((DEBUG_ERROR, "Failed in
> CreateUnitTestSuite for SafeStringTests\n"));
> > +    Status = EFI_OUT_OF_RESOURCES;
> > +    goto EXIT;
> > +  }
> > +
> > +  // --------------Suite-----------Description------
> --------Class Name----------Function--------Pre---Post-
> ------------------Context-----------
> > +  AddTestCase (SafeStringTests,
> "SAFE_STRING_CONSTRAINT_CHECK",
> "SafeStringContraintCheckTest",
> SafeStringContraintCheckTest, NULL, NULL, NULL);
> > +
> >     //
> >     // Execute the tests.
> >     //
> >
> 
>