[perf/core PATCH 0/4] perf-buildid-cache: Cleanup, enhance --update and add --remove-all

[perf/core PATCH 0/4] perf-buildid-cache: Cleanup, enhance --update and add --remove-all

[Masami Hiramatsu]

Hi,

Here is a sereis of patches for perf buildid-cache update.
I'm moving ahead to integrated perf-cache, but before that
current buildid-cache code should be cleaned up and change
a bit.

The first 2 patches are cleanup code, just reducing the
redundant code. The 3rd one modifies --update option not
to fail but just add new binary if there is no cache entry
which has same build-id of given binary. The 4th one adds
--remove-all option which allows us to cleanup all caches
which related to given path.


Thank you,


---

Masami Hiramatsu (4):
      perf-buildid-cache: Remove unneeded debugdir parameters
      perf buildid-cache: Consolidate .build-id cache path generators
      perf buildid-cache: Add new buildid cache if update target is not cached
      perf buildid-cache: Add --remove-all FILE to remove all caches of FILE


 tools/perf/Documentation/perf-buildid-cache.txt |    8 +
 tools/perf/builtin-buildid-cache.c              |   85 ++++++++--
 tools/perf/util/build-id.c                      |  187 ++++++++++++++++-------
 tools/perf/util/build-id.h                      |    6 -
 4 files changed, 208 insertions(+), 78 deletions(-)

--
Masami HIRAMATSU
Software Platform Research Dpt. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

[perf/core PATCH 1/4] perf-buildid-cache: Remove unneeded debugdir parameters

[Masami Hiramatsu]

Functions related to buildid-cache subcommand use debugdir
parameters for passing buildid cache directory path. However
all callers just pass buildid_dir global variable. Moreover,
other functions which refer buildid cache use buildid_dir
directly.
This removes unneeded debugdir parameters from those functions
and use buildid_dir if needed.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-buildid-cache.c |   37 +++++++++++++-----------------
 tools/perf/util/build-id.c         |   44 ++++++++++++++++--------------------
 tools/perf/util/build-id.h         |    4 ++-
 3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 50e6b66..d929d95 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -125,8 +125,7 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 	return ret;
 }
 
-static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
-				     bool force)
+static int build_id_cache__add_kcore(const char *filename, bool force)
 {
 	char dir[32], sbuildid[BUILD_ID_SIZE * 2 + 1];
 	char from_dir[PATH_MAX], to_dir[PATH_MAX];
@@ -143,7 +142,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s",
-		  debugdir, sbuildid);
+		  buildid_dir, sbuildid);
 
 	if (!force &&
 	    !build_id_cache__kcore_existing(from_dir, to_dir, sizeof(to_dir))) {
@@ -155,7 +154,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s/%s",
-		  debugdir, sbuildid, dir);
+		  buildid_dir, sbuildid, dir);
 
 	if (mkdir_p(to_dir, 0755))
 		return -1;
@@ -183,7 +182,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 	return 0;
 }
 
-static int build_id_cache__add_file(const char *filename, const char *debugdir)
+static int build_id_cache__add_file(const char *filename)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 	u8 build_id[BUILD_ID_SIZE];
@@ -195,7 +194,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__add_s(sbuild_id, debugdir, filename,
+	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
 	if (verbose)
 		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
@@ -203,8 +202,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	return err;
 }
 
-static int build_id_cache__remove_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__remove_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -217,7 +215,7 @@ static int build_id_cache__remove_file(const char *filename,
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id, debugdir);
+	err = build_id_cache__remove_s(sbuild_id);
 	if (verbose)
 		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
 			err ? "FAIL" : "Ok");
@@ -252,8 +250,7 @@ static int build_id_cache__fprintf_missing(struct perf_session *session, FILE *f
 	return 0;
 }
 
-static int build_id_cache__update_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__update_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -266,11 +263,10 @@ static int build_id_cache__update_file(const char *filename,
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id, debugdir);
-	if (!err) {
-		err = build_id_cache__add_s(sbuild_id, debugdir, filename,
-					    false, false);
-	}
+	err = build_id_cache__remove_s(sbuild_id);
+	if (!err)
+		err = build_id_cache__add_s(sbuild_id, filename, false, false);
+
 	if (verbose)
 		pr_info("Updating %s %s: %s\n", sbuild_id, filename,
 			err ? "FAIL" : "Ok");
@@ -338,7 +334,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, add_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__add_file(pos->s, buildid_dir)) {
+				if (build_id_cache__add_file(pos->s)) {
 					if (errno == EEXIST) {
 						pr_debug("%s already in the cache\n",
 							 pos->s);
@@ -356,7 +352,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, remove_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__remove_file(pos->s, buildid_dir)) {
+				if (build_id_cache__remove_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -377,7 +373,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, update_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__update_file(pos->s, buildid_dir)) {
+				if (build_id_cache__update_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -391,8 +387,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		}
 	}
 
-	if (kcore_filename &&
-	    build_id_cache__add_kcore(kcore_filename, buildid_dir, force))
+	if (kcore_filename && build_id_cache__add_kcore(kcore_filename, force))
 		pr_warning("Couldn't add %s\n", kcore_filename);
 
 out:
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 0c72680..9f764f6 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -259,8 +259,8 @@ void disable_buildid_cache(void)
 	no_buildid_cache = true;
 }
 
-int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
-			  const char *name, bool is_kallsyms, bool is_vdso)
+int build_id_cache__add_s(const char *sbuild_id, const char *name,
+			  bool is_kallsyms, bool is_vdso)
 {
 	const size_t size = PATH_MAX;
 	char *realname, *filename = zalloc(size),
@@ -282,7 +282,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 		goto out_free;
 
 	len = scnprintf(filename, size, "%s%s%s",
-		       debugdir, slash ? "/" : "",
+		       buildid_dir, slash ? "/" : "",
 		       is_vdso ? DSO__NAME_VDSO : realname);
 	if (mkdir_p(filename, 0755))
 		goto out_free;
@@ -298,13 +298,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 	}
 
 	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-		       debugdir, sbuild_id);
+			buildid_dir, sbuild_id);
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
 	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
-	targetname = filename + strlen(debugdir) - 5;
+	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
 	if (symlink(targetname, linkname) == 0)
@@ -318,18 +318,17 @@ out_free:
 }
 
 static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
-				 const char *name, const char *debugdir,
-				 bool is_kallsyms, bool is_vdso)
+				 const char *name, bool is_kallsyms,
+				 bool is_vdso)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
 	build_id__sprintf(build_id, build_id_size, sbuild_id);
 
-	return build_id_cache__add_s(sbuild_id, debugdir, name,
-				     is_kallsyms, is_vdso);
+	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
+int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
@@ -340,7 +339,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 		goto out_free;
 
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, sbuild_id + 2);
+		 buildid_dir, sbuild_id, sbuild_id + 2);
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -355,7 +354,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 	 * Since the link is relative, we must make it absolute:
 	 */
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, filename);
+		 buildid_dir, sbuild_id, filename);
 
 	if (unlink(linkname))
 		goto out_free;
@@ -367,8 +366,7 @@ out_free:
 	return err;
 }
 
-static int dso__cache_build_id(struct dso *dso, struct machine *machine,
-			       const char *debugdir)
+static int dso__cache_build_id(struct dso *dso, struct machine *machine)
 {
 	bool is_kallsyms = dso->kernel && dso->long_name[0] != '/';
 	bool is_vdso = dso__is_vdso(dso);
@@ -381,28 +379,26 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine,
 		name = nm;
 	}
 	return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
-				     debugdir, is_kallsyms, is_vdso);
+				     is_kallsyms, is_vdso);
 }
 
 static int __dsos__cache_build_ids(struct list_head *head,
-				   struct machine *machine, const char *debugdir)
+				   struct machine *machine)
 {
 	struct dso *pos;
 	int err = 0;
 
 	dsos__for_each_with_build_id(pos, head)
-		if (dso__cache_build_id(pos, machine, debugdir))
+		if (dso__cache_build_id(pos, machine))
 			err = -1;
 
 	return err;
 }
 
-static int machine__cache_build_ids(struct machine *machine, const char *debugdir)
+static int machine__cache_build_ids(struct machine *machine)
 {
-	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine,
-					  debugdir);
-	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine,
-				       debugdir);
+	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine);
+	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine);
 	return ret;
 }
 
@@ -417,11 +413,11 @@ int perf_session__cache_build_ids(struct perf_session *session)
 	if (mkdir(buildid_dir, 0755) != 0 && errno != EEXIST)
 		return -1;
 
-	ret = machine__cache_build_ids(&session->machines.host, buildid_dir);
+	ret = machine__cache_build_ids(&session->machines.host);
 
 	for (nd = rb_first(&session->machines.guests); nd; nd = rb_next(nd)) {
 		struct machine *pos = rb_entry(nd, struct machine, rb_node);
-		ret |= machine__cache_build_ids(pos, buildid_dir);
+		ret |= machine__cache_build_ids(pos);
 	}
 	return ret ? -1 : 0;
 }
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 8236319..31b3c63 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -22,9 +22,9 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
-int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
+int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
+int build_id_cache__remove_s(const char *sbuild_id);
 void disable_buildid_cache(void);
 
 #endif

[perf/core PATCH 2/4] perf buildid-cache: Consolidate .build-id cache path generators

[Masami Hiramatsu]

Consolidate .build-id cache path generating routines to
build_id__filename() function. Other functions must use
it to get the buildid cache path (link path) from build-id.
This can reduce the risk of partial-update.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/build-id.c |   58 +++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 9f764f6..adbc360 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -93,6 +93,35 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
 	return raw - build_id;
 }
 
+/* asnprintf consolidates asprintf and snprintf */
+static int asnprintf(char **strp, size_t size, const char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	if (!strp)
+		return -EINVAL;
+
+	va_start(ap, fmt);
+	if (*strp)
+		ret = vsnprintf(*strp, size, fmt, ap);
+	else
+		ret = vasprintf(strp, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
+{
+	char *tmp = bf;
+	int ret = asnprintf(&bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
+			    sbuild_id, sbuild_id + 2);
+	if (ret < 0 || (tmp && size < (unsigned int)ret))
+		return NULL;
+	return bf;
+}
+
 char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 {
 	char build_id_hex[BUILD_ID_SIZE * 2 + 1];
@@ -101,14 +130,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 		return NULL;
 
 	build_id__sprintf(dso->build_id, sizeof(dso->build_id), build_id_hex);
-	if (bf == NULL) {
-		if (asprintf(&bf, "%s/.build-id/%.2s/%s", buildid_dir,
-			     build_id_hex, build_id_hex + 2) < 0)
-			return NULL;
-	} else
-		snprintf(bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
-			 build_id_hex, build_id_hex + 2);
-	return bf;
+	return build_id__filename(build_id_hex, bf, size);
 }
 
 #define dsos__for_each_with_build_id(pos, head)	\
@@ -264,7 +286,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 {
 	const size_t size = PATH_MAX;
 	char *realname, *filename = zalloc(size),
-	     *linkname = zalloc(size), *targetname;
+	     *linkname = zalloc(size), *targetname, *tmp;
 	int len, err = -1;
 	bool slash = is_kallsyms || is_vdso;
 
@@ -297,13 +319,15 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			goto out_free;
 	}
 
-	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-			buildid_dir, sbuild_id);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
+	tmp = strrchr(linkname, '/');
+	*tmp = '\0';
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
-	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
+	*tmp = '/';
 	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
@@ -332,14 +356,14 @@ int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
-	     *linkname = zalloc(size);
+	     *linkname = zalloc(size), *tmp;
 	int err = -1;
 
 	if (filename == NULL || linkname == NULL)
 		goto out_free;
 
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, sbuild_id + 2);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -353,8 +377,8 @@ int build_id_cache__remove_s(const char *sbuild_id)
 	/*
 	 * Since the link is relative, we must make it absolute:
 	 */
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, filename);
+	tmp = strrchr(linkname, '/') + 1;
+	snprintf(tmp, size - (tmp - linkname), "%s", filename);
 
 	if (unlink(linkname))
 		goto out_free;

[perf/core PATCH 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Masami Hiramatsu]

Add new buildid cache if the update target file is not cached.
This can happen when an old binary is replaced by new one
after caching the old one. In this case, user sees his operation
just failed. But it does not look straight, since user just
pass the binary "path", not "build-id".

  ----
  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  ./perf wasn't in the cache
  #
  ----

This patch adds given new binary to cache if the new binary is
not cached. So we'll not see the above error.

  ----
  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  #
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/Documentation/perf-buildid-cache.txt |    2 ++
 tools/perf/builtin-buildid-cache.c              |    6 ++++--
 tools/perf/util/build-id.c                      |   12 ++++++++++++
 tools/perf/util/build-id.h                      |    1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 0294c57..6575dce 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -44,6 +44,8 @@ OPTIONS
 --update::
 	Update specified file of the cache. It can be used to update kallsyms
 	kernel dso to vmlinux in order to support annotation.
+	If there is no cache which has given binary's build-id, this just adds
+	new one.
 -v::
 --verbose::
 	Be more verbose.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index d929d95..e7568f5 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
-	int err;
+	int err = 0;
 
 	if (filename__read_build_id(filename, &build_id, sizeof(build_id)) < 0) {
 		pr_debug("Couldn't read a build-id in %s\n", filename);
@@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id);
+	if (build_id_cache__cached(sbuild_id))
+		err = build_id_cache__remove_s(sbuild_id);
+
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index adbc360..0bc33be 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
 	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
+bool build_id_cache__cached(const char *sbuild_id)
+{
+	bool ret = false;
+	char *filename = build_id__filename(sbuild_id, NULL, 0);
+
+	if (filename && !access(filename, F_OK))
+		ret = true;
+	free(filename);
+
+	return ret;
+}
+
 int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 31b3c63..2a09498 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
+bool build_id_cache__cached(const char *sbuild_id);
 int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
 int build_id_cache__remove_s(const char *sbuild_id);

[perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE

[Masami Hiramatsu]

Add --remove-all FILE to remove all caches of FILE.
Since the current --remove FILE removes a cache which has
same build-id of given FILE. Since the command takes a
FILE path, it can confuse user who tries to remove cache
about FILE path.

  -----
  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --remove ./perf
  Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
  ./perf wasn't in the cache
  -----
Actually, the --remove's FAIL is not shown, it just silently fails.

So, this patch adds --remove-all FILE action for such usecase.
perf buildid-cache --remove-all FILE removes all caches which
has same FILE path.
In other words, it removes all caches including old binaries.

  -----
  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --remove-all ./perf
  Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  -----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/Documentation/perf-buildid-cache.txt |    6 +-
 tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
 tools/perf/util/build-id.c                      |   81 ++++++++++++++++++-----
 tools/perf/util/build-id.h                      |    1 
 4 files changed, 115 insertions(+), 17 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 6575dce..01f295e 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -36,7 +36,11 @@ OPTIONS
         actually made.
 -r::
 --remove=::
-        Remove specified file from the cache.
+        Remove a cached binary which has same build-id of specified file
+        from the cache.
+-R::
+--remove-all=::
+        Remove all cached binary which has specified path from the cache.
 -M::
 --missing=::
 	List missing build ids in the cache for the specified file.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index e7568f5..3a76d51 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
 	return err;
 }
 
+static int build_id_cache__remove_path(const char *pathname)
+{
+	struct strlist *list;
+	struct str_node *pos;
+	int err;
+
+	list = build_id_cache__list_build_ids(pathname);
+	if (!list)
+		return 0;
+
+	strlist__for_each(pos, list) {
+		err = build_id_cache__remove_s(pos->s);
+		if (verbose)
+			pr_info("Removing %s %s: %s\n", pos->s, pathname,
+				err ? "FAIL" : "Ok");
+		if (err)
+			break;
+	}
+	strlist__delete(list);
+
+	return err;
+}
+
 static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
 {
 	char filename[PATH_MAX];
@@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 	bool force = false;
 	char const *add_name_list_str = NULL,
 		   *remove_name_list_str = NULL,
+		   *remove_all_name_list_str = NULL,
 		   *missing_filename = NULL,
 		   *update_name_list_str = NULL,
 		   *kcore_filename = NULL;
@@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
 		   "file", "kcore file to add"),
 	OPT_STRING('r', "remove", &remove_name_list_str, "file list",
 		    "file(s) to remove"),
+	OPT_STRING('R', "remove-all", &remove_all_name_list_str, "path list",
+		    "path(s) to remove (remove old caches too)"),
 	OPT_STRING('M', "missing", &missing_filename, "file",
 		   "to find missing build ids in the cache"),
 	OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
@@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
 		}
 	}
 
+	if (remove_all_name_list_str) {
+		list = strlist__new(true, remove_all_name_list_str);
+		if (list) {
+			strlist__for_each(pos, list)
+				if (build_id_cache__remove_path(pos->s)) {
+					if (errno == ENOENT) {
+						pr_debug("%s wasn't in the cache\n",
+							 pos->s);
+						continue;
+					}
+					pr_warning("Couldn't remove %s: %s\n",
+						   pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
+				}
+
+			strlist__delete(list);
+		}
+	}
+
 	if (missing_filename)
 		ret = build_id_cache__fprintf_missing(session, stdout);
 
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 0bc33be..e688311 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -281,35 +281,84 @@ void disable_buildid_cache(void)
 	no_buildid_cache = true;
 }
 
+static char *build_id_cache__dirname_from_path(const char *name,
+					       bool is_kallsyms, bool is_vdso)
+{
+	char *realname = (char *)name, *filename;
+	bool slash = is_kallsyms || is_vdso;
+
+	if (!slash) {
+		realname = realpath(name, NULL);
+		if (!realname)
+			return NULL;
+	}
+
+	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
+		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
+		filename = NULL;
+	free(realname);
+
+	return filename;
+}
+
+struct strlist *build_id_cache__list_build_ids(const char *pathname)
+{
+	struct strlist *list;
+	char *dirname;
+	DIR *dir;
+	struct dirent *d;
+
+	list = strlist__new(true, NULL);
+	dirname = build_id_cache__dirname_from_path(pathname, false, false);
+	if (!list || !dirname)
+		goto error_free;
+
+	/* List up all dirents */
+	dir = opendir(dirname);
+	if (!dir)
+		goto error_free;
+	while ((d = readdir(dir)) != NULL) {
+		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
+			continue;
+		strlist__add(list, d->d_name);
+	}
+	closedir(dir);
+
+	free(dirname);
+	return list;
+
+error_free:
+	free(dirname);
+	if (list)
+		strlist__delete(list);
+	return NULL;
+}
+
 int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			  bool is_kallsyms, bool is_vdso)
 {
 	const size_t size = PATH_MAX;
-	char *realname, *filename = zalloc(size),
+	char *realname = NULL, *filename = NULL,
 	     *linkname = zalloc(size), *targetname, *tmp;
-	int len, err = -1;
-	bool slash = is_kallsyms || is_vdso;
+	int err = -1;
 
-	if (is_kallsyms) {
-		if (symbol_conf.kptr_restrict) {
-			pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
-			err = 0;
-			goto out_free;
-		}
-		realname = (char *) name;
-	} else
+	if (!is_kallsyms) {
 		realname = realpath(name, NULL);
+		if (!realname)
+			goto out_free;
+	}
 
-	if (realname == NULL || filename == NULL || linkname == NULL)
+	filename = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+	if (!filename)
 		goto out_free;
 
-	len = scnprintf(filename, size, "%s%s%s",
-		       buildid_dir, slash ? "/" : "",
-		       is_vdso ? DSO__NAME_VDSO : realname);
 	if (mkdir_p(filename, 0755))
 		goto out_free;
 
-	snprintf(filename + len, size - len, "/%s", sbuild_id);
+	if (asprintf(&filename, "%s/%s", filename, sbuild_id) < 0) {
+		filename = NULL;
+		goto out_free;
+	}
 
 	if (access(filename, F_OK)) {
 		if (is_kallsyms) {
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 2a09498..cbcadea 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
+struct strlist *build_id_cache__list_build_ids(const char *pathname);
 bool build_id_cache__cached(const char *sbuild_id);
 int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);

Re: [perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE

[Namhyung Kim]

Hi Masami,

On Sat, Feb 07, 2015 at 07:14:19PM +0900, Masami Hiramatsu wrote:
> Add --remove-all FILE to remove all caches of FILE.
> Since the current --remove FILE removes a cache which has
> same build-id of given FILE. Since the command takes a
> FILE path, it can confuse user who tries to remove cache
> about FILE path.
> 
>   -----
>   # ./perf buildid-cache -v --add ./perf
>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   # (update the ./perf binary)
>   # ./perf buildid-cache -v --remove ./perf
>   Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
>   ./perf wasn't in the cache
>   -----
> Actually, the --remove's FAIL is not shown, it just silently fails.
> 
> So, this patch adds --remove-all FILE action for such usecase.
> perf buildid-cache --remove-all FILE removes all caches which
> has same FILE path.
> In other words, it removes all caches including old binaries.
> 
>   -----
>   # ./perf buildid-cache -v --add ./perf
>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   # (update the ./perf binary)
>   # ./perf buildid-cache -v --remove-all ./perf
>   Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   -----
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/Documentation/perf-buildid-cache.txt |    6 +-
>  tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
>  tools/perf/util/build-id.c                      |   81 ++++++++++++++++++-----
>  tools/perf/util/build-id.h                      |    1 
>  4 files changed, 115 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 6575dce..01f295e 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -36,7 +36,11 @@ OPTIONS
>          actually made.
>  -r::
>  --remove=::
> -        Remove specified file from the cache.
> +        Remove a cached binary which has same build-id of specified file
> +        from the cache.
> +-R::
> +--remove-all=::
> +        Remove all cached binary which has specified path from the cache.
>  -M::
>  --missing=::
>  	List missing build ids in the cache for the specified file.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index e7568f5..3a76d51 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
>  	return err;
>  }
>  
> +static int build_id_cache__remove_path(const char *pathname)
> +{
> +	struct strlist *list;
> +	struct str_node *pos;
> +	int err;
> +
> +	list = build_id_cache__list_build_ids(pathname);
> +	if (!list)
> +		return 0;
> +
> +	strlist__for_each(pos, list) {
> +		err = build_id_cache__remove_s(pos->s);
> +		if (verbose)
> +			pr_info("Removing %s %s: %s\n", pos->s, pathname,
> +				err ? "FAIL" : "Ok");
> +		if (err)
> +			break;
> +	}
> +	strlist__delete(list);
> +
> +	return err;
> +}
> +
>  static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
>  {
>  	char filename[PATH_MAX];
> @@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
>  	bool force = false;
>  	char const *add_name_list_str = NULL,
>  		   *remove_name_list_str = NULL,
> +		   *remove_all_name_list_str = NULL,
>  		   *missing_filename = NULL,
>  		   *update_name_list_str = NULL,
>  		   *kcore_filename = NULL;
> @@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
>  		   "file", "kcore file to add"),
>  	OPT_STRING('r', "remove", &remove_name_list_str, "file list",
>  		    "file(s) to remove"),
> +	OPT_STRING('R', "remove-all", &remove_all_name_list_str, "path list",
> +		    "path(s) to remove (remove old caches too)"),
>  	OPT_STRING('M', "missing", &missing_filename, "file",
>  		   "to find missing build ids in the cache"),
>  	OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
> @@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
>  		}
>  	}
>  
> +	if (remove_all_name_list_str) {
> +		list = strlist__new(true, remove_all_name_list_str);
> +		if (list) {
> +			strlist__for_each(pos, list)
> +				if (build_id_cache__remove_path(pos->s)) {
> +					if (errno == ENOENT) {
> +						pr_debug("%s wasn't in the cache\n",
> +							 pos->s);
> +						continue;
> +					}
> +					pr_warning("Couldn't remove %s: %s\n",
> +						   pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
> +				}
> +
> +			strlist__delete(list);
> +		}
> +	}
> +
>  	if (missing_filename)
>  		ret = build_id_cache__fprintf_missing(session, stdout);
>  
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 0bc33be..e688311 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -281,35 +281,84 @@ void disable_buildid_cache(void)
>  	no_buildid_cache = true;
>  }
>  
> +static char *build_id_cache__dirname_from_path(const char *name,
> +					       bool is_kallsyms, bool is_vdso)
> +{
> +	char *realname = (char *)name, *filename;
> +	bool slash = is_kallsyms || is_vdso;
> +
> +	if (!slash) {
> +		realname = realpath(name, NULL);
> +		if (!realname)
> +			return NULL;
> +	}
> +
> +	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
> +		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
> +		filename = NULL;
> +	free(realname);

Shouldn't it be:

	if (!slash)
		free(realname);

?

Thanks,
Namhyung


> +
> +	return filename;
> +}
> +
> +struct strlist *build_id_cache__list_build_ids(const char *pathname)
> +{
> +	struct strlist *list;
> +	char *dirname;
> +	DIR *dir;
> +	struct dirent *d;
> +
> +	list = strlist__new(true, NULL);
> +	dirname = build_id_cache__dirname_from_path(pathname, false, false);
> +	if (!list || !dirname)
> +		goto error_free;
> +
> +	/* List up all dirents */
> +	dir = opendir(dirname);
> +	if (!dir)
> +		goto error_free;
> +	while ((d = readdir(dir)) != NULL) {
> +		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> +			continue;
> +		strlist__add(list, d->d_name);
> +	}
> +	closedir(dir);
> +
> +	free(dirname);
> +	return list;
> +
> +error_free:
> +	free(dirname);
> +	if (list)
> +		strlist__delete(list);
> +	return NULL;
> +}
> +
>  int build_id_cache__add_s(const char *sbuild_id, const char *name,
>  			  bool is_kallsyms, bool is_vdso)
>  {
>  	const size_t size = PATH_MAX;
> -	char *realname, *filename = zalloc(size),
> +	char *realname = NULL, *filename = NULL,
>  	     *linkname = zalloc(size), *targetname, *tmp;
> -	int len, err = -1;
> -	bool slash = is_kallsyms || is_vdso;
> +	int err = -1;
>  
> -	if (is_kallsyms) {
> -		if (symbol_conf.kptr_restrict) {
> -			pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
> -			err = 0;
> -			goto out_free;
> -		}
> -		realname = (char *) name;
> -	} else
> +	if (!is_kallsyms) {
>  		realname = realpath(name, NULL);
> +		if (!realname)
> +			goto out_free;
> +	}
>  
> -	if (realname == NULL || filename == NULL || linkname == NULL)
> +	filename = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
> +	if (!filename)
>  		goto out_free;
>  
> -	len = scnprintf(filename, size, "%s%s%s",
> -		       buildid_dir, slash ? "/" : "",
> -		       is_vdso ? DSO__NAME_VDSO : realname);
>  	if (mkdir_p(filename, 0755))
>  		goto out_free;
>  
> -	snprintf(filename + len, size - len, "/%s", sbuild_id);
> +	if (asprintf(&filename, "%s/%s", filename, sbuild_id) < 0) {
> +		filename = NULL;
> +		goto out_free;
> +	}
>  
>  	if (access(filename, F_OK)) {
>  		if (is_kallsyms) {
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 2a09498..cbcadea 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
>  int perf_session__write_buildid_table(struct perf_session *session, int fd);
>  int perf_session__cache_build_ids(struct perf_session *session);
>  
> +struct strlist *build_id_cache__list_build_ids(const char *pathname);
>  bool build_id_cache__cached(const char *sbuild_id);
>  int build_id_cache__add_s(const char *sbuild_id,
>  			  const char *name, bool is_kallsyms, bool is_vdso);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: Re: [perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE

[Masami Hiramatsu]

(2015/02/09 12:22), Namhyung Kim wrote:
> Hi Masami,
> 
> On Sat, Feb 07, 2015 at 07:14:19PM +0900, Masami Hiramatsu wrote:
>> Add --remove-all FILE to remove all caches of FILE.
>> Since the current --remove FILE removes a cache which has
>> same build-id of given FILE. Since the command takes a
>> FILE path, it can confuse user who tries to remove cache
>> about FILE path.
>>
>>   -----
>>   # ./perf buildid-cache -v --add ./perf
>>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>>   # (update the ./perf binary)
>>   # ./perf buildid-cache -v --remove ./perf
>>   Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
>>   ./perf wasn't in the cache
>>   -----
>> Actually, the --remove's FAIL is not shown, it just silently fails.
>>
>> So, this patch adds --remove-all FILE action for such usecase.
>> perf buildid-cache --remove-all FILE removes all caches which
>> has same FILE path.
>> In other words, it removes all caches including old binaries.
>>
>>   -----
>>   # ./perf buildid-cache -v --add ./perf
>>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>>   # (update the ./perf binary)
>>   # ./perf buildid-cache -v --remove-all ./perf
>>   Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>>   -----
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/Documentation/perf-buildid-cache.txt |    6 +-
>>  tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
>>  tools/perf/util/build-id.c                      |   81 ++++++++++++++++++-----
>>  tools/perf/util/build-id.h                      |    1 
>>  4 files changed, 115 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
>> index 6575dce..01f295e 100644
>> --- a/tools/perf/Documentation/perf-buildid-cache.txt
>> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
>> @@ -36,7 +36,11 @@ OPTIONS
>>          actually made.
>>  -r::
>>  --remove=::
>> -        Remove specified file from the cache.
>> +        Remove a cached binary which has same build-id of specified file
>> +        from the cache.
>> +-R::
>> +--remove-all=::
>> +        Remove all cached binary which has specified path from the cache.
>>  -M::
>>  --missing=::
>>  	List missing build ids in the cache for the specified file.
>> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
>> index e7568f5..3a76d51 100644
>> --- a/tools/perf/builtin-buildid-cache.c
>> +++ b/tools/perf/builtin-buildid-cache.c
>> @@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
>>  	return err;
>>  }
>>  
>> +static int build_id_cache__remove_path(const char *pathname)
>> +{
>> +	struct strlist *list;
>> +	struct str_node *pos;
>> +	int err;
>> +
>> +	list = build_id_cache__list_build_ids(pathname);
>> +	if (!list)
>> +		return 0;
>> +
>> +	strlist__for_each(pos, list) {
>> +		err = build_id_cache__remove_s(pos->s);
>> +		if (verbose)
>> +			pr_info("Removing %s %s: %s\n", pos->s, pathname,
>> +				err ? "FAIL" : "Ok");
>> +		if (err)
>> +			break;
>> +	}
>> +	strlist__delete(list);
>> +
>> +	return err;
>> +}
>> +
>>  static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
>>  {
>>  	char filename[PATH_MAX];
>> @@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
>>  	bool force = false;
>>  	char const *add_name_list_str = NULL,
>>  		   *remove_name_list_str = NULL,
>> +		   *remove_all_name_list_str = NULL,
>>  		   *missing_filename = NULL,
>>  		   *update_name_list_str = NULL,
>>  		   *kcore_filename = NULL;
>> @@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
>>  		   "file", "kcore file to add"),
>>  	OPT_STRING('r', "remove", &remove_name_list_str, "file list",
>>  		    "file(s) to remove"),
>> +	OPT_STRING('R', "remove-all", &remove_all_name_list_str, "path list",
>> +		    "path(s) to remove (remove old caches too)"),
>>  	OPT_STRING('M', "missing", &missing_filename, "file",
>>  		   "to find missing build ids in the cache"),
>>  	OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
>> @@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
>>  		}
>>  	}
>>  
>> +	if (remove_all_name_list_str) {
>> +		list = strlist__new(true, remove_all_name_list_str);
>> +		if (list) {
>> +			strlist__for_each(pos, list)
>> +				if (build_id_cache__remove_path(pos->s)) {
>> +					if (errno == ENOENT) {
>> +						pr_debug("%s wasn't in the cache\n",
>> +							 pos->s);
>> +						continue;
>> +					}
>> +					pr_warning("Couldn't remove %s: %s\n",
>> +						   pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
>> +				}
>> +
>> +			strlist__delete(list);
>> +		}
>> +	}
>> +
>>  	if (missing_filename)
>>  		ret = build_id_cache__fprintf_missing(session, stdout);
>>  
>> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
>> index 0bc33be..e688311 100644
>> --- a/tools/perf/util/build-id.c
>> +++ b/tools/perf/util/build-id.c
>> @@ -281,35 +281,84 @@ void disable_buildid_cache(void)
>>  	no_buildid_cache = true;
>>  }
>>  
>> +static char *build_id_cache__dirname_from_path(const char *name,
>> +					       bool is_kallsyms, bool is_vdso)
>> +{
>> +	char *realname = (char *)name, *filename;
>> +	bool slash = is_kallsyms || is_vdso;
>> +
>> +	if (!slash) {
>> +		realname = realpath(name, NULL);
>> +		if (!realname)
>> +			return NULL;
>> +	}
>> +
>> +	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
>> +		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
>> +		filename = NULL;
>> +	free(realname);
> 
> Shouldn't it be:
> 
> 	if (!slash)
> 		free(realname);

Ah! right:)

Thank you


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com