From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752429AbdA3KdZ (ORCPT ); Mon, 30 Jan 2017 05:33:25 -0500 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33141 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751443AbdA3KcQ (ORCPT ); Mon, 30 Jan 2017 05:32:16 -0500 Subject: Re: [PATCH] perf tools: Create for_each_event{_system, _file} macros for tracepoints iteration To: Jiri Olsa References: <1485765328-21787-1-git-send-email-treeze.taeung@gmail.com> <20170130090812.GD22029@krava> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Namhyung Kim , Ingo Molnar , Peter Zijlstra , Wang Nan , Steven Rostedt , Frederic Weisbecker From: Taeung Song Message-ID: Date: Mon, 30 Jan 2017 19:32:03 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20170130090812.GD22029@krava> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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