* Stephane Eranian <eran...@google.com> wrote: > > The basename() implementation varies a lot between systems. > The Linux man page says: "basename may modify the content of the path, > so it may be desirable to pass a copy when calling the function". > On some other systems, the returned address may come from an internal > buffer which can be reused in subsequent calls, thus the results should > also be copied. > > The dso__set_basename() function was not doing this causing problems > on some systems with wrong library names being shown by perf report, > such as on Android systems. > > This patch fixes the problem. > Thanks to Ben Cheng for tracking down the problem. > > Patch relative to tip.git at commit 631d5ea. > > Reported-by: Ben Cheng <bcch...@google.com> > Signed-off-by: Stephane Eranian <eran...@google.com> > ---
Just three nits: > tools/perf/util/dso.c | 29 ++++++++++++++++++++++++++++- > 1 file changed, 28 insertions(+), 1 deletion(-) > > diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c > index af4c687c..d186ace 100644 > --- a/tools/perf/util/dso.c > +++ b/tools/perf/util/dso.c > @@ -404,7 +404,34 @@ void dso__set_short_name(struct dso *dso, const char > *name) > > static void dso__set_basename(struct dso *dso) > { > - dso__set_short_name(dso, basename(dso->long_name)); > + char *lname, *base; > + > + /* > + * basename may modify path buffer, so we must pass > + * a copy. s/basename may modify path buffer /basename() may modify the path buffer > + */ > + lname = strdup(dso->long_name); > + if (!lname) > + return; > + > + /* > + * basename may return pointer to internal > + * storage which is reused in subsequent calls > + * so copy the result > + */ s/basename may return pointer /basename() may return a pointer Makes for easier reading. (Also please use consistent periods - the first comment has a period, the second one doesn't. ':' works well too:) > + base = strdup(basename(lname)); > + > + free(lname); > + > + if (!base) > + return; > + > + if (dso->sname_alloc) > + free((char *)dso->short_name); That cast is probably not needed. > + else > + dso->sname_alloc = 1; > + > + dso__set_short_name(dso, base); > } > > int dso__name_len(const struct dso *dso) Otherwise: Acked-by: Ingo Molnar <mi...@kernel.org> Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/