[DTrace-devel] [PATCH 1/2] Clean up error reporting in the BPF progran load process

[Kris Van Hees]

The return value for the attach() provider hook was being passed to
the dtrace_go() through dt_bpf_load_progs(), confusing the dtrace
command line utility which was expecting 0 or -1 (with the proper
error code being set).

It turns out return values were misused in a few other places.  The
code will now explicitly check for -1 as failure indication (and return
-1 explicitly where needed) for dt_aggregate_go(), dt_bpf_gmap_create(),
dt_bpf_load_prog(), and dt_bpf_load_progs().

Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
---
 libdtrace/dt_bpf.c  | 12 +++++-------
 libdtrace/dt_work.c | 22 ++++++++--------------
 2 files changed, 13 insertions(+), 21 deletions(-)

diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 95f51e1d..e9f2c4a4 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -1044,7 +1044,7 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
 	 */
 	for (prp = dt_list_next(&dtp->dt_enablings); prp != NULL;
 	     prp = dt_list_next(prp)) {
-		int		fd, rc;
+		int		fd;
 
 		/* Already done. */
 		if (prp == dtp->dt_error)
@@ -1059,15 +1059,13 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
 
 		fd = dt_bpf_load_prog(dtp, prp, dp, cflags);
 		if (fd < 0)
-			return fd;
+			return -1;
 
 		dt_difo_free(dtp, dp);
 
-		if (!prp->prov->impl->attach)
-			return -1;
-		rc = prp->prov->impl->attach(dtp, prp, fd);
-		if (rc < 0)
-			return rc;
+		if (!prp->prov->impl->attach ||
+		    prp->prov->impl->attach(dtp, prp, fd) < 0)
+			return dt_set_errno(dtp, EDT_ENABLING_ERR);
 	}
 
 	return 0;
diff --git a/libdtrace/dt_work.c b/libdtrace/dt_work.c
index da08d9c5..732eba80 100644
--- a/libdtrace/dt_work.c
+++ b/libdtrace/dt_work.c
@@ -53,7 +53,6 @@ int
 dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
 {
 	size_t			size;
-	int			err;
 	struct epoll_event	ev;
 	dtrace_optval_t		lockmem = dtp->dt_options[DTRACEOPT_LOCKMEM];
 	struct rlimit		rl;
@@ -69,17 +68,13 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
                 setrlimit(RLIMIT_MEMLOCK, &rl);
         }
 
-	/*
-	 * Create the global BPF maps.  This is done only once regardless of
-	 * how many programs there are.
-	 */
-	err = dt_bpf_gmap_create(dtp);
-	if (err)
-		return err;
+	/* Create the global BPF maps. */
+	if (dt_bpf_gmap_create(dtp) == -1)
+		return -1;
 
-	err = dt_bpf_load_progs(dtp, cflags);
-	if (err)
-		return err;
+	/* Load the BPF programs.  */
+	if (dt_bpf_load_progs(dtp, cflags) == -1)
+		return -1;
 
 	/*
 	 * Set up the event polling file descriptor.
@@ -116,9 +111,8 @@ dtrace_go(dtrace_hdl_t *dtp, uint_t cflags)
 	 * We must initialize the aggregation consumer handling before we
 	 * trigger the BEGIN probe.
 	 */
-	err = dt_aggregate_go(dtp);
-	if (err)
-		return err;
+	if (dt_aggregate_go(dtp) == -1)
+		return -1;
 
 	if (RUNNING_ON_VALGRIND)
 		VALGRIND_NON_SIMD_CALL0(BEGIN_probe);
-- 
2.34.1