[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <bc0b5c66-0de0-e380-6925-7d4f4d9fccb7@gmail.com>
Date: Mon, 30 Jan 2017 19:32:03 +0900
From: Taeung Song <treeze.taeung@...il.com>
To: Jiri Olsa <jolsa@...hat.com>
Cc: Arnaldo Carvalho de Melo <acme@...nel.org>,
linux-kernel@...r.kernel.org, Jiri Olsa <jolsa@...nel.org>,
Namhyung Kim <namhyung@...nel.org>,
Ingo Molnar <mingo@...nel.org>,
Peter Zijlstra <peterz@...radead.org>,
Wang Nan <wangnan0@...wei.com>,
Steven Rostedt <rostedt@...dmis.org>,
Frederic Weisbecker <fweisbec@...il.com>
Subject: Re: [PATCH] perf tools: Create for_each_event{_system, _file} macros
for tracepoints iteration
On 01/30/2017 06:08 PM, Jiri Olsa wrote:
> On Mon, Jan 30, 2017 at 05:35:28PM +0900, Taeung Song wrote:
>
> SNIP
>
>> +#define for_each_event_system(dir, dent, tps) \
>> + while ((dent = readdir(dir))) \
>> + if (dent->d_type == DT_DIR && \
>> + (strcmp(dent->d_name, ".")) && \
>> + (strcmp(dent->d_name, "..")) && \
>> + (name_in_tp_list(dent->d_name, tps)))
>> +
>> static int copy_event_system(const char *sys, struct tracepoint_path *tps)
>> {
>> struct dirent *dent;
>> @@ -186,12 +193,7 @@ static int copy_event_system(const char *sys, struct tracepoint_path *tps)
>> return -errno;
>> }
>>
>> - while ((dent = readdir(dir))) {
>> - if (dent->d_type != DT_DIR ||
>> - strcmp(dent->d_name, ".") == 0 ||
>> - strcmp(dent->d_name, "..") == 0 ||
>> - !name_in_tp_list(dent->d_name, tps))
>> - continue;
>> + for_each_event_system(dir, dent, tps) {
>> if (asprintf(&format, "%s/%s/format", sys, dent->d_name) < 0) {
>> err = -ENOMEM;
>> goto out;
>> @@ -210,12 +212,7 @@ static int copy_event_system(const char *sys, struct tracepoint_path *tps)
>> }
>>
>> rewinddir(dir);
>> - while ((dent = readdir(dir))) {
>> - if (dent->d_type != DT_DIR ||
>> - strcmp(dent->d_name, ".") == 0 ||
>> - strcmp(dent->d_name, "..") == 0 ||
>> - !name_in_tp_list(dent->d_name, tps))
>> - continue;
>> + for_each_event_system(dir, dent, tps) {
>> if (asprintf(&format, "%s/%s/format", sys, dent->d_name) < 0) {
>> err = -ENOMEM;
>> goto out;
>> @@ -266,6 +263,14 @@ static bool system_in_tp_list(char *sys, struct tracepoint_path *tps)
>> return false;
>> }
>>
>> +#define for_each_event_file(dir, dent, tps) \
>> + while ((dent = readdir(dir))) \
>> + if (dent->d_type == DT_DIR && \
>> + (strcmp(dent->d_name, ".")) && \
>> + (strcmp(dent->d_name, "..")) && \
>> + (strcmp(dent->d_name, "ftrace")) && \
>> + (system_in_tp_list(dent->d_name, tps)))
>
> looks almost the same as for_each_event_system macro,
> what's the difference other than 'ftrace' check?
Little thing.
In parse-events.c for_each_event macro contains
tp_event_has_id() function But this function isn't used in
util/trace-event-info.c
> also why's one 'file' and the other 'system'? looks
Just, I follow the name of each function that use the macros..
> like we coud have just one macro in here
>
I understood. :)
How about this ?
diff --git a/tools/perf/util/trace-event-info.c
b/tools/perf/util/trace-event-info.c
index ceb0e27..fb6d95d 100644
--- a/tools/perf/util/trace-event-info.c
+++ b/tools/perf/util/trace-event-info.c
@@ -170,6 +170,12 @@ static bool name_in_tp_list(char *sys, struct
tracepoint_path *tps)
return false;
}
+#define for_each_event(dir, dent, tps) \
+ while ((dent = readdir(dir))) \
+ if (dent->d_type == DT_DIR && \
+ (strcmp(dent->d_name, ".")) && \
+ (strcmp(dent->d_name, ".."))) \
+
static int copy_event_system(const char *sys, struct tracepoint_path *tps)
{
struct dirent *dent;
@@ -186,12 +192,10 @@ static int copy_event_system(const char *sys,
struct tracepoint_path *tps)
return -errno;
}
- while ((dent = readdir(dir))) {
- if (dent->d_type != DT_DIR ||
- strcmp(dent->d_name, ".") == 0 ||
- strcmp(dent->d_name, "..") == 0 ||
- !name_in_tp_list(dent->d_name, tps))
+ for_each_event(dir, dent, tps) {
+ if (!name_in_tp_list(dent->d_name, tps))
continue;
+
if (asprintf(&format, "%s/%s/format", sys,
dent->d_name) < 0) {
err = -ENOMEM;
goto out;
@@ -210,12 +214,10 @@ static int copy_event_system(const char *sys,
struct tracepoint_path *tps)
}
rewinddir(dir);
- while ((dent = readdir(dir))) {
- if (dent->d_type != DT_DIR ||
- strcmp(dent->d_name, ".") == 0 ||
- strcmp(dent->d_name, "..") == 0 ||
- !name_in_tp_list(dent->d_name, tps))
+ for_each_event(dir, dent, tps) {
+ if (!name_in_tp_list(dent->d_name, tps))
continue;
+
if (asprintf(&format, "%s/%s/format", sys,
dent->d_name) < 0) {
err = -ENOMEM;
goto out;
@@ -290,13 +292,11 @@ static int record_event_files(struct
tracepoint_path *tps)
goto out;
}
- while ((dent = readdir(dir))) {
- if (dent->d_type != DT_DIR ||
- strcmp(dent->d_name, ".") == 0 ||
- strcmp(dent->d_name, "..") == 0 ||
- strcmp(dent->d_name, "ftrace") == 0 ||
+ for_each_event(dir, dent, tps) {
+ if (!strcmp(dent->d_name, "ftrace") ||
!system_in_tp_list(dent->d_name, tps))
continue;
+
count++;
}
@@ -307,13 +307,11 @@ static int record_event_files(struct
tracepoint_path *tps)
}
rewinddir(dir);
- while ((dent = readdir(dir))) {
- if (dent->d_type != DT_DIR ||
- strcmp(dent->d_name, ".") == 0 ||
- strcmp(dent->d_name, "..") == 0 ||
- strcmp(dent->d_name, "ftrace") == 0 ||
+ for_each_event(dir, dent, tps) {
+ if (!strcmp(dent->d_name, "ftrace") ||
!system_in_tp_list(dent->d_name, tps))
continue;
+
if (asprintf(&sys, "%s/%s", path, dent->d_name) < 0) {
err = -ENOMEM;
goto out;
Thanks,
Taeung
Powered by blists - more mailing lists