* [PATCH 1/2] error: report probe name on failed enabling error
@ 2025-02-24 18:43 Kris Van Hees
2025-03-17 19:57 ` Eugene Loh
0 siblings, 1 reply; 3+ messages in thread
From: Kris Van Hees @ 2025-02-24 18:43 UTC (permalink / raw)
To: dtrace, dtrace-devel
Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
---
libdtrace/dt_bpf.c | 35 +++++++++++++++++++++++------------
libdtrace/dt_bpf.h | 2 ++
libdtrace/dt_error.c | 2 ++
libdtrace/dt_prov_uprobe.c | 15 +++------------
4 files changed, 30 insertions(+), 24 deletions(-)
diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
index 662fd81a..e50bb536 100644
--- a/libdtrace/dt_bpf.c
+++ b/libdtrace/dt_bpf.c
@@ -64,6 +64,25 @@ dt_bpf_error(dtrace_hdl_t *dtp, const char *fmt, ...)
return dt_set_errno(dtp, EDT_BPF);
}
+int
+dt_attach_error(dtrace_hdl_t *dtp, int rc, ...)
+{
+ va_list ap, apc;
+ char *fmt;
+
+ if (asprintf(&fmt, "Failed to enable %%s:%%s:%%s:%%s: %s",
+ dtrace_errmsg(dtp, -rc)) > 0) {
+ va_start(ap, rc);
+ va_copy(apc, ap);
+ dt_set_errmsg(dtp, NULL, NULL, NULL, 0, fmt, ap);
+ va_end(ap);
+ dt_debug_printf("bpf", "Failed to enable %s:%s:%s:%s", apc);
+ va_end(apc);
+ }
+
+ return dt_set_errno(dtp, EDT_ENABLING_ERR);
+}
+
int
dt_bpf_lockmem_error(dtrace_hdl_t *dtp, const char *msg)
{
@@ -1335,19 +1354,11 @@ dt_bpf_load_progs(dtrace_hdl_t *dtp, uint_t cflags)
if (prp->prov->impl->attach)
rc = prp->prov->impl->attach(dtp, prp, fd);
- if (rc == -ENOTSUPP) {
- char *s;
-
- close(fd);
- if (asprintf(&s, "Failed to enable %s:%s:%s:%s",
- prp->desc->prv, prp->desc->mod,
- prp->desc->fun, prp->desc->prb) == -1)
- return dt_set_errno(dtp, EDT_ENABLING_ERR);
- dt_handle_rawerr(dtp, s);
- free(s);
- } else if (rc < 0) {
+ if (rc < 0) {
close(fd);
- return dt_set_errno(dtp, EDT_ENABLING_ERR);
+ return dt_attach_error(dtp, rc,
+ prp->desc->prv, prp->desc->mod,
+ prp->desc->fun, prp->desc->prb);
}
}
diff --git a/libdtrace/dt_bpf.h b/libdtrace/dt_bpf.h
index 85934d2d..464f0189 100644
--- a/libdtrace/dt_bpf.h
+++ b/libdtrace/dt_bpf.h
@@ -67,6 +67,8 @@ extern int dt_perf_event_open(struct perf_event_attr *attr, pid_t pid, int cpu,
int group_fd, unsigned long flags);
extern int dt_bpf(enum bpf_cmd cmd, union bpf_attr *attr);
+extern int dt_attach_error(struct dtrace_hdl *, int, ...);
+
extern int dt_bpf_gmap_create(struct dtrace_hdl *);
extern int dt_bpf_lockmem_error(struct dtrace_hdl *dtp, const char *msg);
diff --git a/libdtrace/dt_error.c b/libdtrace/dt_error.c
index 213f0d9e..6721e8e4 100644
--- a/libdtrace/dt_error.c
+++ b/libdtrace/dt_error.c
@@ -111,6 +111,8 @@ dtrace_errmsg(dtrace_hdl_t *dtp, int error)
if (error == EDT_COMPILER && dtp != NULL && dtp->dt_errmsg[0] != '\0')
str = dtp->dt_errmsg;
+ if (error == EDT_ENABLING_ERR && dtp != NULL && dtp->dt_errmsg[0] != '\0')
+ str = dtp->dt_errmsg;
else if (error == EDT_BPF && dtp != NULL && dtp->dt_errmsg[0] != '\0')
str = dtp->dt_errmsg;
else if (error == EDT_CTF && dtp != NULL && dtp->dt_ctferr != 0)
diff --git a/libdtrace/dt_prov_uprobe.c b/libdtrace/dt_prov_uprobe.c
index 17f2f7ee..96a59aef 100644
--- a/libdtrace/dt_prov_uprobe.c
+++ b/libdtrace/dt_prov_uprobe.c
@@ -385,19 +385,10 @@ static int add_probe_uprobe(dtrace_hdl_t *dtp, dt_probe_t *prp)
if (prp->prov->impl->attach)
rc = prp->prov->impl->attach(dtp, prp, fd);
- if (rc == -ENOTSUPP) {
- char *s;
-
- close(fd);
- if (asprintf(&s, "Failed to enable %s:%s:%s:%s",
- prp->desc->prv, prp->desc->mod,
- prp->desc->fun, prp->desc->prb) == -1)
- return dt_set_errno(dtp, EDT_ENABLING_ERR);
- dt_handle_rawerr(dtp, s);
- free(s);
- } else if (rc < 0) {
+ if (rc < 0) {
close(fd);
- return dt_set_errno(dtp, EDT_ENABLING_ERR);
+ return dt_attach_error(dtp, rc, prp->desc->prv, prp->desc->mod,
+ prp->desc->fun, prp->desc->prb);
}
return 0;
--
2.43.5
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] error: report probe name on failed enabling error
2025-02-24 18:43 [PATCH 1/2] error: report probe name on failed enabling error Kris Van Hees
@ 2025-03-17 19:57 ` Eugene Loh
2025-03-17 20:03 ` Kris Van Hees
0 siblings, 1 reply; 3+ messages in thread
From: Eugene Loh @ 2025-03-17 19:57 UTC (permalink / raw)
To: Kris Van Hees, dtrace, dtrace-devel
I don't 100% get our error reporting, but
Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
A few things...
The various files need updated copyright years.
On 2/24/25 13:43, Kris Van Hees wrote:
I don't know if anyone else sees this, but this is the second patch in a
few days that has gotten buried in my inbox due to a stale date. (On
Friday, I got one dated 1/24.) Anyhow, I suppose I now know to look for
such emails.
> diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> +int
> +dt_attach_error(dtrace_hdl_t *dtp, int rc, ...)
> +{
> + va_list ap, apc;
> + char *fmt;
If you asprintf(&fmt), do you want a matching free() to prevent a memory
leak? (Clearly not a big deal, but...)
and finally...
> diff --git a/libdtrace/dt_error.c b/libdtrace/dt_error.c
> @@ -111,6 +111,8 @@ dtrace_errmsg(dtrace_hdl_t *dtp, int error)
>
> if (error == EDT_COMPILER && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> str = dtp->dt_errmsg;
> + if (error == EDT_ENABLING_ERR && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> + str = dtp->dt_errmsg;
> else if (error == EDT_BPF && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> str = dtp->dt_errmsg;
> else if (error == EDT_CTF && dtp != NULL && dtp->dt_ctferr != 0)
Should that be "else if"? Otherwise, if error==EDT_COMPILER, you
trigger both that clause and the later "else" clause.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] error: report probe name on failed enabling error
2025-03-17 19:57 ` Eugene Loh
@ 2025-03-17 20:03 ` Kris Van Hees
0 siblings, 0 replies; 3+ messages in thread
From: Kris Van Hees @ 2025-03-17 20:03 UTC (permalink / raw)
To: Eugene Loh; +Cc: Kris Van Hees, dtrace, dtrace-devel
On Mon, Mar 17, 2025 at 03:57:18PM -0400, Eugene Loh wrote:
> I don't 100% get our error reporting, but
> Reviewed-by: Eugene Loh <eugene.loh@oracle.com>
>
> A few things...
>
> The various files need updated copyright years.
Good point.
> On 2/24/25 13:43, Kris Van Hees wrote:
>
> I don't know if anyone else sees this, but this is the second patch in a few
> days that has gotten buried in my inbox due to a stale date. (On Friday, I
> got one dated 1/24.) Anyhow, I suppose I now know to look for such emails.
My fault - due to how I was sending the patches, it took the date stamp on the
actual patch and this one was written end of last month, but never sent because
I was still working on some other things and it wasn't needed yet.
Will avoid that in the future.
> > diff --git a/libdtrace/dt_bpf.c b/libdtrace/dt_bpf.c
> > +int
> > +dt_attach_error(dtrace_hdl_t *dtp, int rc, ...)
> > +{
> > + va_list ap, apc;
> > + char *fmt;
>
> If you asprintf(&fmt), do you want a matching free() to prevent a memory
> leak? (Clearly not a big deal, but...)
> and finally...
Good catch - fixed.
> > diff --git a/libdtrace/dt_error.c b/libdtrace/dt_error.c
> > @@ -111,6 +111,8 @@ dtrace_errmsg(dtrace_hdl_t *dtp, int error)
> > if (error == EDT_COMPILER && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> > str = dtp->dt_errmsg;
> > + if (error == EDT_ENABLING_ERR && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> > + str = dtp->dt_errmsg;
> > else if (error == EDT_BPF && dtp != NULL && dtp->dt_errmsg[0] != '\0')
> > str = dtp->dt_errmsg;
> > else if (error == EDT_CTF && dtp != NULL && dtp->dt_ctferr != 0)
>
> Should that be "else if"? Otherwise, if error==EDT_COMPILER, you trigger
> both that clause and the later "else" clause.
Absolutely - fixed.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-03-17 20:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-24 18:43 [PATCH 1/2] error: report probe name on failed enabling error Kris Van Hees
2025-03-17 19:57 ` Eugene Loh
2025-03-17 20:03 ` Kris Van Hees
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.