Subject: Re: [PATCH v4 6/8] perf/tools: Enhance JSON/metric infrastructure to handle "?"

From: Jiri Olsa <[email-masked]>
Date: Thu, 12 Mar 2020 11:50:44 +0100

On Mon, Mar 09, 2020 at 11:55:50AM +0530, Kajol Jain wrote:

SNIP

> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index c3a8c701609a..11eeeb929b91 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -474,6 +474,98 @@ static bool metricgroup__has_constraint(struct pmu_event *pe)
>  	return false;
>  }
>  
> +int __weak arch_get_runtimeparam(void)
> +{
> +	return 1;
> +}
> +
> +static int metricgroup__add_metric_runtime_param(struct strbuf *events,
> +			struct list_head *group_list, struct pmu_event *pe)
> +{
> +	int i, count;
> +	int ret = -EINVAL;
> +
> +	count = arch_get_runtimeparam();
> +
> +	/* This loop is added to create multiple
> +	 * events depend on count value and add
> +	 * those events to group_list.
> +	 */
> +
> +	for (i = 0; i < count; i++) {
> +		const char **ids;
> +		int idnum;
> +		struct egroup *eg;
> +		char value[PATH_MAX];
> +
> +		expr__runtimeparam = i;

so the expr__runtimeparam is always set before we call the
expr parsing function - wither expr__find_other or expr__parse,

and it's used inside the normalize flexer function, which has
access to the passed context.. so I don't see a reason why
expr__runtimeparam couldn't be added in struct parse_ctx
and used from there..

while in this, perhaps we should rename parse_ctx to expr_ctx,
to keep the namespace straight (in separate patch)

thanks,
jirka