Re: [PATCH v3 3/6] perf symbols: Factor out duplicate symbol selection

From: Ian Rogers

Date: Fri Sep 25 2026 - 16:22:12 EST


On Fri, Sep 25, 2026 at 12:55 PM Alireza Haghdoost <haghdoost@xxxxxxxx> wrote:
>
> > > -int arch__choose_best_symbol(struct symbol *syma, struct symbol *symb);
> > > +int arch__choose_best_symbol(const struct symbol_candidate *a,
> > > + const struct symbol_candidate *b);
> >
> > Given you can change this function definition and only change 1 C
> > file, is there any reason not to make the function static in symbol.c
> > and move the struct symbol_candidate definition also to symbol.c?
> >
>
> Thanks for the review. Patch 5 calls symbol__choose_best() from
> symbol-elf.c: the lazy index resolves same-address duplicates from its
> index entries, not from struct symbols, and uses the same policy as
> the eager loader. That's why patch 3 makes it non-static.
>
> struct symbol_candidate has to stay in symbol.h as well, because
> arch__choose_best_symbol() takes it and powerpc overrides that weak
> function in arch/powerpc/util/sym-handling.c.
>
> I can say this in the patch 3 commit message in v4.

Thanks, I saw this when I got to the later patches, sorry for the noise.

Ian

> Thanks,
> Alireza