On Wed, 11 May 2016 12:45:00 -0300
Arnaldo Carvalho de Melo <a...@kernel.org> wrote:
> Em Wed, May 11, 2016 at 10:51:37PM +0900, Masami Hiramatsu escreveu:
> > Fix to set correct dso name ("[kernel.kallsyms]") for
> > kallsyms, as for vdso does.
>
> Can you rewrite the above comment and also break this down in two
> patches,
No, could you explain what parts this consists of?
I think this patch is done just one thing. Actually I can rewrite
this as one-line patch like below
if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
- is_vdso ? DSO__NAME_VDSO : realname) < 0)
+ is_vdso ? DSO__NAME_VDSO : (is_kallsyms ?
"[kernel.kallsyms]": realname)) < 0)
But indeed this is not readable so clean it up like below.
> probably decribing what is the problem that it fixes?
Ah, indeed. This is actually not a fix for existing bug, instead
it will prevent buggy behavior.　Current function can get any filepath
with is_vdso = true or is_kallsyms = true, but it seems easily giving
contradictory combination.
Hmm, OK, I think I have to solve this issue in another way.
Thanks!
>
> - Arnaldo
>
> > Signed-off-by: Masami Hiramatsu <mhira...@kernel.org>
> > ---
> > tools/perf/util/build-id.c | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index b6ecf87..234d8a1 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -343,21 +343,25 @@ void disable_buildid_cache(void)
> > static char *build_id_cache__dirname_from_path(const char *name,
> > bool is_kallsyms, bool is_vdso)
> > {
> > - char *realname = (char *)name, *filename;
> > + const char *realname = name;
> > + char *filename;
> > bool slash = is_kallsyms || is_vdso;
> >
> > if (!slash) {
> > realname = realpath(name, NULL);
> > if (!realname)
> > return NULL;
> > - }
> > + } else if (is_vdso)
> > + realname = DSO__NAME_VDSO;
> > + else
> > + realname = "[kernel.kallsyms]";
> >
> > if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
> > - is_vdso ? DSO__NAME_VDSO : realname) < 0)
> > + realname) < 0)
> > filename = NULL;
> >
> > if (!slash)
> > - free(realname);
> > + free((char *)realname);
> >
> > return filename;
> > }
--
Masami Hiramatsu <mhira...@kernel.org>
