linux-kernel - Re: [PATCH v7 1/5] perf metric: Restructure struct expr_parse

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20210115100846.GC1478666@krava>
Date:   Fri, 15 Jan 2021 11:08:46 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     Ian Rogers <irogers@...gle.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        linux-kernel@...r.kernel.org, Andi Kleen <ak@...ux.intel.com>,
        Jin Yao <yao.jin@...ux.intel.com>,
        John Garry <john.garry@...wei.com>,
        Paul Clarke <pc@...ibm.com>, kajoljain <kjain@...ux.ibm.com>,
        Stephane Eranian <eranian@...gle.com>,
        Sandeep Dasgupta <sdasgup@...gle.com>
Subject: Re: [PATCH v7 1/5] perf metric: Restructure struct expr_parse_ctx.

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