Messages in this thread | | | From | Blake Jones <> | Date | Wed, 29 Jun 2022 14:36:03 -0700 | Subject | Re: [PATCH v2 RESEND] Add a "-m" option to "perf buildid-list". |
| |
Thanks for taking a look at this!
On Wed, Jun 29, 2022 at 2:38 AM Jiri Olsa <olsajiri@gmail.com> 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
| |