[PATCH v1 1/5] perf trace-event: Report tracepoint format errors with NULL and errno

[Ian Rogers <irogers@google.com>]

trace_event__tp_format() encoded failure with ERR_PTR() while
trace_event__tp_format_id() returned a plain NULL when tep_find_event()
found nothing, and tp_format() itself returned NULL when
tep_parse_format() failed, as its return value was discarded. Callers
test with IS_ERR(), which NULL does not satisfy, so those two failures
were taken for success. In syscall__read_info() that leads straight to:

	if (IS_ERR(sc->tp_format)) {
		...
		return err;
	}
	if (syscall__alloc_arg_fmts(sc, sc->tp_format->format.nr_fields - 1))

which dereferences NULL when a format file fails to parse.

Mixing encoded error pointers with pointers that are compared against
NULL is what allows that to happen, so drop ERR_PTR() here and report
failures the way the rest of these paths already expect, by returning
NULL with errno set. evsel__tp_format() no longer has to translate the
error back into errno before printing it with %m, and the remaining
callers become NULL tests.

syscall__scnprintf_args() gains back its fallback of printing raw
arguments: it asked for IS_ERR(sc->tp_format), but syscall__read_info()
had already replaced the error pointer with NULL, so the branch could
never be taken.

Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/builtin-kmem.c     |  3 +-
 tools/perf/builtin-sched.c    |  5 ++--
 tools/perf/builtin-trace.c    | 10 +++----
 tools/perf/util/evsel.c       |  5 +---
 tools/perf/util/trace-event.c | 52 ++++++++++++++++++++++++++---------
 5 files changed, 47 insertions(+), 28 deletions(-)

diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index e1b2f5bc1ba8..8693c6b135ca 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -1870,8 +1870,7 @@ static bool slab_legacy_tp_is_exposed(void)
 	 * means the tool is running on an old kernel, we need to
 	 * rollback to support these legacy tracepoints.
 	 */
-	return IS_ERR(trace_event__tp_format("kmem", "kmalloc_node")) ?
-		false : true;
+	return trace_event__tp_format("kmem", "kmalloc_node");
 }
 
 static int __cmd_record(int argc, const char **argv)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index dd39a4fb6c7a..5b4092ae33ee 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -5182,8 +5182,7 @@ static bool schedstat_events_exposed(void)
 	 * Select "sched:sched_stat_wait" event to check
 	 * whether schedstat tracepoints are exposed.
 	 */
-	return IS_ERR(trace_event__tp_format("sched", "sched_stat_wait")) ?
-		false : true;
+	return trace_event__tp_format("sched", "sched_stat_wait");
 }
 
 static int __cmd_record(int argc, const char **argv)
@@ -5240,7 +5239,7 @@ static int __cmd_record(int argc, const char **argv)
 
 	rec_argv[i++] = strdup("-e");
 	waking_event = trace_event__tp_format("sched", "sched_waking");
-	if (!IS_ERR(waking_event))
+	if (waking_event)
 		rec_argv[i++] = strdup("sched:sched_waking");
 	else
 		rec_argv[i++] = strdup("sched:sched_wakeup");
diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
index 20fffc24507b..5bd62b61287e 100644
--- a/tools/perf/builtin-trace.c
+++ b/tools/perf/builtin-trace.c
@@ -2385,7 +2385,7 @@ static int syscall__read_info(struct syscall *sc, struct trace *trace)
 	snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->name);
 	sc->tp_format = trace_event__tp_format("syscalls", tp_name);
 
-	if (IS_ERR(sc->tp_format) && sc->fmt && sc->fmt->alias) {
+	if (!sc->tp_format && sc->fmt && sc->fmt->alias) {
 		snprintf(tp_name, sizeof(tp_name), "sys_enter_%s", sc->fmt->alias);
 		sc->tp_format = trace_event__tp_format("syscalls", tp_name);
 	}
@@ -2394,11 +2394,9 @@ static int syscall__read_info(struct syscall *sc, struct trace *trace)
 	 * Fails to read trace point format via sysfs node, so the trace point
 	 * doesn't exist.  Set the 'nonexistent' flag as true.
 	 */
-	if (IS_ERR(sc->tp_format)) {
+	if (!sc->tp_format) {
 		sc->nonexistent = true;
-		err = PTR_ERR(sc->tp_format);
-		sc->tp_format = NULL;
-		return err;
+		return -errno;
 	}
 
 	/*
@@ -2681,7 +2679,7 @@ static size_t syscall__scnprintf_args(struct syscall *sc, char *bf, size_t size,
 			printed += syscall_arg_fmt__scnprintf_val(&sc->arg_fmt[arg.idx],
 								  bf + printed, size - printed, &arg, val);
 		}
-	} else if (IS_ERR(sc->tp_format)) {
+	} else if (!sc->tp_format) {
 		/*
 		 * If we managed to read the tracepoint /format file, then we
 		 * may end up not having any args, like with gettid(), so only
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index c663aafa88b2..2570ea8d5d7b 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -720,10 +720,7 @@ struct tep_event *evsel__tp_format(struct evsel *evsel)
 	else
 		tp_format = trace_event__tp_format(evsel->tp_sys, evsel->tp_name);
 
-	if (IS_ERR(tp_format)) {
-		int err = -PTR_ERR(tp_format);
-
-		errno = err;
+	if (!tp_format) {
 		pr_err("Error getting tracepoint format '%s': %m\n",
 			evsel__name(evsel));
 		return NULL;
diff --git a/tools/perf/util/trace-event.c b/tools/perf/util/trace-event.c
index 000c1e1d68c1..10a7652f0305 100644
--- a/tools/perf/util/trace-event.c
+++ b/tools/perf/util/trace-event.c
@@ -7,7 +7,6 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <linux/kernel.h>
-#include <linux/err.h>
 #include <event-parse.h>
 #include <api/fs/tracing_path.h>
 #include <api/fs/fs.h>
@@ -77,7 +76,7 @@ void trace_event__cleanup(struct trace_event *t)
 }
 
 /*
- * Returns pointer with encoded error via <linux/err.h> interface.
+ * Returns NULL and sets errno on failure.
  */
 static struct tep_event*
 tp_format(const char *sys, const char *name)
@@ -90,38 +89,65 @@ tp_format(const char *sys, const char *name)
 	char *data;
 	int err;
 
-	if (!tp_dir)
-		return ERR_PTR(-errno);
+	if (!tp_dir) {
+		errno = ENOMEM;
+		return NULL;
+	}
 
 	scnprintf(path, PATH_MAX, "%s/%s/format", tp_dir, name);
 	put_events_file(tp_dir);
 
 	err = filename__read_str(path, &data, &size);
-	if (err)
-		return ERR_PTR(err);
+	if (err) {
+		errno = -err;
+		return NULL;
+	}
 
-	tep_parse_format(pevent, &event, data, size, sys);
+	err = tep_parse_format(pevent, &event, data, size, sys);
 
 	free(data);
+
+	/*
+	 * A parse failure leaves no event behind, report it rather than
+	 * letting a NULL be mistaken for a successfully parsed format.
+	 */
+	if (err != TEP_ERRNO__SUCCESS || !event) {
+		errno = EINVAL;
+		return NULL;
+	}
+
 	return event;
 }
 
 /*
- * Returns pointer with encoded error via <linux/err.h> interface.
+ * Returns NULL and sets errno on failure.
  */
 struct tep_event*
 trace_event__tp_format(const char *sys, const char *name)
 {
-	if (!tevent_initialized && trace_event__init2())
-		return ERR_PTR(-ENOMEM);
+	if (!tevent_initialized && trace_event__init2()) {
+		errno = ENOMEM;
+		return NULL;
+	}
 
 	return tp_format(sys, name);
 }
 
+/*
+ * Returns NULL and sets errno on failure.
+ */
 struct tep_event *trace_event__tp_format_id(int id)
 {
-	if (!tevent_initialized && trace_event__init2())
-		return ERR_PTR(-ENOMEM);
+	struct tep_event *event;
 
-	return tep_find_event(tevent.pevent, id);
+	if (!tevent_initialized && trace_event__init2()) {
+		errno = ENOMEM;
+		return NULL;
+	}
+
+	event = tep_find_event(tevent.pevent, id);
+	if (!event)
+		errno = ENOENT;
+
+	return event;
 }
-- 
2.55.0.1082.g2b9226bbc0-goog