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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAP_z_Cg4ZjfjYuCVZeCsReJZQnHrLZKvAk_Txb1OoiNojT3L6w@mail.gmail.com>
Date:   Wed, 29 Jun 2022 14:36:03 -0700
From:   Blake Jones <blakejones@...gle.com>
To:     Jiri Olsa <olsajiri@...il.com>
Cc:     Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Namhyung Kim <namhyung@...nel.org>,
        Ian Rogers <irogers@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
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@...il.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