All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Miaoqian Lin <linmq006@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>, Andi Kleen <ak@linux.intel.com>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] perf expr: Fix missing check for return value of hashmap__new
Date: Mon, 13 Dec 2021 10:06:02 -0300	[thread overview]
Message-ID: <YbdFOvHedDCVZ04A@kernel.org> (raw)
In-Reply-To: <20211212062504.25841-1-linmq006@gmail.com>

Em Sun, Dec 12, 2021 at 06:25:02AM +0000, Miaoqian Lin escreveu:
> The hashmap__new() function may return ERR_PTR(-ENOMEM) when malloc
> fails, add IS_ERR checking for ctx->ids.
> 
> Signed-off-by: Miaoqian Lin <linmq006@gmail.com>

Thanks, applied.

As a follow up you may consider using ids__new() instead:

struct hashmap *ids__new(void)
{
        return hashmap__new(key_hash, key_equal, NULL);
}

And I noticed that the users of ids__new() are not using IS_ERR() on its
return in tools/perf/tests/expr.c, e.g.:

static int test_ids_union(void)
{
        struct hashmap *ids1, *ids2;

        /* Empty union. */
        ids1 = ids__new();
        TEST_ASSERT_VAL("ids__new", ids1);
        ids2 = ids__new();
        TEST_ASSERT_VAL("ids__new", ids2);

This needs converting to TEST_ASSERT_VAL_IS_ERR() probably after
introducing it, I haven't checked if this variation exists.

- Arnaldo

> ---
>  tools/perf/util/expr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/tools/perf/util/expr.c b/tools/perf/util/expr.c
> index 1d532b9fed29..c94c9ea30d1a 100644
> --- a/tools/perf/util/expr.c
> +++ b/tools/perf/util/expr.c
> @@ -299,6 +299,10 @@ struct expr_parse_ctx *expr__ctx_new(void)
>  		return NULL;
>  
>  	ctx->ids = hashmap__new(key_hash, key_equal, NULL);
> +	if (IS_ERR(ctx->ids)) {
> +		kfree(ctx);
> +		return NULL;
> +	}
>  	ctx->runtime = 0;
>  
>  	return ctx;
> -- 
> 2.17.1

-- 

- Arnaldo

  reply	other threads:[~2021-12-13 13:06 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-12  6:25 [PATCH] perf expr: Fix missing check for return value of hashmap__new Miaoqian Lin
2021-12-13 13:06 ` Arnaldo Carvalho de Melo [this message]
2021-12-13 13:08   ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YbdFOvHedDCVZ04A@kernel.org \
    --to=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linmq006@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.