Re: [PATCH v4 12/19] gendwarfksyms: Add symtypes output

Sami Tolvanen
2024-10-17T17:35:44Z

Hi Petr,

On Thu, Oct 17, 2024 at 7:13 AM Petr Pavlu <petr.pavlu@suse.com> wrote:
>
> On 10/8/24 20:38, Sami Tolvanen wrote:
> > + if (symtypes_file) {
> > + symfile = fopen(symtypes_file, "w");
> > +
> > + if (!symfile) {
> > + error("fopen failed for '%s': %s", symtypes_file,
> > + strerror(errno));
> > + return -1;
>
> Nit: The 'return -1;' statement is not needed since error() doesn't
> return.

Ah, missed this one. I'll fix this in v5.

> > @@ -107,7 +108,8 @@ static void get_symbol(struct symbol *sym, void *arg)
> > {
> > struct symbol **res = arg;
> >
> > - *res = sym;
> > + if (sym->state == SYMBOL_UNPROCESSED)
> > + *res = sym;
> > }
>
> What is the reason to check that the symbol is in the unprocessed state
> here?

At this point it's mostly a sanity check to avoid extra work in case
we run into more than one DIE that matches a symbol (or its aliases).
This actually happened in earlier versions because we handled symbol
type pointers (patch 17) as soon as we found them, but now that we
just save them for later in case they're needed, this probably isn't
strictly necessary anymore. I don't see any downsides in keeping this
check though.

Sami