Re: [RFC 1/3] perf tool: Introduce arch-specific supplemental perf open strerror capability

From: Kim Phillips
Date: Tue Oct 24 2017 - 21:23:14 EST


On Tue, 24 Oct 2017 12:27:44 -0200
Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:

> Em Tue, Oct 24, 2017 at 03:04:04AM -0500, Kim Phillips escreveu:
> > Introduce new tools/perf/arch/*/util/evsel.c:perf_evsel__suppl_strerror()
>
> The naming... suppl for what? for the open operation? strerror() methods
> are associated with another method, one for which it can use context
> like who is the user asking for that operation, procfs/sysfs knobs,
> hardware, etc, so we need to have CLASS__METHOD_strerror()
>
> In this case, CLASS == perf_evsel, METHOD = open, so, if you want to
> have something that is _arch_ specific, we could use something like:
>
> int __weak perf_evsel__open_strerror_arch(struct perf_evsel *evsel __maybe_unused,
> struct target *target __maybe_unused,
> int err __maybe_unused,
> char *msg __maybe_unused,
> size_t size __maybe_unused)
> {
> return 0;
> }
>
> But then you're calling it _before_ the existing switch (err), humm...
> I.e. it will add stuff before the string that will be formatted later...
> The messages may end up being conflicting, I wonder if the model
> wouldn't be better as: ask the arch specific if it wants to override
> that specific error with something, if not, fallback to the existing
> perf_evsel__open_strerror() code, that could be moved to be
> __perf_evsel__open_strerror() and called by the arch specific code.
>
> Thoughts?

Thanks for your good feedback, yes, very good idea, fully agreed and
implemented - see below.

Thanks,

Kim