Re: [PATCH v3 03/20] tools: Add gendwarfksyms

[Sami Tolvanen]

Hi Petr,

On Tue, Oct 1, 2024 at 2:04 PM Petr Pavlu <petr.pavlu@xxxxxxxx> wrote:
>
> On 9/23/24 20:18, Sami Tolvanen wrote:
> > +             if (dwfl_getmodules(dwfl, &process_module, NULL, 0)) {
> > +                     error("dwfl_getmodules failed for '%s'", argv[n]);
> > +                     return -1;
> > +             }
>
> Nit: The four error() calls don't need to be followed by 'return -1;'
> since the function now calls exit(1).

Good catch, I forgot to clean these up. I'll fix these.

> > +     for_each(name, get_symbol, &sym);
> > +     return sym;
> > +}
>
> Nit: The code inconsistently checks for a potential error from the
> function for_each(). Looking at the whole series, the value is checked
> using checkp() in functions symbol_set_crc(), symbol_set_ptr(),
> symbol_set_die(), is_exported(), but not in symbol_get() and
> elf_set_symbol_addr(). It would be good to unify this, or perhaps even
> make for_each() return an unsigned int to indicate it never fails?

True, there's no need to use check() anymore here. I'll change the
return value to unsigned int and clean up the error handling.

Sami