[PATCH RESEND 0/3] lib/string: search for NUL with strchr/strnchr

[PATCH RESEND 0/3] lib/string: search for NUL with strchr/strnchr

[Peter Rosin]

[With better address for Matthew?]

Hi!

I noticed an inconsistency where strchr and strnchr do not behave the
same with respect to the trailing NUL. strchr is standardised and the
kernel function conforms, and the kernel relies on the behavior.
So, naturally strchr stays as-is and strnchr is what I change.

While writing a few tests to verify that my new strnchr loop was sane, I
noticed that the tests for memset16/32/64 had a problem. Since it's all
about the lib/string.c file I made a short series of it all...

But where to send it? get_maintainer suggests no victim, so I'm aiming
at those that signed-off on the memset16/32/64 bug...

Cheers,
Peter

Peter Rosin (3):
  lib/string: allow searching for NUL with strnchr
  lib/test_string: avoid masking memset16/32/64 failures
  lib/test_string: add some testcases for strchr and strnchr

 lib/string.c      | 11 +++++++-
 lib/test_string.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 90 insertions(+), 4 deletions(-)

-- 
2.11.0

[PATCH RESEND 1/3] lib/string: allow searching for NUL with strnchr

[Peter Rosin]

strchr considers the terminating NUL to be part of the string, and NUL
can thus be searched for with that function. For consistency, do the
same with strnchr.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 lib/string.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/string.c b/lib/string.c
index 3ab861c1a857..9d64d7ab401a 100644
--- a/lib/string.c
+++ b/lib/string.c
@@ -367,6 +367,9 @@ EXPORT_SYMBOL(strncmp);
  * strchr - Find the first occurrence of a character in a string
  * @s: The string to be searched
  * @c: The character to search for
+ *
+ * Note that the %NUL-terminator is considered part of the string, and can
+ * be searched for.
  */
 char *strchr(const char *s, int c)
 {
@@ -420,12 +423,18 @@ EXPORT_SYMBOL(strrchr);
  * @s: The string to be searched
  * @count: The number of characters to be searched
  * @c: The character to search for
+ *
+ * Note that the %NUL-terminator is considered part of the string, and can
+ * be searched for.
  */
 char *strnchr(const char *s, size_t count, int c)
 {
-	for (; count-- && *s != '\0'; ++s)
+	while (count--) {
 		if (*s == (char)c)
 			return (char *)s;
+		if (*s++ == '\0')
+			break;
+	}
 	return NULL;
 }
 EXPORT_SYMBOL(strnchr);
-- 
2.11.0

[PATCH RESEND 2/3] lib/test_string: avoid masking memset16/32/64 failures

[Peter Rosin]

If a memsetXX implementation is completely broken and fails in the first
iteration, when i, j, and k are all zero, the failure is masked as zero
is returned. Failing in the first iteration is perhaps the most likely
failure, so this makes the tests pretty much useless. Avoid the situation
by always setting a random unused bit in the result on failure.

Fixes: 03270c13c5ff ("lib/string.c: add testcases for memset16/32/64")
Signed-off-by: Peter Rosin <peda@axentia.se>
---
 lib/test_string.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/test_string.c b/lib/test_string.c
index 0fcdb82dca86..98a787e7a1fd 100644
--- a/lib/test_string.c
+++ b/lib/test_string.c
@@ -35,7 +35,7 @@ static __init int memset16_selftest(void)
 fail:
 	kfree(p);
 	if (i < 256)
-		return (i << 24) | (j << 16) | k;
+		return (i << 24) | (j << 16) | k | 0x8000;
 	return 0;
 }
 
@@ -71,7 +71,7 @@ static __init int memset32_selftest(void)
 fail:
 	kfree(p);
 	if (i < 256)
-		return (i << 24) | (j << 16) | k;
+		return (i << 24) | (j << 16) | k | 0x8000;
 	return 0;
 }
 
@@ -107,7 +107,7 @@ static __init int memset64_selftest(void)
 fail:
 	kfree(p);
 	if (i < 256)
-		return (i << 24) | (j << 16) | k;
+		return (i << 24) | (j << 16) | k | 0x8000;
 	return 0;
 }
 
-- 
2.11.0

[PATCH RESEND 3/3] lib/test_string: add some testcases for strchr and strnchr

[Peter Rosin]

Make sure that the trailing NUL is considered part of the string and can
be found.

Signed-off-by: Peter Rosin <peda@axentia.se>
---
 lib/test_string.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 77 insertions(+)

diff --git a/lib/test_string.c b/lib/test_string.c
index 98a787e7a1fd..613fd5cc9872 100644
--- a/lib/test_string.c
+++ b/lib/test_string.c
@@ -111,6 +111,73 @@ static __init int memset64_selftest(void)
 	return 0;
 }
 
+static __init int strchr_selftest(void)
+{
+	const char *test_string = "abcdefghijkl";
+	const char *empty_string = "";
+	char *result;
+	int i;
+
+	for (i = 0; i < strlen(test_string) + 1; i++) {
+		result = strchr(test_string, test_string[i]);
+		if (result - test_string != i)
+			return i + 'a';
+	}
+
+	result = strchr(empty_string, '\0');
+	if (result != empty_string)
+		return 0x101;
+
+	result = strchr(empty_string, 'a');
+	if (result)
+		return 0x102;
+
+	result = strchr(test_string, 'z');
+	if (result)
+		return 0x103;
+
+	return 0;
+}
+
+static __init int strnchr_selftest(void)
+{
+	const char *test_string = "abcdefghijkl";
+	const char *empty_string = "";
+	char *result;
+	int i, j;
+
+	for (i = 0; i < strlen(test_string) + 1; i++) {
+		for (j = 0; j < strlen(test_string) + 2; j++) {
+			result = strnchr(test_string, j, test_string[i]);
+			if (j <= i) {
+				if (!result)
+					continue;
+				return ((i + 'a') << 8) | j;
+			}
+			if (result - test_string != i)
+				return ((i + 'a') << 8) | j;
+		}
+	}
+
+	result = strnchr(empty_string, 0, '\0');
+	if (result)
+		return 0x10001;
+
+	result = strnchr(empty_string, 1, '\0');
+	if (result != empty_string)
+		return 0x10002;
+
+	result = strnchr(empty_string, 1, 'a');
+	if (result)
+		return 0x10003;
+
+	result = strnchr(NULL, 0, '\0');
+	if (result)
+		return 0x10004;
+
+	return 0;
+}
+
 static __init int string_selftest_init(void)
 {
 	int test, subtest;
@@ -130,6 +197,16 @@ static __init int string_selftest_init(void)
 	if (subtest)
 		goto fail;
 
+	test = 4;
+	subtest = strchr_selftest();
+	if (subtest)
+		goto fail;
+
+	test = 5;
+	subtest = strnchr_selftest();
+	if (subtest)
+		goto fail;
+
 	pr_info("String selftests succeeded\n");
 	return 0;
 fail:
-- 
2.11.0