Re: [PATCH v7 1/5] perf metric: Restructure struct expr_parse_ctx.

[Jiri Olsa <jolsa@redhat.com>]

On Tue, Jan 12, 2021 at 03:04:30PM -0800, Ian Rogers wrote:

SNIP

> diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
> index 0ca6a5a53523..cf5afd126934 100644
> --- a/tools/perf/tests/pmu-events.c
> +++ b/tools/perf/tests/pmu-events.c
> @@ -478,9 +478,14 @@ static int test_parsing(void)
>  	struct pmu_event *pe;
>  	int i, j, k;
>  	int ret = 0;
> -	struct expr_parse_ctx ctx;
> +	struct expr_parse_ctx *ctx;
>  	double result;
>  
> +	ctx = expr__ctx_new();
> +	if (!ctx) {
> +		pr_debug("expr__ctx_new failed");
> +		return TEST_FAIL;
> +	}
>  	i = 0;
>  	for (;;) {
>  		map = &pmu_events_map[i++];
> @@ -496,8 +501,8 @@ static int test_parsing(void)
>  				break;
>  			if (!pe->metric_expr)
>  				continue;
> -			expr__ctx_init(&ctx);
> -			if (expr__find_other(pe->metric_expr, NULL, &ctx, 0)
> +			expr__ctx_clear(ctx);

why move the clear from bottom in here?


SNIP

>  
> -void expr__ctx_init(struct expr_parse_ctx *ctx)
> +struct expr_parse_ctx *expr__ctx_new(void)
>  {
> -	hashmap__init(&ctx->ids, key_hash, key_equal, NULL);
> +	struct expr_parse_ctx *ctx;
> +
> +	ctx = malloc(sizeof(struct expr_parse_ctx));
> +	if (!ctx)
> +		return NULL;
> +
> +	ctx->ids = hashmap__new(key_hash, key_equal, NULL);

we should check for allocation failuer

jirka

> +	ctx->parent = NULL;
> +	return ctx;
>  }
>  
>  void expr__ctx_clear(struct expr_parse_ctx *ctx)
> @@ -221,11 +229,24 @@ void expr__ctx_clear(struct expr_parse_ctx *ctx)
>  	struct hashmap_entry *cur;
>  	size_t bkt;
>  
> -	hashmap__for_each_entry((&ctx->ids), cur, bkt) {
> +	hashmap__for_each_entry(ctx->ids, cur, bkt) {
> +		free((char *)cur->key);
> +		free(cur->value);

SNIP