Hi,
Here is the 4th version of of perf buildid-cache update.
This has a small updates from v2 for each patch. According to
Arnaldo's comment, I updated the document about --update
option in the first patch, and renamed --remove-all to
--purge in the second patch (it also updated document).
Only one change from 3rd to 4th version.
- Release a buffer allocated by build_id_cache__dirname_from_path()
in build_id_cache__add_s() [patch 2/2]
Thank you,
---
Masami Hiramatsu (2):
perf buildid-cache: Add new buildid cache if update target is not cached
perf buildid-cache: Add --purge FILE to remove all caches of FILE
tools/perf/Documentation/perf-buildid-cache.txt | 24 ++++--
tools/perf/builtin-buildid-cache.c | 50 +++++++++++-
tools/perf/util/build-id.c | 98 +++++++++++++++++++----
tools/perf/util/build-id.h | 2
4 files changed, 148 insertions(+), 26 deletions(-)
--
Masami HIRAMATSU
Software Platform Research Dpt. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
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 <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 11 ++++++++---
tools/perf/builtin-buildid-cache.c | 6 ++++--
tools/perf/util/build-id.c | 12 ++++++++++++
tools/perf/util/build-id.h | 1 +
4 files changed, 25 insertions(+), 5 deletions(-)
diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 0294c57..cec6b57 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -41,9 +41,14 @@ OPTIONS
--missing=::
List missing build ids in the cache for the specified file.
-u::
---update::
- Update specified file of the cache. It can be used to update kallsyms
- kernel dso to vmlinux in order to support annotation.
+--update=::
+ Update specified file of the cache. Note that this doesn't remove
+ older entires since those may be still needed for annotating old
+ (or remote) perf.data. Only if there is already a cache which has
+ exactly same build-id, that is replaced by new one. It can be used
+ to update kallsyms and kernel dso to vmlinux in order to support
+ annotation.
+
-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);
Add --purge 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 --purge FILE action for such usecase.
perf buildid-cache --purge 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 --purge ./perf
Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
-----
BTW, if you want to purge all the caches, remove ~/.debug/* .
Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 13 ++-
tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
tools/perf/util/build-id.c | 86 ++++++++++++++++++-----
tools/perf/util/build-id.h | 1
4 files changed, 123 insertions(+), 21 deletions(-)
diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index cec6b57..dd07b55 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -12,9 +12,9 @@ SYNOPSIS
DESCRIPTION
-----------
-This command manages the build-id cache. It can add and remove files to/from
-the cache. In the future it should as well purge older entries, set upper
-limits for the space used by the cache, etc.
+This command manages the build-id cache. It can add, remove, update and purge
+files to/from the cache. In the future it should as well set upper limits for
+the space used by the cache, etc.
OPTIONS
-------
@@ -36,7 +36,12 @@ 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.
+-p::
+--purge=::
+ Purge all cached binaries including older caches which have 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..37182bb 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__purge_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,
+ *purge_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('p', "purge", &purge_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 (purge_name_list_str) {
+ list = strlist__new(true, purge_name_list_str);
+ if (list) {
+ strlist__for_each(pos, list)
+ if (build_id_cache__purge_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..5a9a216 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -281,35 +281,86 @@ 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;
+
+ if (!slash)
+ 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, *dirname = 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)
+ dirname = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+ if (!dirname)
goto out_free;
- len = scnprintf(filename, size, "%s%s%s",
- buildid_dir, slash ? "/" : "",
- is_vdso ? DSO__NAME_VDSO : realname);
- if (mkdir_p(filename, 0755))
+ if (mkdir_p(dirname, 0755))
goto out_free;
- snprintf(filename + len, size - len, "/%s", sbuild_id);
+ if (asprintf(&filename, "%s/%s", dirname, sbuild_id) < 0) {
+ filename = NULL;
+ goto out_free;
+ }
if (access(filename, F_OK)) {
if (is_kallsyms) {
@@ -337,6 +388,7 @@ out_free:
if (!is_kallsyms)
free(realname);
free(filename);
+ free(dirname);
free(linkname);
return err;
}
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);
Hi Masami,
Just a small suggestion below.
On 02/20/2015 03:11 PM, Masami Hiramatsu wrote:
> [SNIP]
> +
> +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);
Maybe we don't need the "if (list)" check here as strlist__delete
already checks for this.
> + 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, *dirname = 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)
> + dirname = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
> + if (!dirname)
> goto out_free;
>
> - len = scnprintf(filename, size, "%s%s%s",
> - buildid_dir, slash ? "/" : "",
> - is_vdso ? DSO__NAME_VDSO : realname);
> - if (mkdir_p(filename, 0755))
> + if (mkdir_p(dirname, 0755))
> goto out_free;
>
> - snprintf(filename + len, size - len, "/%s", sbuild_id);
> + if (asprintf(&filename, "%s/%s", dirname, sbuild_id) < 0) {
> + filename = NULL;
> + goto out_free;
> + }
>
> if (access(filename, F_OK)) {
> if (is_kallsyms) {
> @@ -337,6 +388,7 @@ out_free:
> if (!is_kallsyms)
> free(realname);
> free(filename);
> + free(dirname);
> free(linkname);
> return err;
> }
> 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);
>
--
Thanks,
Hemant Kumar
Hi Masami,
On Fri, Feb 20, 2015 at 06:41:50PM +0900, Masami Hiramatsu wrote:
> Add --purge 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 --purge FILE action for such usecase.
> perf buildid-cache --purge 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 --purge ./perf
> Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> -----
>
> BTW, if you want to purge all the caches, remove ~/.debug/* .
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
I have a nitpick below - other than that both patches look good.
Acked-by: Namhyung Kim <[email protected]>
Thanks,
Namhyung
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 13 ++-
> tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
> tools/perf/util/build-id.c | 86 ++++++++++++++++++-----
> tools/perf/util/build-id.h | 1
> 4 files changed, 123 insertions(+), 21 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index cec6b57..dd07b55 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -12,9 +12,9 @@ SYNOPSIS
>
> DESCRIPTION
> -----------
> -This command manages the build-id cache. It can add and remove files to/from
> -the cache. In the future it should as well purge older entries, set upper
> -limits for the space used by the cache, etc.
> +This command manages the build-id cache. It can add, remove, update and purge
> +files to/from the cache. In the future it should as well set upper limits for
> +the space used by the cache, etc.
>
> OPTIONS
> -------
> @@ -36,7 +36,12 @@ 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.
> +-p::
> +--purge=::
> + Purge all cached binaries including older caches which have 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..37182bb 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__purge_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");
You can simply use pr_debug() here. :)
Thanks,
Namhyung
> + if (err)
> + break;
> + }
> + strlist__delete(list);
> +
> + return err;
> +}
> +
(2015/02/21 22:56), Namhyung Kim wrote:
> Hi Masami,
>
> On Fri, Feb 20, 2015 at 06:41:50PM +0900, Masami Hiramatsu wrote:
>> Add --purge 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 --purge FILE action for such usecase.
>> perf buildid-cache --purge 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 --purge ./perf
>> Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>> -----
>>
>> BTW, if you want to purge all the caches, remove ~/.debug/* .
>>
>> Signed-off-by: Masami Hiramatsu <[email protected]>
>
> I have a nitpick below - other than that both patches look good.
>
> Acked-by: Namhyung Kim <[email protected]>
Thanks!
>
> Thanks,
> Namhyung
>
[...]
>> +static int build_id_cache__purge_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");
>
> You can simply use pr_debug() here. :)
Yes, but other operations already uses pr_info instead of pr_debug.
I thinks we'd better change them all at once.
Thank you,
>
> Thanks,
> Namhyung
>
>
>> + if (err)
>> + break;
>> + }
>> + strlist__delete(list);
>> +
>> + return err;
>> +}
>> +
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
(2015/02/21 4:07), Hemant Kumar wrote:
> Hi Masami,
>
> Just a small suggestion below.
>
> On 02/20/2015 03:11 PM, Masami Hiramatsu wrote:
>> [SNIP]
>> +
>> +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);
>
> Maybe we don't need the "if (list)" check here as strlist__delete
> already checks for this.
Ah, right!
Thanks!
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]