On Fri, 22 May 2026 10:45:21 -0400 Steven Rostedt <[email protected]> wrote:
> 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? This is because a wrong eprobe syntax parser error position indicator. In tools/testing/selftests/ftrace/test.d/dynevent/eprobes_syntax_errors.tc: check_error 'e:foo/bar syscalls/sys_enter_openat arg=^dfd' # BAD_FETCH_ARG check_error 'e:foo/bar syscalls/sys_enter_openat ^arg=$foo' # BAD_ATTACH_ARG BAD_FETCH_ARG points the fetcharg name correctly, but the BAD_ATTACH_ARG points wrong place in the test case. I think we should fix test case. (Previously, since it was a cleanup, I didn't changed it) Thank you, -- Masami Hiramatsu (Google) <[email protected]>
