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

[Masami Hiramatsu (Google)]

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.

Thanks,

> 
> -- Stevwe
> 
> 
> 
> > 
> > Fixes: 80765597bc58 ("tracing: Rewrite filter logic to be simpler and faster")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> > ---
> >  kernel/trace/trace_events_filter.c |    1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> > index 3bd0b47a627f..bcd37fd6c109 100644
> > --- a/kernel/trace/trace_events_filter.c
> > +++ b/kernel/trace/trace_events_filter.c
> > @@ -2208,6 +2208,7 @@ static int calc_stack(const char *str, int *parens, int *preds, int *err)
> >  		case '&':
> >  			if (str[i+1] != str[i])
> >  				break;
> > +			i++;
> >  			is_pred = false;
> >  			continue;
> >  		case '(':
> 
>

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