Hi Petr, On Thu, Oct 17, 2024 at 7:13 AM Petr Pavlu <petr.pavlu@xxxxxxxx> 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