Re: [PATCH v2 RESEND] Add a "-m" option to "perf buildid-list".

[Blake Jones]

Thanks for taking a look at this!

On Wed, Jun 29, 2022 at 2:38 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
> > +static int buildid__map_cb(struct map *map, void *arg __maybe_unused)
> > +{
> > +     const struct dso *dso = map->dso;
> > +     char bid_buf[SBUILD_ID_SIZE];
> > +
> > +     memset(bid_buf, 0, sizeof(bid_buf));
> > +     if (dso->has_build_id)
> > +             build_id__sprintf(&dso->bid, bid_buf);
> > +     printf("%s %16lx %16lx", bid_buf, map->start, map->end);
> > +     if (dso->long_name != NULL)
> > +             printf(" %s", dso->long_name);
>
> nit, should we display short_name in case long_name is missing?

I can do that.

> > +int machine__for_each_kernel_map(struct machine *machine, machine__map_t fn, void *priv)
> > +{
> > +     struct maps *maps = machine__kernel_maps(machine);
> > +     struct map *map;
> > +     int err = 0;
> > +
> > +     for (map = maps__first(maps); map != NULL; map = map__next(map)) {
> > +             if (fn(map, priv))
> > +                     err = -1;
>
> I think we should rather break in here and return user's error

I'd structured it this way to be analogous to machine_for_each_dso(),
immediately above it, but I'm happy to make this change.

> other than that, the patch looks good to me

Great, thanks! Updated patch coming shortly.

Blake