On Fri, 22 May 2026 07:23:22 -0400
Steven Rostedt <[email protected]> wrote:
> > > @@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
> > > return -EOPNOTSUPP;
> > > }
> > >
> > > + if (ctx->flags & TPARG_FL_TEVENT) {
> > > + int ret;
> > > +
> > > + ret = parse_trace_event(varname, code, ctx);
> > > + if (ret < 0)
> > > + return ret;
>
> > When parse_trace_event() returns a negative error code (such as -EINVAL or
> > -ENOENT) because a field name is invalid, the error is propagated back up
> > the stack. Does this path miss calling trace_probe_log_err()?
> > If so, users might receive a generic failure without context or a caret
> > pointing to the specific syntax error.
>
> Hmm, there's a comment in the parse_trace_event() that sets ctx->offset for
> backward compatibility. I'll investigate to see if we can fix that now.
Masami,
I looked at the code for parse_trace_event() that has:
/* backward compatibility */
ctx->offset = 0;
return -EINVAL;
And it was originally introduced by commit 1b8b0cd754cd ("tracing/probes:
Move event parameter fetching code to common parser"), with:
+ ret = parse_trace_event_arg(arg, code, ctx);
+ if (!ret)
+ return 0;
+ if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
+ code->op = FETCH_OP_COMM;
+ return 0;
+ }
+ /* backward compatibility */
+ ctx->offset = 0;
+ goto inval;
+ }
+
What was the reason for the "backward compatibility"? Can we make it a real
error now?
-- Steve
