Re: [PATCH v2 6/9] tracing/filters: Skip second character of '&&' and '||' in calc_stack()

[Masami Hiramatsu (Google)]

On Tue, 27 Oct 2026 08:50:42 +0900, Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:
> On Mon, 26 Oct 2026 16:47:27 -0400, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> > On Sun, 25 Oct 2026 17:38:05 +0900
> > "Masami Hiramatsu (Google)" <mhiramat@xxxxxxxxxx> wrote:
> > 
> > > From: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > > 
> > > In calc_stack(), when encountering '||' or '&&' (where str[i+1] == str[i]),
> > > the loop sets 'is_pred = false' and continues without advancing 'i' past
> > > the second character of the operator. On the next iteration, the second '|'
> > > or '&' sees str[i+1] != str[i], breaks out of the switch statement, and is
> > > counted as the start of a new predicate (incrementing nr_preds and setting
> > > is_pred = true).
> > > 
> > > Consequently, a trailing logical operator (e.g. 'common_pid == 1 ||') is
> > > counted as having 2 predicates instead of 1, and any subsequent non-space
> > > character before a delimiter fails to increment nr_preds.
> > 
> >  "common_pid ||' is an invalid expression.
> > 
> > > 
> > > Increment 'i' when matching '||' or '&&' so the entire two-character
> > > operator is skipped.
> > 
> > This is just wrong.
> > 
> > Masami, don't blindly trust AI.
> > 
> 
> Oops, I might mislead by the code. (I thought I've checked it...)
> 
> > I purposely did it this way. Because && and || creates a new predicate.
> > 
> >  If you have "x||y"
> >               ^
> >               i
> 
> OK,
> 
> > 
> >  Start with i = 0; is_pred = false; nr_preds = 0;
> > 
> >    str[i] == 'x'
> >    if (!is_pred) {
> > 	nr_preds++;  // nr_preds = 1
> >         is_pred = true;
> >    }
> > 
> >  in for loop i++;  // i = 1; str[i] = '|'
> > 
> >    switch (str[i])
> >    case '|':
> >       if (str[i+1] != str[i]) // false
> 
> Ah, somehow I completely misread this is if (str[i+1] == str[i]) ....
> 
> >       is_pred = false;
> >       continue;
> > 
> >  in for loop i++; // i = 2; str[i] == '|';
> > 
> >    switch (str[i])
> >    case '|':
> >       if (str[i+1] != str[i]) // true
> >           break;
> > 
> >    if (!is_pred) {
> > 	nr_preds++;  // nr_preds = 2
> >         is_pred = true;
> >    }
> > 
> >  in for loop i++; // i = 3; str[i] == 'y';
> >  
> >    if (!is_pred) { // false because is_pred == true
> > 
> > Loop ends.
> > 
> > nr_preds == 2. This is correct.
> > 
> > This patch is garbage.
> 
> OK, let me drop it.

BTW, to check the correct input is encoded correctly, we may need
in kernel selftest or a feature to dump the interpreted pred after
"#" line via filter file. Latter one allows us to test it via tracefs
(ftracetest).

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>