diff mbox series

string: Implement KUnit test for str*cmp functions

Message ID 20240417135415.614284-1-ivan.orlov0322@gmail.com (mailing list archive)
State In Next
Headers show
Series string: Implement KUnit test for str*cmp functions | expand

Commit Message

Ivan Orlov April 17, 2024, 1:54 p.m. UTC
Currently, str*cmp functions (strcmp, strncmp, strcasecmp and
strncasecmp) are not covered with tests. Implement the test which
covers them all.

The strcmp test suite consist of 8 test cases:

1) strcmp test
2) strcmp test on long strings (2048 chars)
3) strncmp test
4) strncmp test on long strings (2048 chars)
5) strcasecmp test
6) strcasecmp test on long strings
7) strncasecmp test
8) strncasecmp test on long strings

The test aims at covering as many edge cases as possible, including
the tests on empty strings, situations when the different symbol is
placed at the end of one of the strings, etc.

Signed-off-by: Ivan Orlov <ivan.orlov0322@gmail.com>
---
 MAINTAINERS        |   5 ++
 lib/Kconfig.debug  |  16 +++++
 lib/Makefile       |   1 +
 lib/strcmp_kunit.c | 170 +++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 192 insertions(+)
 create mode 100644 lib/strcmp_kunit.c

Comments

Andy Shevchenko April 17, 2024, 2:28 p.m. UTC | #1
On Wed, Apr 17, 2024 at 02:54:15PM +0100, Ivan Orlov wrote:
> Currently, str*cmp functions (strcmp, strncmp, strcasecmp and
> strncasecmp) are not covered with tests. Implement the test which
> covers them all.
> 
> The strcmp test suite consist of 8 test cases:
> 
> 1) strcmp test
> 2) strcmp test on long strings (2048 chars)
> 3) strncmp test
> 4) strncmp test on long strings (2048 chars)
> 5) strcasecmp test
> 6) strcasecmp test on long strings
> 7) strncasecmp test
> 8) strncasecmp test on long strings
> 
> The test aims at covering as many edge cases as possible, including
> the tests on empty strings, situations when the different symbol is
> placed at the end of one of the strings, etc.

...

>  lib/strcmp_kunit.c | 170 +++++++++++++++++++++++++++++++++++++++++++++

Why is not part of the existing string_kunit.c?
Ivan Orlov April 17, 2024, 2:46 p.m. UTC | #2
On 4/17/24 15:28, Andy Shevchenko wrote:
> On Wed, Apr 17, 2024 at 02:54:15PM +0100, Ivan Orlov wrote:
>> Currently, str*cmp functions (strcmp, strncmp, strcasecmp and
>> strncasecmp) are not covered with tests. Implement the test which
>> covers them all.
>>
>> The strcmp test suite consist of 8 test cases:
>>
>> 1) strcmp test
>> 2) strcmp test on long strings (2048 chars)
>> 3) strncmp test
>> 4) strncmp test on long strings (2048 chars)
>> 5) strcasecmp test
>> 6) strcasecmp test on long strings
>> 7) strncasecmp test
>> 8) strncasecmp test on long strings
>>
>> The test aims at covering as many edge cases as possible, including
>> the tests on empty strings, situations when the different symbol is
>> placed at the end of one of the strings, etc.
> 
> ...
> 
>>   lib/strcmp_kunit.c | 170 +++++++++++++++++++++++++++++++++++++++++++++
> 
> Why is not part of the existing string_kunit.c?
> 

There are already 2 other KUnit tests in `lib/` covering different 
groups of string functions separately (lib/strscpy_kunit.c, 
lib/strcat_kunit.c), so this patch just follows this pattern. I believe 
it makes sense: the tests are separated to cover one specific group of 
string functions with a similar purpose

---
Kind regards,
Ivan Orlov
Andy Shevchenko April 17, 2024, 2:53 p.m. UTC | #3
On Wed, Apr 17, 2024 at 03:46:44PM +0100, Ivan Orlov wrote:
> On 4/17/24 15:28, Andy Shevchenko wrote:
> > On Wed, Apr 17, 2024 at 02:54:15PM +0100, Ivan Orlov wrote:

...

> > >   lib/strcmp_kunit.c | 170 +++++++++++++++++++++++++++++++++++++++++++++
> > 
> > Why is not part of the existing string_kunit.c?
> 
> There are already 2 other KUnit tests in `lib/` covering different groups of
> string functions separately (lib/strscpy_kunit.c, lib/strcat_kunit.c), so
> this patch just follows this pattern. I believe it makes sense: the tests
> are separated to cover one specific group of string functions with a similar
> purpose

We have handful of the string functions, are you going to have a file per
function? Isn't it way too many?

P.S>
Having those does not prove it's a correct approach. I would rather expect
somebody to incorporate those into string_kunit.c.
Ivan Orlov April 17, 2024, 3:12 p.m. UTC | #4
On 4/17/24 15:53, Andy Shevchenko wrote:
>> There are already 2 other KUnit tests in `lib/` covering different groups of
>> string functions separately (lib/strscpy_kunit.c, lib/strcat_kunit.c), so
>> this patch just follows this pattern. I believe it makes sense: the tests
>> are separated to cover one specific group of string functions with a similar
>> purpose
> 
> We have handful of the string functions, are you going to have a file per
> function? Isn't it way too many?
> 
> P.S>
> Having those does not prove it's a correct approach. I would rather expect
> somebody to incorporate those into string_kunit.c.
> 

Makes sense. Also, probably having all of them in `string_kunit.c` would 
fit better into the KUnit test style guidelines.

I'll merge this strcmp test into `string_kunit.c` in V2 of this patch, 
thank you for the review.
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index c23fda1aa1f0..e23e6b91e2f1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21182,6 +21182,11 @@  W:	http://www.stlinux.com
 F:	Documentation/networking/device_drivers/ethernet/stmicro/
 F:	drivers/net/ethernet/stmicro/stmmac/
 
+STRCMP KUNIT TEST
+M:	Ivan Orlov <ivan.orlov0322@gmail.com>
+S:	Maintained
+F:	lib/strcmp_kunit.c
+
 SUN HAPPY MEAL ETHERNET DRIVER
 M:	Sean Anderson <seanga2@gmail.com>
 S:	Maintained
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index c63a5fbf1f1c..a6992631f32e 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2763,6 +2763,22 @@  config STRCAT_KUNIT_TEST
 	depends on KUNIT
 	default KUNIT_ALL_TESTS
 
+config STRCMP_KUNIT_TEST
+	tristate "Test strcmp() family of functions at runtime" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This option enables the KUnit test for strcmp family of functions:
+	  strcmp, strncmp, strcasecmp and strncasecmp.
+
+	  KUnit tests run during boot and output the results to the debug
+	  log in TAP format (https://testanything.org/). Only useful for
+	  kernel devs running KUnit test harness and are not for inclusion
+	  into a production build.
+
+	  For more information on KUnit and unit tests in general, refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
 config STRSCPY_KUNIT_TEST
 	tristate "Test strscpy*() family of functions at runtime" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/Makefile b/lib/Makefile
index ffc6b2341b45..1559d3bbea7d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -404,6 +404,7 @@  CFLAGS_fortify_kunit.o += $(call cc-disable-warning, stringop-truncation)
 CFLAGS_fortify_kunit.o += $(DISABLE_STRUCTLEAK_PLUGIN)
 obj-$(CONFIG_FORTIFY_KUNIT_TEST) += fortify_kunit.o
 obj-$(CONFIG_STRCAT_KUNIT_TEST) += strcat_kunit.o
+obj-$(CONFIG_STRCMP_KUNIT_TEST) += strcmp_kunit.o
 obj-$(CONFIG_STRSCPY_KUNIT_TEST) += strscpy_kunit.o
 obj-$(CONFIG_SIPHASH_KUNIT_TEST) += siphash_kunit.o
 
diff --git a/lib/strcmp_kunit.c b/lib/strcmp_kunit.c
new file mode 100644
index 000000000000..8c444ea35269
--- /dev/null
+++ b/lib/strcmp_kunit.c
@@ -0,0 +1,170 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * KUnit test for the `strcmp` family of functions: `strcmp`, `strncmp`,
+ * `strcasecmp`, `strncasecmp`.
+ */
+
+#include <kunit/test.h>
+#include <linux/string.h>
+
+#define LARGE_BUF_LEN 2048
+#define CHANGE_POINT 1337
+
+#define STRCMP_TEST_EXPECT_EQUAL(test, fn, ...) KUNIT_EXPECT_EQ(test, fn(__VA_ARGS__), 0)
+
+#define STRCMP_TEST_EXPECT_LOWER(test, fn, ...) KUNIT_EXPECT_LT(test, fn(__VA_ARGS__), 0)
+
+#define STRCMP_TEST_EXPECT_GREATER(test, fn, ...) KUNIT_EXPECT_GT(test, fn(__VA_ARGS__), 0)
+
+static char buffer1[LARGE_BUF_LEN];
+static char buffer2[LARGE_BUF_LEN];
+
+static void strcmp_test(struct kunit *test)
+{
+	/* Equal strings */
+	STRCMP_TEST_EXPECT_EQUAL(test, strcmp, "Hello, Kernel!", "Hello, Kernel!");
+	/* First string is lexicographically less than the second */
+	STRCMP_TEST_EXPECT_LOWER(test, strcmp, "Hello, KUnit!", "Hello, Kernel!");
+	/* First string is lexicographically larger than the second */
+	STRCMP_TEST_EXPECT_GREATER(test, strcmp, "Hello, Kernel!", "Hello, KUnit!");
+	/* Empty string is always lexicographically less than any non-empty string */
+	STRCMP_TEST_EXPECT_LOWER(test, strcmp, "", "Non-empty string");
+	/* Two empty strings should be equal */
+	STRCMP_TEST_EXPECT_EQUAL(test, strcmp, "", "");
+	/* Compare two strings which have only one char difference */
+	STRCMP_TEST_EXPECT_LOWER(test, strcmp, "Abacaba", "Abadaba");
+	/* Compare two strings which have the same prefix*/
+	STRCMP_TEST_EXPECT_LOWER(test, strcmp, "Just a string", "Just a string and something else");
+}
+
+static void fill_buffers(char fill1, char fill2)
+{
+	memset(buffer1, fill1, LARGE_BUF_LEN);
+	memset(buffer2, fill2, LARGE_BUF_LEN);
+	buffer1[LARGE_BUF_LEN - 1] = 0;
+	buffer2[LARGE_BUF_LEN - 1] = 0;
+}
+
+static void strcmp_long_strings_test(struct kunit *test)
+{
+	fill_buffers('B', 'B');
+	STRCMP_TEST_EXPECT_EQUAL(test, strcmp, buffer1, buffer2);
+
+	buffer1[CHANGE_POINT] = 'A';
+	STRCMP_TEST_EXPECT_LOWER(test, strcmp, buffer1, buffer2);
+
+	buffer1[CHANGE_POINT] = 'C';
+	STRCMP_TEST_EXPECT_GREATER(test, strcmp, buffer1, buffer2);
+}
+
+static void strncmp_test(struct kunit *test)
+{
+	/* Equal strings */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, "Hello, KUnit!", "Hello, KUnit!", 13);
+	/* First string is lexicographically less than the second */
+	STRCMP_TEST_EXPECT_LOWER(test, strncmp, "Hello, KUnit!", "Hello, Kernel!", 13);
+	/* Result is always 'equal' when count = 0 */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, "Hello, Kernel!", "Hello, KUnit!", 0);
+	/* Strings with common prefix are equal if count = length of prefix */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, "Abacaba", "Abadaba", 3);
+	/* Strings with common prefix are not equal when count = length of prefix + 1 */
+	STRCMP_TEST_EXPECT_LOWER(test, strncmp, "Abacaba", "Abadaba", 4);
+	/* If one string is a prefix of another, the shorter string is lexicographically smaller */
+	STRCMP_TEST_EXPECT_LOWER(test, strncmp, "Just a string", "Just a string and something else",
+				 strlen("Just a string and something else"));
+	/*
+	 * If one string is a prefix of another, and we check first length
+	 * of prefix chars, the result is 'equal'
+	 */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, "Just a string", "Just a string and something else",
+				 strlen("Just a string"));
+}
+
+static void strncmp_long_strings_test(struct kunit *test)
+{
+	fill_buffers('B', 'B');
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, buffer1, buffer2, LARGE_BUF_LEN);
+
+	buffer1[CHANGE_POINT] = 'A';
+	STRCMP_TEST_EXPECT_LOWER(test, strncmp, buffer1, buffer2, LARGE_BUF_LEN);
+
+	buffer1[CHANGE_POINT] = 'C';
+	STRCMP_TEST_EXPECT_GREATER(test, strncmp, buffer1, buffer2, LARGE_BUF_LEN);
+	/* the strings are equal up to CHANGE_POINT */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncmp, buffer1, buffer2, CHANGE_POINT);
+	STRCMP_TEST_EXPECT_GREATER(test, strncmp, buffer1, buffer2, CHANGE_POINT + 1);
+}
+
+static void strcasecmp_test(struct kunit *test)
+{
+	/* Same strings in different case should be equal */
+	STRCMP_TEST_EXPECT_EQUAL(test, strcasecmp, "Hello, Kernel!", "HeLLO, KErNeL!");
+	/* Empty strings should be equal */
+	STRCMP_TEST_EXPECT_EQUAL(test, strcasecmp, "", "");
+	/* Despite ascii code for 'a' is larger than ascii code for 'B', 'a' < 'B' */
+	STRCMP_TEST_EXPECT_LOWER(test, strcasecmp, "a", "B");
+	STRCMP_TEST_EXPECT_GREATER(test, strcasecmp, "B", "a");
+	/* Special symbols and numbers should be processed correctly */
+	STRCMP_TEST_EXPECT_EQUAL(test, strcasecmp, "-+**.1230ghTTT~^", "-+**.1230Ghttt~^");
+}
+
+static void strcasecmp_long_strings_test(struct kunit *test)
+{
+	fill_buffers('b', 'B');
+	STRCMP_TEST_EXPECT_EQUAL(test, strcasecmp, buffer1, buffer2);
+
+	buffer1[CHANGE_POINT] = 'a';
+	STRCMP_TEST_EXPECT_LOWER(test, strcasecmp, buffer1, buffer2);
+
+	buffer1[CHANGE_POINT] = 'C';
+	STRCMP_TEST_EXPECT_GREATER(test, strcasecmp, buffer1, buffer2);
+}
+
+static void strncasecmp_test(struct kunit *test)
+{
+	/* Same strings in different case should be equal */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncasecmp, "AbAcAbA", "Abacaba", strlen("Abacaba"));
+	/* strncasecmp should check 'count' chars only */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncasecmp, "AbaCaBa", "abaCaDa", 5);
+	STRCMP_TEST_EXPECT_LOWER(test, strncasecmp, "a", "B", 1);
+	STRCMP_TEST_EXPECT_GREATER(test, strncasecmp, "B", "a", 1);
+	/* Result is always 'equal' when count = 0 */
+	STRCMP_TEST_EXPECT_EQUAL(test, strncasecmp, "Abacaba", "Not abacaba", 0);
+}
+
+static void strncasecmp_long_strings_test(struct kunit *test)
+{
+	fill_buffers('b', 'B');
+	STRCMP_TEST_EXPECT_EQUAL(test, strncasecmp, buffer1, buffer2, LARGE_BUF_LEN);
+
+	buffer1[CHANGE_POINT] = 'a';
+	STRCMP_TEST_EXPECT_LOWER(test, strncasecmp, buffer1, buffer2, LARGE_BUF_LEN);
+
+	buffer1[CHANGE_POINT] = 'C';
+	STRCMP_TEST_EXPECT_GREATER(test, strncasecmp, buffer1, buffer2, LARGE_BUF_LEN);
+
+	STRCMP_TEST_EXPECT_EQUAL(test, strncasecmp, buffer1, buffer2, CHANGE_POINT);
+	STRCMP_TEST_EXPECT_GREATER(test, strncasecmp, buffer1, buffer2, CHANGE_POINT + 1);
+}
+
+static struct kunit_case strcmp_test_cases[] = {
+	KUNIT_CASE(strcmp_test),
+	KUNIT_CASE(strcmp_long_strings_test),
+	KUNIT_CASE(strncmp_test),
+	KUNIT_CASE(strncmp_long_strings_test),
+	KUNIT_CASE(strcasecmp_test),
+	KUNIT_CASE(strcasecmp_long_strings_test),
+	KUNIT_CASE(strncasecmp_test),
+	KUNIT_CASE(strncasecmp_long_strings_test),
+	{},
+};
+
+static struct kunit_suite strcmp_test_suite = {
+	.name = "strcmp",
+	.test_cases = strcmp_test_cases,
+};
+
+kunit_test_suite(strcmp_test_suite);
+
+MODULE_AUTHOR("Ivan Orlov <ivan.orlov0322@gmail.com>");
+MODULE_LICENSE("GPL");