subject:"\[edk2\-devel\] \[Patch v8 2\/2\] MdePkg\/Test\/BaseLib\: Add SAFE_STRING_CONSTRAINT

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

2020-05-20 Thread Sean


Bug created.
https://bugzilla.tianocore.org/show_bug.cgi?id=2729


thanks
Sean



On 5/20/2020 1:11 PM, Kinney, Michael D wrote:

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  On
Behalf Of Sean
Sent: Wednesday, May 20, 2020 12:05 PM
To: devel@edk2.groups.io; Kinney, Michael D

Cc: Andrew Fish ; Ard Biesheuvel
; Bret Barkelew
; Brian J . Johnson
; Chiu, Chasel
; Justen, Jordan L
; Laszlo Ersek
; Leif Lindholm ;
Gao, Liming ; Marvin H?user
; Zimmer, Vincent
; Gao, Zhichao
; Yao, Jiewen
; Vitaly Cheptsov

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 
Cc: Ard Biesheuvel 
Cc: Bret Barkelew 
Cc: Brian J. Johnson 
Cc: Chasel Chiu 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Marvin H?user 
Cc: Michael D Kinney 
Cc: Vincent Zimmer 
Cc: Zhichao Gao 
Cc: Jiewen Yao 
Cc: Vitaly Cheptsov 
Signed-off-by: Michael D Kinney



---
   .../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 char

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

2020-05-20 Thread Michael D Kinney

Hi Vitaly,

I agree with all the comments here.  I have made these updates in V9.

Mike

From: vit9696 
Sent: Tuesday, May 19, 2020 11:08 PM
To: Kinney, Michael D 
Cc: devel@edk2.groups.io; Andrew Fish ; Ard Biesheuvel 
; Bret Barkelew ; Brian 
J . Johnson ; Chiu, Chasel ; 
Justen, Jordan L ; Laszlo Ersek ; 
Leif Lindholm ; Gao, Liming ; Marvin 
H?user ; Zimmer, Vincent ; Gao, 
Zhichao ; Yao, Jiewen 
Subject: Re: [Patch v8 2/2] MdePkg/Test/BaseLib: Add 
SAFE_STRING_CONSTRAINT_CHECK unit test

Mike,

This looks generally good to me, but there are few issues.

1. There is a mistake in the description here:

+  // Negative test case with DestMax smaller than Source size
+  //
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), 
Destination);

This should read «Negative test case with Destination and Source matching» 
probably or something alike.

2. For completeness I would also add zeroing before running every StrCpyS test, 
like this:

+  ZeroMem (Destination, sizeof (Destination));
+  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));

Otherwise UT_ASSERT_MEM_EQUAL will miss the check of StrCpyS doing anything — 
Destination could remain unchanged since case 1.


3. I will also make sense to check the Destination string remains unchanged on 
failure for all the other cases, like this:


+  CHAR16 Destination[20];
+  CHAR16 AllZero[20];
+
+  ZeroMem (AllZero, sizeof (AllZero));

+  //
+  // Negative test case with DestMax too big
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, MAX_UINTN, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));

This behaviour was very unintuitive to me when I met with EDK II BaseLib, 
because I expected something like strlcpy behaviour, where the string is 
truncated to fit, so it is definitely worth testing to ensure we do not break 
things.


Best wishes,
Vitaly


20 мая 2020 г., в 06:11, Michael D Kinney 
mailto:michael.d.kin...@intel.com>> написал(а):


Use the safe string function StrCpyS() in BaseLib to test the
SAFE_STRING_CONSTRAINT_CHECK() macro.

Cc: Andrew Fish mailto:af...@apple.com>>
Cc: Ard Biesheuvel mailto:ard.biesheu...@linaro.org>>
Cc: Bret Barkelew 
mailto:bret.barke...@microsoft.com>>
Cc: Brian J. Johnson mailto:brian.john...@hpe.com>>
Cc: Chasel Chiu mailto:chasel.c...@intel.com>>
Cc: Jordan Justen mailto:jordan.l.jus...@intel.com>>
Cc: Laszlo Ersek mailto:ler...@redhat.com>>
Cc: Leif Lindholm mailto:l...@nuviainc.com>>
Cc: Liming Gao mailto:liming@intel.com>>
Cc: Marvin H?user mailto:mhaeu...@outlook.de>>
Cc: Michael D Kinney 
mailto:michael.d.kin...@intel.com>>
Cc: Vincent Zimmer mailto:vincent.zim...@intel.com>>
Cc: Zhichao Gao mailto:zhichao@intel.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Vitaly Cheptsov mailto:vit9...@protonmail.com>>
Signed-off-by: Michael D Kinney 
mailto:michael.d.kin...@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

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

2020-05-20 Thread Michael D Kinney

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  On
> Behalf Of Sean
> Sent: Wednesday, May 20, 2020 12:05 PM
> To: devel@edk2.groups.io; Kinney, Michael D
> 
> Cc: Andrew Fish ; Ard Biesheuvel
> ; Bret Barkelew
> ; Brian J . Johnson
> ; Chiu, Chasel
> ; Justen, Jordan L
> ; Laszlo Ersek
> ; Leif Lindholm ;
> Gao, Liming ; Marvin H?user
> ; Zimmer, Vincent
> ; Gao, Zhichao
> ; Yao, Jiewen
> ; Vitaly Cheptsov
> 
> 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 
> > Cc: Ard Biesheuvel 
> > Cc: Bret Barkelew 
> > Cc: Brian J. Johnson 
> > Cc: Chasel Chiu 
> > Cc: Jordan Justen 
> > Cc: Laszlo Ersek 
> > Cc: Leif Lindholm 
> > Cc: Liming Gao 
> > Cc: Marvin H?user 
> > Cc: Michael D Kinney 
> > Cc: Vincent Zimmer 
> > Cc: Zhichao Gao 
> > Cc: Jiewen Yao 
> > Cc: Vitaly Cheptsov 
> > Signed-off-by: Michael D Kinney
> 
> > ---
> >   .../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);
>

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

2020-05-20 Thread Sean


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 
Cc: Ard Biesheuvel 
Cc: Bret Barkelew 
Cc: Brian J. Johnson 
Cc: Chasel Chiu 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Marvin H?user 
Cc: Michael D Kinney 
Cc: Vincent Zimmer 
Cc: Zhichao Gao 
Cc: Jiewen Yao 
Cc: Vitaly Cheptsov 
Signed-off-by: Michael D Kinney 
---
  .../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, );
AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", 
RfcDecodeTest, NULL, CleanUpB64TestContext, );
  
+  //

+  // Populate the safe string Unit Test Suite.
+  //
+  Status = CreateUnitTestSuite (, 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--FunctionPre---Post---Context---
+  AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", 
"SafeStringContraintCheckTest", SafeStringContraintCheckTest, NULL, NULL, NULL);
+
//
// Execute the tests.
//



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You

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

2020-05-20 Thread Laszlo Ersek

On 05/20/20 05:11, Michael D Kinney wrote:
> Use the safe string function StrCpyS() in BaseLib to test the
> SAFE_STRING_CONSTRAINT_CHECK() macro.
> 
> Cc: Andrew Fish 
> Cc: Ard Biesheuvel 
> Cc: Bret Barkelew 
> Cc: Brian J. Johnson 
> Cc: Chasel Chiu 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Liming Gao 
> Cc: Marvin H?user 
> Cc: Michael D Kinney 
> Cc: Vincent Zimmer 
> Cc: Zhichao Gao 
> Cc: Jiewen Yao 
> Cc: Vitaly Cheptsov 
> Signed-off-by: Michael D Kinney 
> ---
>  .../UnitTest/Library/BaseLib/Base64UnitTest.c | 85 +++
>  1 file changed, 85 insertions(+)

I'll defer to other reviewers on this patch.

Thanks,
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59987): https://edk2.groups.io/g/devel/message/59987
Mute This Topic: https://groups.io/mt/74342114/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-05-20 Thread Philippe Mathieu-Daudé


On 5/20/20 8:07 AM, Vitaly Cheptsov via groups.io wrote:

Mike,

This looks generally good to me, but there are few issues.

1. There is a mistake in the description here:

+  // Negative test case with DestMax smaller than Source size
+  //
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof 
(CHAR16), Destination);


This should read «Negative test case with Destination and Source 
matching» probably or something alike.


Yes, I was also confused there.

With the description updated:
Reviewed-by: Philippe Mathieu-Daude 



2. For completeness I would also add zeroing before running 
every StrCpyS test, like this:


+ ZeroMem (Destination, sizeof (Destination));
+  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));

Otherwise UT_ASSERT_MEM_EQUAL will miss the check of StrCpyS doing 
anything — Destination could remain unchanged since case 1.


3. I will also make sense to check the Destination string remains 
unchanged on failure for all the other cases, like this:


+  CHAR16         Destination[20];
+  CHAR16         AllZero[20];
+
+  ZeroMem (AllZero, sizeof (AllZero));

+  //
+  // Negative test case with DestMax too big
+  //
+ ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, MAX_UINTN, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));

This behaviour was very unintuitive to me when I met with EDK II 
BaseLib, because I expected something like strlcpy behaviour, where the 
string is truncated to fit, so it is definitely worth testing to ensure 
we do not break things.


Good idea. Feel free to keep my R-b tag if you include Vitaly's suggestions.



Best wishes,
Vitaly

20 мая 2020 г., в 06:11, Michael D Kinney > написал(а):



Use the safe string function StrCpyS() in BaseLib to test the
SAFE_STRING_CONSTRAINT_CHECK() macro.

Cc: Andrew Fish mailto:af...@apple.com>>
Cc: Ard Biesheuvel >
Cc: Bret Barkelew >
Cc: Brian J. Johnson >

Cc: Chasel Chiu mailto:chasel.c...@intel.com>>
Cc: Jordan Justen >

Cc: Laszlo Ersek mailto:ler...@redhat.com>>
Cc: Leif Lindholm mailto:l...@nuviainc.com>>
Cc: Liming Gao mailto:liming@intel.com>>
Cc: Marvin H?user mailto:mhaeu...@outlook.de>>
Cc: Michael D Kinney >
Cc: Vincent Zimmer >

Cc: Zhichao Gao mailto:zhichao@intel.com>>
Cc: Jiewen Yao mailto:jiewen@intel.com>>
Cc: Vitaly Cheptsov >
Signed-off-by: Michael D Kinney >

---
.../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

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

2020-05-20 Thread Vitaly Cheptsov via groups.io

Mike,

This looks generally good to me, but there are few issues.

1. There is a mistake in the description here:

+  // Negative test case with DestMax smaller than Source size
+  //
+  Status = StrCpyS (Destination, sizeof (Destination) / sizeof (CHAR16), 
Destination);

This should read «Negative test case with Destination and Source matching» 
probably or something alike.

2. For completeness I would also add zeroing before running every StrCpyS test, 
like this:

+  ZeroMem (Destination, sizeof (Destination));
+  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));

Otherwise UT_ASSERT_MEM_EQUAL will miss the check of StrCpyS doing anything — 
Destination could remain unchanged since case 1.

3. I will also make sense to check the Destination string remains unchanged on 
failure for all the other cases, like this:

+  CHAR16 Destination[20];
+  CHAR16 AllZero[20];
+
+  ZeroMem (AllZero, sizeof (AllZero));

+  //
+  // Negative test case with DestMax too big
+  //
+  ZeroMem (Destination, sizeof (Destination));
+  Status = StrCpyS (Destination, MAX_UINTN, SOURCE_STRING);
+  UT_ASSERT_STATUS_EQUAL (Status, RETURN_INVALID_PARAMETER);
+  UT_ASSERT_MEM_EQUAL (Destination, AllZero, sizeof (AllZero));

This behaviour was very unintuitive to me when I met with EDK II BaseLib, 
because I expected something like strlcpy behaviour, where the string is 
truncated to fit, so it is definitely worth testing to ensure we do not break 
things.

Best wishes,
Vitaly

> 20 мая 2020 г., в 06:11, Michael D Kinney  
> написал(а):
> 
> 
> Use the safe string function StrCpyS() in BaseLib to test the
> SAFE_STRING_CONSTRAINT_CHECK() macro.
> 
> Cc: Andrew Fish 
> Cc: Ard Biesheuvel 
> Cc: Bret Barkelew 
> Cc: Brian J. Johnson 
> Cc: Chasel Chiu 
> Cc: Jordan Justen 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> Cc: Liming Gao 
> Cc: Marvin H?user 
> Cc: Michael D Kinney 
> Cc: Vincent Zimmer 
> Cc: Zhichao Gao 
> Cc: Jiewen Yao 
> Cc: Vitaly Cheptsov 
> Signed-off-by: Michael D Kinney 
> ---
> .../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);
> +
>

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

2020-05-19 Thread Michael D Kinney

Use the safe string function StrCpyS() in BaseLib to test the
SAFE_STRING_CONSTRAINT_CHECK() macro.

Cc: Andrew Fish 
Cc: Ard Biesheuvel 
Cc: Bret Barkelew 
Cc: Brian J. Johnson 
Cc: Chasel Chiu 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Marvin H?user 
Cc: Michael D Kinney 
Cc: Vincent Zimmer 
Cc: Zhichao Gao 
Cc: Jiewen Yao 
Cc: Vitaly Cheptsov 
Signed-off-by: Michael D Kinney 
---
 .../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, );
   AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", 
RfcDecodeTest, NULL, CleanUpB64TestContext, );
 
+  //
+  // Populate the safe string Unit Test Suite.
+  //
+  Status = CreateUnitTestSuite (, 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--FunctionPre---Post---Context---
+  AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", 
"SafeStringContraintCheckTest", SafeStringContraintCheckTest, NULL, NULL, NULL);
+
   //
   // Execute the tests.
   //
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59918): https://edk2.groups.io/g/devel/message/59918
Mute This Topic: https://groups.io/mt/74342114/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

2020-05-19 Thread Michael D Kinney

Use the safe string function StrCpyS() in BaseLib to test the
SAFE_STRING_CONSTRAINT_CHECK() macro.

Cc: Andrew Fish 
Cc: Ard Biesheuvel 
Cc: Bret Barkelew 
Cc: Brian J. Johnson 
Cc: Chasel Chiu 
Cc: Jordan Justen 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 
Cc: Liming Gao 
Cc: Marvin H?user 
Cc: Michael D Kinney 
Cc: Vincent Zimmer 
Cc: Zhichao Gao 
Cc: Jiewen Yao 
Cc: Vitaly Cheptsov 
Signed-off-by: Michael D Kinney 
---
 .../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, );
   AddTestCase (b64DecodeTests, "Too small of output buffer", "Error5", 
RfcDecodeTest, NULL, CleanUpB64TestContext, );
 
+  //
+  // Populate the safe string Unit Test Suite.
+  //
+  Status = CreateUnitTestSuite (, 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--FunctionPre---Post---Context---
+  AddTestCase (SafeStringTests, "SAFE_STRING_CONSTRAINT_CHECK", 
"SafeStringContraintCheckTest", SafeStringContraintCheckTest, NULL, NULL, NULL);
+
   //
   // Execute the tests.
   //
-- 
2.21.0.windows.1


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#59913): https://edk2.groups.io/g/devel/message/59913
Mute This Topic: https://groups.io/mt/74341930/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

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

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

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

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

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

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

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

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

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

9 matches

Site Navigation

Mail list logo

Footer information