Skip to contentSkip to conversation

selftests/mm: fix read_file() return value check

PATCH Applied Hongfu Li 2 participants 3 messages Aug 7 – Aug 10

Subsystems (4)
lore
Share & tools
lore

Hongfu Li

author
#0 lore raw

PATCH 1/1 selftests/mm: fix read_file() return value check

From: Hongfu Li <[email protected]>

read_file() returns 0 on open/read failures and never returns negative values. Existing < 0 error checks never trigger, so read failures are silently ignored. Check for zero return to detect read_file() failures.

Also fix misleading error message in get_finfo(). The error string incorrectly references read_num when reading uevent files.

---
 tools/testing/selftests/mm/khugepaged.c | 4 ++--
 tools/testing/selftests/mm/vm_util.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
tools/testing/selftests/mm/khugepaged.c +2 -2
index 10e8dedcb087..506310d4b4d5 100644
@@ -138,8 +138,8 @@ static void get_finfo(const char *dir)
 		     major(path_stat.st_dev), minor(path_stat.st_dev))
 	    >= sizeof(path))
 		ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
-	if (read_file(path, buf, sizeof(buf)) < 0)
-		ksft_exit_fail_perror("read_file(read_num)");
+	if (!read_file(path, buf, sizeof(buf)))
+		ksft_exit_fail_perror("read_file(uevent)");
 	if (strstr(buf, "DEVTYPE=disk")) {
 		/* Found it */
 		if (snprintf(finfo.dev_queue_read_ahead_path,
tools/testing/selftests/mm/vm_util.c +1 -1
index ef1ea11981a7..2697d50d07f4 100644
@@ -755,7 +755,7 @@ unsigned long read_num(const char *path)
 {
 	char buf[21];
 
-	if (read_file(path, buf, sizeof(buf)) < 0)
+	if (!read_file(path, buf, sizeof(buf)))
 		ksft_exit_fail_perror("read_file()");
 
 	return strtoul(buf, NULL, 10);
Signature · 1 line
2.54.0
maintainer Acked-by on v1 1/1
#1 lore raw
Quoted 35 lines · Hongfu Li

From: Hongfu Li <[email protected]>

read_file() returns 0 on open/read failures and never returns negative values. Existing < 0 error checks never trigger, so read failures are silently ignored. Check for zero return to detect read_file() failures.

Also fix misleading error message in get_finfo(). The error string incorrectly references read_num when reading uevent files.

---
 tools/testing/selftests/mm/khugepaged.c | 4 ++--
 tools/testing/selftests/mm/vm_util.c    | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
tools/testing/selftests/mm/khugepaged.c +2 -2
index 10e8dedcb087..506310d4b4d5 100644
@@ -138,8 +138,8 @@ static void get_finfo(const char *dir)
 		     major(path_stat.st_dev), minor(path_stat.st_dev))
 	    >= sizeof(path))
 		ksft_exit_fail_msg("%s: Pathname is too long\n", __func__);
-	if (read_file(path, buf, sizeof(buf)) < 0)
-		ksft_exit_fail_perror("read_file(read_num)");
+	if (!read_file(path, buf, sizeof(buf)))
+		ksft_exit_fail_perror("read_file(uevent)");
 	if (strstr(buf, "DEVTYPE=disk")) {
 		/* Found it */
 		if (snprintf(finfo.dev_queue_read_ahead_path,
tools/testing/selftests/mm/vm_util.c +0 -0
index ef1ea11981a7..2697d50d07f4 100644

tools/testing/selftests/mm/vm_util.c unsigned long read_num(const char *path) · line 761

Quoted 8 lines
tools/testing/selftests/mm/vm_util.c +1 -1
@@ -755,7 +755,7 @@ unsigned long read_num(const char *path)
 {
 	char buf[21];
 
-	if (read_file(path, buf, sizeof(buf)) < 0)
+	if (!read_file(path, buf, sizeof(buf)))
 		ksft_exit_fail_perror("read_file()");
 
 	return strtoul(buf, NULL, 10);

Yeah, that looks correct. Do we have Fixes: tags?

I think it was already wrong in read_num() in

commit e0c13f9761df8f97cf5e81495d12ecbc4075684a

Author: Kiryl Shutsemau <[email protected]>
Date:   Wed Jun 3 16:00:06 2020 -0700

khugepaged: add self test

where we had

+ ret = read_file(path, buf, sizeof(buf));
+ if (ret < 0) {
+ perror("read_file(read_num)");
+ exit(EXIT_FAILURE);
+ }

So likely

Fixes e0c13f9761df ("khugepaged: add self test")
Signature · 3 lines
Cheers,

David
author
#2 lore raw
Quoted 36 lines

tools/testing/selftests/mm/vm_util.c unsigned long read_num(const char *path) · line 761

Quoted 13 lines
tools/testing/selftests/mm/vm_util.c +1 -1
index ef1ea11981a7..2697d50d07f4 100644
@@ -755,7 +755,7 @@ unsigned long read_num(const char *path)
 {
 	char buf[21];
 
-	if (read_file(path, buf, sizeof(buf)) < 0)
+	if (!read_file(path, buf, sizeof(buf)))
 		ksft_exit_fail_perror("read_file()");
 
 	return strtoul(buf, NULL, 10);

Yeah, that looks correct. Do we have Fixes: tags?

I think it was already wrong in read_num() in

commit e0c13f9761df8f97cf5e81495d12ecbc4075684a

Author: Kiryl Shutsemau <[email protected]>
Date:   Wed Jun 3 16:00:06 2020 -0700

khugepaged: add self test

where we had

(quoted diff) +5 -0
+       ret = read_file(path, buf, sizeof(buf));
+       if (ret < 0) {
+               perror("read_file(read_num)");
+               exit(EXIT_FAILURE);
+       }
Quoted 5 lines

So likely

Fixes e0c13f9761df ("khugepaged: add self test")

Thank you for the review and tracking down the original commit. I will include the Fixes tag you mentioned in the next patch revision.

Best regards,
Hongfu