From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752181AbcELCCY (ORCPT ); Wed, 11 May 2016 22:02:24 -0400 Received: from mail.kernel.org ([198.145.29.136]:33867 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751450AbcELCCW (ORCPT ); Wed, 11 May 2016 22:02:22 -0400 Date: Thu, 12 May 2016 11:02:12 +0900 From: Masami Hiramatsu To: Arnaldo Carvalho de Melo Cc: linux-kernel@vger.kernel.org, Namhyung Kim , Peter Zijlstra , Ingo Molnar , Hemant Kumar , Ananth N Mavinakayanahalli , Brendan Gregg Subject: Re: [PATCH perf/core v7 02/21] perf buildid: Fix to set correct dso name for kallsyms Message-Id: <20160512110212.c8bb188b823e1a637e53ae26@kernel.org> In-Reply-To: <20160511154500.GF3619@kernel.org> References: <20160511135110.23943.38738.stgit@devbox> <20160511135136.23943.31500.stgit@devbox> <20160511154500.GF3619@kernel.org> X-Mailer: Sylpheed 3.4.3 (GTK+ 2.24.28; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-2022-JP Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 11 May 2016 12:45:00 -0300 Arnaldo Carvalho de Melo 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 > > --- > > 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