Re: [External] Re: [RFC 02/12] perf event action: Add parsing const expr support

[Yang Jihong]

Hello,

On 11/29/24 04:23, Arnaldo Carvalho de Melo wrote:
> On Thu, Nov 28, 2024 at 09:35:43PM +0800, Yang Jihong wrote:
> > Event action requires constant expression parsing support,
> > which include constant integer and constant string.
> >
> > Signed-off-by: Yang Jihong <yangjihong@xxxxxxxxxxxxx>
> > ---
> >   tools/perf/util/parse-action.c | 27 +++++++++++++++++++++++++++
> >   tools/perf/util/parse-action.h |  5 +++++
> >   2 files changed, 32 insertions(+)
> >
> > diff --git a/tools/perf/util/parse-action.c b/tools/perf/util/parse-action.c
> > index 01c8c7fdea59..391546bf3d73 100644
> > --- a/tools/perf/util/parse-action.c
> > +++ b/tools/perf/util/parse-action.c
> > @@ -4,6 +4,9 @@
> >    * Actions are the programs that run when the sampling event is triggered.
> >    * The action is a list of expressions separated by semicolons (;).
> >    * Each action is an expression, added to actions_head node as list_head node.
> > + *
> > + * Supported expressions:
> > + *   - constant:
>
> This seems incomplete, what should come after the :?
>
> the patch description, at the beginning of this message has more details
> than here.
This patch only implements a general constant expression category. The 
next patch will support parsing of specific string constant expressions 
and integer constant expressions.


> >    */
> >   
> >   #include "util/debug.h"
> > @@ -115,7 +118,31 @@ void event_actions__free(void)
> >   	(void)event_actions__for_each_expr_safe(do_action_free, NULL, false);
> >   }
> >   
> > +static struct evtact_expr_ops *expr_const_ops_list[EVTACT_EXPR_CONST_TYPE_MAX] = {
> > +};
> > +
> > +static int expr_const_set_ops(struct evtact_expr *expr, u32 opcode)
> > +{
> > +	if (opcode >= EVTACT_EXPR_CONST_TYPE_MAX) {
> > +		pr_err("expr_const opcode invalid: %u\n", opcode);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (expr_const_ops_list[opcode] == NULL) {
> > +		pr_err("expr_const opcode not supported: %u\n", opcode);
> > +		return -ENOTSUP;
> > +	}
>
> Since expr_const_ops_list[EVTACT_EXPR_TYPE_CONST] is NULL, this will
> always fail?
Yes, this patch does not support specific constant expressions, so it is 
empty.

Thanks,
Yang.