[PATCH] perf strlist: Preserve entries without a trailing newline

[Yibiao Li]

From: Yibiao Li <biaobiao2233@xxxxxxxxx>

strlist__load() unconditionally replaces the last character read by
fgets() with a NUL byte. If the final line has no trailing newline,
this removes part of the entry instead. For example, a file containing
just "python3" is loaded as "python", so perf report --comms file://...
silently excludes samples from the intended command. The same loader
is used for file-based DSO and symbol filters.

Only remove the final character when it is a newline. Add regression
tests for empty files, blank lines, single-character entries, and
single- and multi-line lists with and without a trailing newline.

Fixes: 25903407da21 ("perf report: Add --dsos parameter")
Assisted-by: LLM
Signed-off-by: Yibiao Li <biaobiao2233@xxxxxxxxx>
---
Tested on aarch64 on a Singapore development server:
- Built perf with optional libelf, libtraceevent, Python, NUMA and BPF
  support disabled; no new compiler warnings.
- The new test fails on the original loader at the no-trailing-newline
  case and passes with the fix. All three util subtests pass.
- Recorded 12 cpu-clock:u samples from a short Python workload. With
  the original loader, --comms file://... reads an unterminated python3
  as python and displays no sample rows. With the fix, filter files
  with and without a trailing newline produce identical sample rows.
- The source diff passes checkpatch.pl --strict --no-signoff. The
  formatted mail also passes with UNKNOWN_COMMIT_ID ignored because
  the checkout is shallow; the Fixes commit and its original diff were
  independently verified against torvalds/linux.

AI assistance was used to inspect the loader, develop the fix and
regression tests, run validation, and prepare this message.

 tools/perf/tests/util.c   | 58 +++++++++++++++++++++++++++++++++++++++
 tools/perf/util/strlist.c |  3 +-
 2 files changed, 60 insertions(+), 1 deletion(-)

diff --git a/tools/perf/tests/util.c b/tools/perf/tests/util.c
index f9abd9911..8345df0b5 100644
--- a/tools/perf/tests/util.c
+++ b/tools/perf/tests/util.c
@@ -2,10 +2,13 @@
 #include "tests.h"
 #include "util/blake2s.h"
 #include "util/debug.h"
+#include "util/strlist.h"

 #include <linux/compiler.h>
 #include <stdlib.h>
+#include <string.h>
 #include <string2.h>
+#include <unistd.h>

 static int test_strreplace(char needle, const char *haystack,
 			   const char *replace, const char *expected)
@@ -103,8 +106,63 @@ static int test__strreplace(struct test_suite *t
__maybe_unused, int subtest __m
 	return 0;
 }

+static int test_strlist_load(const char *contents, const char *first,
+			     const char *second)
+{
+	char path[] = "/tmp/perf-test-strlist-XXXXXX";
+	struct strlist *list = NULL;
+	unsigned int nr_entries = !!first + !!second;
+	ssize_t len = strlen(contents);
+	int fd, ret = TEST_FAIL;
+
+	fd = mkstemp(path);
+	if (fd < 0) {
+		pr_debug("mkstemp failed\n");
+		return TEST_FAIL;
+	}
+	if (write(fd, contents, len) != len) {
+		pr_debug("Failed to write string list\n");
+		goto out;
+	}
+
+	list = strlist__new(NULL, NULL);
+	if (!list || strlist__load(list, path)) {
+		pr_debug("Failed to load string list\n");
+		goto out;
+	}
+	if (strlist__nr_entries(list) != nr_entries ||
+	    (first && !strlist__has_entry(list, first)) ||
+	    (second && !strlist__has_entry(list, second))) {
+		pr_debug("Unexpected string list entries for '%s'\n", contents);
+		goto out;
+	}
+
+	ret = TEST_OK;
+out:
+	strlist__delete(list);
+	close(fd);
+	unlink(path);
+	return ret;
+}
+
+static int test__strlist_load(struct test_suite *t __maybe_unused,
+			      int subtest __maybe_unused)
+{
+	TEST_ASSERT_VAL("empty file", !test_strlist_load("", NULL, NULL));
+	TEST_ASSERT_VAL("blank line", !test_strlist_load("\n", "", NULL));
+	TEST_ASSERT_VAL("trailing newline", !test_strlist_load("nginx\n",
"nginx", NULL));
+	TEST_ASSERT_VAL("no trailing newline", !test_strlist_load("nginx",
"nginx", NULL));
+	TEST_ASSERT_VAL("single character", !test_strlist_load("x", "x", NULL));
+	TEST_ASSERT_VAL("multiple lines",
+			!test_strlist_load("sshd\nnginx\n", "sshd", "nginx"));
+	TEST_ASSERT_VAL("unterminated final line",
+			!test_strlist_load("sshd\nnginx", "sshd", "nginx"));
+	return TEST_OK;
+}
+
 static struct test_case tests__util[] = {
 	TEST_CASE("String replacement", strreplace),
+	TEST_CASE("Load string lists from files", strlist_load),
 	TEST_CASE("BLAKE2s hash", blake2s_case),
 	{ .name = NULL, }
 };
diff --git a/tools/perf/util/strlist.c b/tools/perf/util/strlist.c
index 50add7257..7cbc6dcb0 100644
--- a/tools/perf/util/strlist.c
+++ b/tools/perf/util/strlist.c
@@ -73,7 +73,8 @@ int strlist__load(struct strlist *slist, const char *filename)

 		if (len == 0)
 			continue;
-		entry[len - 1] = '\0';
+		if (entry[len - 1] == '\n')
+			entry[len - 1] = '\0';

 		err = strlist__add(slist, entry);
 		if (err != 0)

base-commit: 70dd4ee72eb2d5793c62e71c14e3aba78e5a4e56
-- 
2.43.0