Change in coreboot[master]: tests: Complete lib/string-test test case

[Anna Karas (Code Review)]

Anna Karas has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

tests: Complete lib/string-test test case

Implement unit tests for remaining string library functions. Fix memory leak
in test_strdup().

Signed-off-by: Anna Karas 
Change-Id: I8ac6a6b2413d9077dc9ea81f638a2b0acd5c8862
---
M tests/lib/string-test.c
1 file changed, 212 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/42311/1

diff --git a/tests/lib/string-test.c b/tests/lib/string-test.c
index 08f4177..39e9be9 100644
--- a/tests/lib/string-test.c
+++ b/tests/lib/string-test.c
@@ -1,50 +1,243 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <string.h>
+#include <stdlib.h>
+#include <stdint.h>
 #include <tests/test.h>

+#define DST_SIZE 20
+
 /*
  * Important note: In every particular test, don't use any string-related
  * functions other than function under test. We are linking against
  * src/lib/string.c not the standard library. This is important for proper test
- * isolation. One can use __builtin_xxx for many of the most simple str*()
- * functions, when non-coreboot one is required.
+ * isolation.
  */

-struct strings_t {
-	char *str;
-	size_t size;
-} strings[] = {
-	{"coreboot",	8},
-	{"is\0very",	2}, /* strlen should be 2 because of the embedded \0 */
-	{"nice\n",	5}
+struct string_pairs_t {
+	char *dst;
+	char *src;
+} string_pairs[] = {
+	{"Hello ", "world!"},
+	{"He\0llo ", "world"},
+	{"", "world!"},
+	{"", ""},
 };

-static void test_strlen_strings(void **state)
-{
-	int i;
+const char *strings[] = {
+	"coreboot",
+	"is\0very",
+	"nice\n"
+};

-	for (i = 0; i < ARRAY_SIZE(strings); i++)
-		assert_int_equal(strings[i].size, strlen(strings[i].str));
-}
+struct str_with_l_val_t {
+	char *str;
+	long value;
+} str_with_l_val[] = {
+	{"42", 42},
+	{"four42", 0},
+	{"42five5",	42},
+	{"4\02", 4},
+	{"+42", 42},
+	{"-42", -42},
+	{"\t\n\r\f\v-42", -42},
+};
+
+struct str_with_u_val_t {
+	char *str;
+	uint32_t value;
+} str_with_u_val[] = {
+	{"42aa", 42},
+	{"a", 0},
+	{"0", 0},
+	{"4a2", 4},
+};

static void test_strdup(void **state)
 {
    char str[] = "Hello coreboot\n";
-	char *duplicate;
-
-	duplicate = strdup(str);
+	char *duplicate = strdup(str);

/* There is a more suitable Cmocka's function 'assert_string_equal()', but it
       is using strcmp() internally. */
    assert_int_equal(0, memcmp(str, duplicate, __builtin_strlen(str)));
+
+	free(duplicate);
+}
+
+static void test_strconcat(void **state)
+{
+	int i;
+	size_t str_len, str2_len, res_len;
+	char *result;
+
+	for (i = 0; i < ARRAY_SIZE(string_pairs); i++) {
+		str_len = __builtin_strlen(string_pairs[i].dst);
+		str2_len = __builtin_strlen(string_pairs[i].src);
+
+		result = strconcat(string_pairs[i].dst, string_pairs[i].src);
+		res_len = __builtin_strlen(result);
+
+		assert_int_equal(res_len, str_len + str2_len);
+		assert_int_equal(0, memcmp(string_pairs[i].dst, result, str_len));
+		assert_int_equal(0, memcmp(string_pairs[i].src, result + str_len, str2_len));
+
+		free(result);
+	}
+}
+
+static void test_strnlen(void **state)
+{
+	int i, n = 5;
+	size_t str_len, limited_len;
+
+	for (i = 0; i < ARRAY_SIZE(strings); i++) {
+		str_len = __builtin_strlen(strings[i]);
+		limited_len = str_len > n ? n : str_len;
+		assert_int_equal(limited_len, strnlen(strings[i], n));
+	}
+}
+
+static void test_strlen(void **state)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(strings); i++)
+		assert_int_equal(__builtin_strlen(strings[i]), strlen(strings[i]));
+}
+
+static void test_strchr(void **state)
+{
+	char str[] = "Abracadabra!\n";
+
+	assert_ptr_equal(str, strchr(str, 'A'));
+	assert_ptr_equal(str + 3, strchr(str, 'a'));
+	assert_ptr_not_equal(str + 10, strchr(str, 'a'));
+
+	assert_null(strchr(str, 'z'));
+}
+
+
+static void test_strrchr(void **state)
+{
+	char str[] = "Abracadabra!\n";
+
+	assert_ptr_equal(str + 9, strrchr(str, 'r'));
+	assert_ptr_not_equal(str + 2, strrchr(str, 'r'));
+
+	assert_null(strrchr(str, 'z'));
+}
+
+
+static void test_strncpy(void **state)
+{
+	int i, n = 7;
+	char dst[DST_SIZE];
+	char src[] = "Hello coreboot\n";
+	size_t src_len = __builtin_strlen(src);
+
+	strncpy(dst, src, n);
+
+	assert_int_equal(0, memcmp(dst, src, n));
+
+	for (i = src_len; i < n; i++)
+		assert_true(dst[i] == '\0');
+}
+
+static void test_strcpy(void **state)
+{
+	char dst[DST_SIZE];
+	char src[] = "Hello coreboot\n";
+
+	/* Make sure that strcpy() sets '\0' by initializing a whole
+	   dst array to fixed, non-'\0' value */
+	memset(dst, 'x', DST_SIZE);
+
+	strcpy(dst, src);
+
+	assert_int_equal(0, memcmp(dst, src, __builtin_strlen(src)));
+	assert_true(dst[__builtin_strlen(dst)] == '\0');
+}
+
+static void test_strcmp(void **state)
+{
+	char str[] = "Banana";
+	char str2[] = "Bananas";
+
+	assert_true(strcmp(str, str2) < 0);
+	assert_int_equal(0, strcmp(str, str));
+	assert_true(strcmp(str2, str) > 0);
+}
+
+static void test_strncmp(void **state)
+{
+	char str[] = "Banana";
+	char str2[] = "Bananas";
+
+	size_t str2_len = __builtin_strlen(str2);
+
+	assert_true(strncmp(str, str2, str2_len) < 0);
+	assert_int_equal(0, strncmp(str, str2, str2_len - 1));
+}
+
+static void test_skip_atoi(void **state)
+{
+	int i;
+	char *ptr;
+
+	for (i = 0; i < ARRAY_SIZE(str_with_u_val); i++) {
+		ptr = str_with_u_val[i].str;
+		assert_int_equal(str_with_u_val[i].value, skip_atoi(&ptr));
+	}
+}
+
+static void test_strspn(void **state)
+{
+	char str[] = "123404567";
+	char str2[] = "01234";
+	char str3[] = "1234";
+
+	assert_int_equal(4, strspn(str, str3));
+	assert_int_equal(0, strspn(str2, str3));
+}
+
+static void test_strcspn(void **state)
+{
+	char str[] = "12340000";
+	char str2[] = "00001234";
+	char str3[] = "1234";
+
+	assert_int_equal(0, strcspn(str, str3));
+	assert_int_equal(4, strcspn(str2, str3));
+}
+
+/* Please bear in mind that `atol()` uses `strspn()` internally, so the result
+   of `test_atol` is dependent on the result of `test_strspn`. */
+static void test_atol(void **state)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(str_with_l_val); i++)
+		assert_int_equal(str_with_l_val[i].value, atol(str_with_l_val[i].str));
 }

int main(void)
 {
    const struct CMUnitTest tests[] = {
-		cmocka_unit_test(test_strlen_strings),
    	cmocka_unit_test(test_strdup),
+		cmocka_unit_test(test_strconcat),
+		cmocka_unit_test(test_strnlen),
+		cmocka_unit_test(test_strlen),
+		cmocka_unit_test(test_strchr),
+		cmocka_unit_test(test_strrchr),
+		cmocka_unit_test(test_strncpy),
+		cmocka_unit_test(test_strcpy),
+		cmocka_unit_test(test_strcmp),
+		cmocka_unit_test(test_strncmp),
+		cmocka_unit_test(test_skip_atoi),
+		cmocka_unit_test(test_strspn),
+		cmocka_unit_test(test_strcspn),
+		cmocka_unit_test(test_atol),
    };

return cmocka_run_group_tests(tests, NULL, NULL);

[HAOUAS Elyes (Code Review)]

HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@5 
PS1, Line 5: stdint.h
<stddef.h> for size_t

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 1:

(3 comments)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@33 
PS1, Line 33: struct str_with_l_val_t {
Consider adding comments here and for str_with_u_val_t that these are being used to test atol and atoi

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@48 
PS1, Line 48: uint32_t
atoi returns a signed int

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@190 
PS1, Line 190: assert_int_equal
assert_int_equal expects signed integers. Once you fix the type for str_with_u_val.value this will be OK

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 1:

(9 comments)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@a10 
PS1, Line 10:  One can use __builtin_xxx for many of the most simple str*()
            :  * functions, when non-coreboot one is required.
Not sure why you're removing this? I think it's still useful advice...

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@96 
PS1, Line 96: str_len > n ? n : str_len;
nit: could also be MAX(n, str_len)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@126 
PS1, Line 126: 	assert_ptr_not_equal(str + 2, strrchr(str, 'r'));
nit: This one is kinda redundant with the line above it. str + 9 could never equal str + 2. Maybe instead try to find the 'A' or the '\n' to make sure there are no edge cases with first/last character?

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@135 
PS1, Line 135: DST_SIZE
Could just reorder the definitions and say sizeof(src) here so you don't need to hardcode a constant for this.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@139 
PS1, Line 139: n
nit: might also be interesting to copy less and ensure that characters behind the limit are not overwritten.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@159 
PS1, Line 159: 	assert_true(dst[__builtin_strlen(dst)] == '\0');
This is sort of a tautology because __builtin_strlen() always finds a \0 by definition (so this will be true even if the string wasn't terminated and __builtin_strlen(dst) finds some other zero byte further down the stack). Should say __builtin_strlen(src) here to make sure the \0 is really where it is supposed to be. (Alternatively, could just add +1 to the size of the line above and memcmp() will already check that for you.)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@168 
PS1, Line 168: 	assert_int_equal(0, strcmp(str, str));
nit: this would also be true if strcmp() just compared pointers, I think comparing two distinct arrays would be better.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@191 
PS1, Line 191: 	}
Would be good to pass a real (char **) variable and confirm that the pointer is updated to point behind the parsed number, as described in the skip_atoi() header comment.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@198 
PS1, Line 198: 	char str3[] = "1234";
The order of these should be different than the order in the tested string, to make sure strspn() doesn't just do a strcmp() but actually treats this as a set of characters. (Also, would be better if a few of these characters appear more than once in the found segment, just to make sure strspn() doesn't try to check for each of them only once.)

[Jan Dabros (Code Review)]

Jan Dabros has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 1:

(1 comment)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@a10 
PS1, Line 10:  One can use __builtin_xxx for many of the most simple str*()
            :  * functions, when non-coreboot one is required.
> Not sure why you're removing this? I think it's still useful advice...

Actually this comment is dangerous a little bit. Most of calls to the __builtin_*() functions (e.g. __builtin_strspn()), generates simple call to C library (e.g. strspn()) in the output object file. After linkage, this ends up as a call to the UUT. __builtin_strlen() is working OK, since compiler is optimizing this, by inserting constant value. Considering above, I agree that we should remove this line (optionally add a comment to keep using only __builtin_strlen() throughout the module).

[Anna Karaś (Code Review)]

Hello build bot (Jenkins), Patrick Georgi, Julius Werner, Jan Dabros, Paul Fagerburg,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/42311

to look at the new patch set (#2).

Change subject: tests: Complete lib/string-test test case
......................................................................

tests: Complete lib/string-test test case

Implement unit tests for remaining string library functions. Fix memory leak
in test_strdup().

Signed-off-by: Anna Karas 
Change-Id: I8ac6a6b2413d9077dc9ea81f638a2b0acd5c8862
---
M tests/lib/string-test.c
1 file changed, 236 insertions(+), 19 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/11/42311/2

[Anna Karaś (Code Review)]

Anna Karaś has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 2:

(12 comments)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@5 
PS1, Line 5: stdint.h
> <stddef. […]

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@33 
PS1, Line 33: struct str_with_l_val_t {
> Consider adding comments here and for str_with_u_val_t that these are being used to test atol and at […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@48 
PS1, Line 48: uint32_t
> atoi returns a signed int

I based choice of function return type on an already existing atoi implementation from src/lib/string.c - "unsigned int skip_atoi(char **s)".

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@96 
PS1, Line 96: str_len > n ? n : str_len;
> nit: could also be MAX(n, str_len)

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@126 
PS1, Line 126: 	assert_ptr_not_equal(str + 2, strrchr(str, 'r'));
> nit: This one is kinda redundant with the line above it. str + 9 could never equal str + 2. […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@135 
PS1, Line 135: DST_SIZE
> Could just reorder the definitions and say sizeof(src) here so you don't need to hardcode a constant […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@139 
PS1, Line 139: n
> nit: might also be interesting to copy less and ensure that characters behind the limit are not over […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@159 
PS1, Line 159: 	assert_true(dst[__builtin_strlen(dst)] == '\0');
> This is sort of a tautology because __builtin_strlen() always finds a \0 by definition (so this will […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@168 
PS1, Line 168: 	assert_int_equal(0, strcmp(str, str));
> nit: this would also be true if strcmp() just compared pointers, I think comparing two distinct arra […]

Done.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@190 
PS1, Line 190: assert_int_equal
> assert_int_equal expects signed integers. Once you fix the type for str_with_u_val. […]

Until we determine (see one of comments above) whether to keep the unsigned int type or change to signed int, I suggest temporarily replacing assert_int_equal with assert_true.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@191 
PS1, Line 191: 	}
> Would be good to pass a real (char **) variable and confirm that the pointer is updated to point beh […]

I suppose that confirmation of the update is still possible with this implementation. Just add an "offset" field to str_with_u_val and compare it with a difference between updated ptr and a copy of ptr (the copy made before the update of ptr).

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@198 
PS1, Line 198: 	char str3[] = "1234";
> The order of these should be different than the order in the tested string, to make sure strspn() do […]

Done.

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 2:

For any comments that you have resolved, you should mark them resolved so that we can see which ones are still outstanding.

[Anna Karaś (Code Review)]

Anna Karaś has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 2:

(8 comments)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@a10 
PS1, Line 10:  One can use __builtin_xxx for many of the most simple str*()
            :  * functions, when non-coreboot one is required.
> Actually this comment is dangerous a little bit. Most of calls to the __builtin_*() functions (e.g. […]

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@33 
PS1, Line 33: struct str_with_l_val_t {
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@126 
PS1, Line 126: 	assert_ptr_not_equal(str + 2, strrchr(str, 'r'));
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@135 
PS1, Line 135: DST_SIZE
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@139 
PS1, Line 139: n
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@159 
PS1, Line 159: 	assert_true(dst[__builtin_strlen(dst)] == '\0');
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@168 
PS1, Line 168: 	assert_int_equal(0, strcmp(str, str));
> Done.

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@198 
PS1, Line 198: 	char str3[] = "1234";
> Done.

Done

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 2: Code-Review+2

[Anna Karaś (Code Review)]

Anna Karaś has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 2:

I have already updated this patch, could you please take a look at it? 😊

[Paul Fagerburg (Code Review)]

Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 3: Code-Review+2

[Julius Werner (Code Review)]

Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

Patch Set 3:

(3 comments)

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c 
File tests/lib/string-test.c:

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@48 
PS1, Line 48: uint32_t
> I based choice of function return type on an already existing atoi implementation from src/lib/strin […]

You always need to mark all comments as "Resolved" before Gerrit allows us to merge something. You can check the "Comment Threads" -> "Only unresolved threads" view to see what's still missing.

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@190 
PS1, Line 190: assert_int_equal
> Until we determine (see one of comments above) whether to keep the unsigned int type or change to si […]

Done

https://review.coreboot.org/c/coreboot/+/42311/1/tests/lib/string-test.c@191 
PS1, Line 191: 	}
> I suppose that confirmation of the update is still possible with this implementation. […]

Done

[Julius Werner (Code Review)]

Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/42311 )

Change subject: tests: Complete lib/string-test test case
......................................................................

tests: Complete lib/string-test test case

Implement unit tests for remaining string library functions. Fix memory leak
in test_strdup().

Signed-off-by: Anna Karas 
Change-Id: I8ac6a6b2413d9077dc9ea81f638a2b0acd5c8862
Reviewed-on: https://review.coreboot.org/c/coreboot/+/42311
Reviewed-by: Paul Fagerburg 
Reviewed-by: Julius Werner 
Tested-by: build bot (Jenkins) 
---
M tests/lib/string-test.c
1 file changed, 236 insertions(+), 19 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Julius Werner: Looks good to me, approved
  Paul Fagerburg: Looks good to me, approved

diff --git a/tests/lib/string-test.c b/tests/lib/string-test.c
index 08f4177..859f749 100644
--- a/tests/lib/string-test.c
+++ b/tests/lib/string-test.c
@@ -1,50 +1,267 @@
 /* SPDX-License-Identifier: GPL-2.0-only */

#include <string.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stddef.h>
 #include <tests/test.h>
+#include <stdio.h>

/*
  * Important note: In every particular test, don't use any string-related
  * functions other than function under test. We are linking against
  * src/lib/string.c not the standard library. This is important for proper test
- * isolation. One can use __builtin_xxx for many of the most simple str*()
- * functions, when non-coreboot one is required.
+ * isolation.
  */

-struct strings_t {
-	char *str;
-	size_t size;
-} strings[] = {
-	{"coreboot",	8},
-	{"is\0very",	2}, /* strlen should be 2 because of the embedded \0 */
-	{"nice\n",	5}
+struct string_pairs_t {
+	char *dst;
+	char *src;
+} string_pairs[] = {
+	{"Hello ", "world!"},
+	{"He\0llo ", "world"},
+	{"", "world!"},
+	{"", ""},
 };

-static void test_strlen_strings(void **state)
-{
-	int i;
+const char *strings[] = {
+	"coreboot",
+	"is\0very",
+	"nice\n"
+};

-	for (i = 0; i < ARRAY_SIZE(strings); i++)
-		assert_int_equal(strings[i].size, strlen(strings[i].str));
-}
+/* Used to test atol */
+struct str_with_l_val_t {
+	char *str;
+	long value;
+} str_with_l_val[] = {
+	{"42", 42},
+	{"four42", 0},
+	{"42five5",	42},
+	{"4\02", 4},
+	{"+42", 42},
+	{"-42", -42},
+	{"\t\n\r\f\v-42", -42},
+};
+
+/* Used to test skip_atoi */
+struct str_with_u_val_t {
+	char *str;
+	uint32_t value;
+} str_with_u_val[] = {
+	{"42aa", 42},
+	{"a", 0},
+	{"0", 0},
+	{"4a2", 4},
+};

static void test_strdup(void **state)
 {
    char str[] = "Hello coreboot\n";
-	char *duplicate;
-
-	duplicate = strdup(str);
+	char *duplicate = strdup(str);

/* There is a more suitable Cmocka's function 'assert_string_equal()', but it
       is using strcmp() internally. */
    assert_int_equal(0, memcmp(str, duplicate, __builtin_strlen(str)));
+
+	free(duplicate);
+}
+
+static void test_strconcat(void **state)
+{
+	int i;
+	size_t str_len, str2_len, res_len;
+	char *result;
+
+	for (i = 0; i < ARRAY_SIZE(string_pairs); i++) {
+		str_len = __builtin_strlen(string_pairs[i].dst);
+		str2_len = __builtin_strlen(string_pairs[i].src);
+
+		result = strconcat(string_pairs[i].dst, string_pairs[i].src);
+		res_len = __builtin_strlen(result);
+
+		assert_int_equal(res_len, str_len + str2_len);
+		assert_int_equal(0, memcmp(string_pairs[i].dst, result, str_len));
+		assert_int_equal(0, memcmp(string_pairs[i].src, result + str_len, str2_len));
+
+		free(result);
+	}
+}
+
+static void test_strnlen(void **state)
+{
+	int i, n = 5;
+	size_t str_len, limited_len;
+
+	for (i = 0; i < ARRAY_SIZE(strings); i++) {
+		str_len = __builtin_strlen(strings[i]);
+		limited_len = MIN(n, str_len);
+		assert_int_equal(limited_len, strnlen(strings[i], n));
+	}
+}
+
+static void test_strlen(void **state)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(strings); i++)
+		assert_int_equal(__builtin_strlen(strings[i]), strlen(strings[i]));
+}
+
+static void test_strchr(void **state)
+{
+	char str[] = "Abracadabra!\n";
+
+	assert_ptr_equal(str, strchr(str, 'A'));
+	assert_ptr_equal(str + 3, strchr(str, 'a'));
+	assert_ptr_equal(str + 12, strchr(str, '\n'));
+
+	assert_null(strchr(str, 'z'));
+}
+
+
+static void test_strrchr(void **state)
+{
+	char str[] = "Abracadabra!\n";
+
+	assert_ptr_equal(str, strrchr(str, 'A'));
+	assert_ptr_equal(str + 9, strrchr(str, 'r'));
+	assert_ptr_equal(str + 12, strrchr(str, '\n'));
+
+	assert_null(strrchr(str, 'z'));
+}
+
+static void test_strncpy(void **state)
+{
+	int i;
+	int n1 = 2, n2 = 8;
+	char src[] = "Hello";
+	char dst[sizeof(src) + 5];
+	size_t src_len = __builtin_strlen(src);
+	size_t dst_len = sizeof(dst);
+
+	/* n1 case  */
+
+	/* Needed for ensuring that characters behind the limit
+	   are not overwritten */
+	memset(dst, 'x', dst_len);
+
+	strncpy(dst, src, n1);
+
+	assert_int_equal(0, memcmp(dst, src, n1));
+
+	for (i = n1; i < dst_len; i++)
+		assert_true(dst[i] == 'x');
+
+	/* n2 case: */
+
+	memset(dst, 'x', dst_len);
+
+	strncpy(dst, src, n2);
+
+	assert_int_equal(0, memcmp(dst, src, src_len));
+
+	for (i = src_len; i < n2; i++)
+		assert_true(dst[i] == '\0');
+
+	for (i = n2; i < dst_len; i++)
+		assert_true(dst[i] == 'x');
+}
+
+static void test_strcpy(void **state)
+{
+	char src[] = "Hello coreboot\n";
+	char dst[sizeof(src)];
+
+	/* Make sure that strcpy() sets '\0' by initializing a whole
+	   dst array to fixed, non-'\0' value */
+	memset(dst, 'x', sizeof(dst));
+
+	strcpy(dst, src);
+
+	assert_int_equal(0, memcmp(dst, src, __builtin_strlen(src) + 1));
+}
+
+static void test_strcmp(void **state)
+{
+	char str[] = "Banana";
+	char str2[] = "Banana";
+	char str3[] = "Bananas";
+
+	assert_true(strcmp(str, str3) < 0);
+	assert_int_equal(0, strcmp(str, str2));
+	assert_true(strcmp(str3, str2) > 0);
+}
+
+static void test_strncmp(void **state)
+{
+	char str[] = "Banana";
+	char str2[] = "Bananas";
+
+	size_t str2_len = __builtin_strlen(str2);
+
+	assert_true(strncmp(str, str2, str2_len) < 0);
+	assert_int_equal(0, strncmp(str, str2, str2_len - 1));
+}
+
+static void test_skip_atoi(void **state)
+{
+	int i;
+	char *ptr;
+
+	for (i = 0; i < ARRAY_SIZE(str_with_u_val); i++) {
+		ptr = str_with_u_val[i].str;
+		assert_true(str_with_u_val[i].value == skip_atoi(&ptr));
+	}
+}
+
+static void test_strspn(void **state)
+{
+	char str[] = "4213401234";
+	char str2[] = "01234";
+	char str3[] = "1234";
+
+	assert_int_equal(5, strspn(str, str3));
+	assert_int_equal(0, strspn(str2, str3));
+}
+
+static void test_strcspn(void **state)
+{
+	char str[] = "12340000";
+	char str2[] = "00001234";
+	char str3[] = "1234";
+
+	assert_int_equal(0, strcspn(str, str3));
+	assert_int_equal(4, strcspn(str2, str3));
+}
+
+/* Please bear in mind that `atol()` uses `strspn()` internally, so the result
+   of `test_atol` is dependent on the result of `test_strspn`. */
+static void test_atol(void **state)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(str_with_l_val); i++)
+		assert_int_equal(str_with_l_val[i].value, atol(str_with_l_val[i].str));
 }

int main(void)
 {
    const struct CMUnitTest tests[] = {
-		cmocka_unit_test(test_strlen_strings),
    	cmocka_unit_test(test_strdup),
+		cmocka_unit_test(test_strconcat),
+		cmocka_unit_test(test_strnlen),
+		cmocka_unit_test(test_strlen),
+		cmocka_unit_test(test_strchr),
+		cmocka_unit_test(test_strrchr),
+		cmocka_unit_test(test_strncpy),
+		cmocka_unit_test(test_strcpy),
+		cmocka_unit_test(test_strcmp),
+		cmocka_unit_test(test_strncmp),
+		cmocka_unit_test(test_skip_atoi),
+		cmocka_unit_test(test_strspn),
+		cmocka_unit_test(test_strcspn),
+		cmocka_unit_test(test_atol),
    };

return cmocka_run_group_tests(tests, NULL, NULL);