Re: [PATCH] perf annotate: Fix NULL vs IS_ERR() check in annotated_source__alloc_histograms()

[Ian Rogers]

On Thu, Mar 5, 2026 at 2:26 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
>
> On Wed, Mar 4, 2026 at 6:18 PM Chen Ni <nichen@xxxxxxxxxxx> wrote:
> >
> > The hashmap__new() function never returns NULL, it returns error
> > pointers. Fix the error checking to match.
> >
> > Fixes: d3e7cad6f36d ("perf annotate: Add a hashmap for symbol histogram")
> > Signed-off-by: Chen Ni <nichen@xxxxxxxxxxx>
>
> More on the ERR_PTR is a source of bugs..
>
> Reviewed-by: Ian Rogers <irogers@xxxxxxxxxx>
>
> Thanks,
> Ian
>
> > ---
> >  tools/perf/util/annotate.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> > index 2e3522905046..64ab5c36ac02 100644
> > --- a/tools/perf/util/annotate.c
> > +++ b/tools/perf/util/annotate.c
> > @@ -44,6 +44,7 @@
> >  #include "strbuf.h"
> >  #include <regex.h>
> >  #include <linux/bitops.h>
> > +#include <linux/err.h>
> >  #include <linux/kernel.h>
> >  #include <linux/string.h>
> >  #include <linux/zalloc.h>
> > @@ -137,7 +138,7 @@ static int annotated_source__alloc_histograms(struct annotated_source *src,
> >                 return -1;
> >
> >         src->samples = hashmap__new(sym_hist_hash, sym_hist_equal, NULL);
> > -       if (src->samples == NULL)
> > +       if (IS_ERR(src->samples))

Actually, should probably assign `src->samples = NULL;` here in case a
later read tries to use the ERR_PTR.

Thanks,
Ian

> >                 zfree(&src->histograms);
> >
> >         return src->histograms ? 0 : -1;
> > --
> > 2.25.1
> >