* [PATCH 3/3] trace-cmd: fold option parsing @ 2011-03-10 9:58 Lai Jiangshan 2011-03-10 13:16 ` Steven Rostedt 0 siblings, 1 reply; 4+ messages in thread From: Lai Jiangshan @ 2011-03-10 9:58 UTC (permalink / raw) To: Steven Rostedt, LKML Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- trace-listen.c | 12 +++++------- trace-read.c | 35 +++++++++++++++-------------------- trace-record.c | 14 +++++--------- trace-stack.c | 32 +++++++++++++++----------------- 4 files changed, 40 insertions(+), 53 deletions(-) diff --git a/trace-listen.c b/trace-listen.c index e60c96b..daaffca 100644 --- a/trace-listen.c +++ b/trace-listen.c @@ -647,6 +647,8 @@ static void start_daemon(void) die("starting daemon"); } +#define OPT_debug 255 + void trace_listen(int argc, char **argv) { char *logfile = NULL; @@ -666,7 +668,7 @@ void trace_listen(int argc, char **argv) static struct option long_options[] = { {"port", required_argument, NULL, 'p'}, {"help", no_argument, NULL, '?'}, - {"debug", no_argument, NULL, 0}, + {"debug", no_argument, NULL, OPT_debug}, {NULL, 0, NULL, 0} }; @@ -696,12 +698,8 @@ void trace_listen(int argc, char **argv) case 'D': daemon = 1; break; - case 0: - switch (option_index) { - case 2: - debug = 1; - break; - } + case OPT_debug: + debug = 1; break; default: usage(argv); diff --git a/trace-read.c b/trace-read.c index 932f5aa..018476d 100644 --- a/trace-read.c +++ b/trace-read.c @@ -812,6 +812,10 @@ static void add_functions(struct pevent *pevent, const char *file) free(buf); } +#define OPT_cpu 255 +#define OPT_events 254 +#define OPT_kallsyms 253 + void trace_report (int argc, char **argv) { struct tracecmd_input *handle; @@ -845,10 +849,10 @@ void trace_report (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"cpu", required_argument, NULL, 0}, - {"events", no_argument, NULL, 0}, + {"cpu", required_argument, NULL, OPT_cpu}, + {"events", no_argument, NULL, OPT_events}, {"filter-test", no_argument, NULL, 'T'}, - {"kallsyms", required_argument, NULL, 0}, + {"kallsyms", required_argument, NULL, OPT_kallsyms}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -917,23 +921,14 @@ void trace_report (int argc, char **argv) case 'q': silence_warnings = 1; break; - case 0: - switch(option_index) { - case 0: /* cpu */ - parse_cpulist(optarg); - break; - case 1: /* events */ - print_events = 1; - break; - case 2: /* filter-test */ - test_filters = 1; - break; - case 3: /* kallsyms */ - functions = optarg; - break; - default: - usage(argv); - } + case OPT_cpu: + parse_cpulist(optarg); + break; + case OPT_events: + print_events = 1; + break; + case OPT_kallsyms: + functions = optarg; break; default: usage(argv); diff --git a/trace-record.c b/trace-record.c index 2611bfa..1cf13b6 100644 --- a/trace-record.c +++ b/trace-record.c @@ -1809,6 +1809,8 @@ static void record_all_events(void) listed_events = list; } +#define OPT_date 255 + void trace_record (int argc, char **argv) { const char *plugin = NULL; @@ -1862,7 +1864,7 @@ void trace_record (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"date", no_argument, NULL, 0}, + {"date", no_argument, NULL, OPT_date}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -2002,14 +2004,8 @@ void trace_record (int argc, char **argv) case 'i': ignore_event_not_found = 1; break; - case 0: - switch (option_index) { - case 0: /* date */ - date = 1; - break; - default: - usage(argv); - } + case OPT_date: + date = 1; break; default: usage(argv); diff --git a/trace-stack.c b/trace-stack.c index 3d9f392..24ba194 100644 --- a/trace-stack.c +++ b/trace-stack.c @@ -153,6 +153,10 @@ static void read_trace(void) fclose(fp); } +#define OPT_start 255 +#define OPT_stop 254 +#define OPT_reset 253 + void trace_stack (int argc, char **argv) { enum stack_type trace_type = STACK_REPORT; @@ -167,9 +171,9 @@ void trace_stack (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"start", no_argument, NULL, 0}, - {"stop", no_argument, NULL, 0}, - {"reset", no_argument, NULL, 0}, + {"start", no_argument, NULL, OPT_start}, + {"stop", no_argument, NULL, OPT_stop}, + {"reset", no_argument, NULL, OPT_reset}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -183,20 +187,14 @@ void trace_stack (int argc, char **argv) case 'h': usage(argv); break; - case 0: - switch(option_index) { - case 0: - trace_type = STACK_START; - break; - case 1: - trace_type = STACK_STOP; - break; - case 2: - trace_type = STACK_RESET; - break; - default: - usage(argv); - } + case OPT_start: + trace_type = STACK_START; + break; + case OPT_stop: + trace_type = STACK_STOP; + break; + case OPT_reset: + trace_type = STACK_RESET; break; default: usage(argv); -- 1.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3] trace-cmd: fold option parsing 2011-03-10 9:58 [PATCH 3/3] trace-cmd: fold option parsing Lai Jiangshan @ 2011-03-10 13:16 ` Steven Rostedt 2011-03-14 2:42 ` [PATCH 3/3 V2] " Lai Jiangshan 0 siblings, 1 reply; 4+ messages in thread From: Steven Rostedt @ 2011-03-10 13:16 UTC (permalink / raw) To: Lai Jiangshan; +Cc: LKML On Thu, 2011-03-10 at 17:58 +0800, Lai Jiangshan wrote: > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> Thanks! > > +#define OPT_debug 255 > + > > +#define OPT_cpu 255 > +#define OPT_events 254 > +#define OPT_kallsyms 253 > + I wonder if it would look cleaner if we converted them all to enums: enum { OPT_kallsyms = 253, OPT_events = 254, OPT_cpu = 255, }; As in the kernel, defines look ugly, and can have side-effects. -- Steve ^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 3/3 V2] trace-cmd: fold option parsing 2011-03-10 13:16 ` Steven Rostedt @ 2011-03-14 2:42 ` Lai Jiangshan 2011-03-14 13:29 ` Steven Rostedt 0 siblings, 1 reply; 4+ messages in thread From: Lai Jiangshan @ 2011-03-14 2:42 UTC (permalink / raw) To: Steven Rostedt; +Cc: LKML Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> --- trace-listen.c | 14 +++++++------- trace-read.c | 37 +++++++++++++++++-------------------- trace-record.c | 16 +++++++--------- trace-stack.c | 34 +++++++++++++++++----------------- 4 files changed, 48 insertions(+), 53 deletions(-) diff --git a/trace-listen.c b/trace-listen.c index e60c96b..e1cf9ea 100644 --- a/trace-listen.c +++ b/trace-listen.c @@ -647,6 +647,10 @@ static void start_daemon(void) die("starting daemon"); } +enum { + OPT_debug = 128, +}; + void trace_listen(int argc, char **argv) { char *logfile = NULL; @@ -666,7 +670,7 @@ void trace_listen(int argc, char **argv) static struct option long_options[] = { {"port", required_argument, NULL, 'p'}, {"help", no_argument, NULL, '?'}, - {"debug", no_argument, NULL, 0}, + {"debug", no_argument, NULL, OPT_debug}, {NULL, 0, NULL, 0} }; @@ -696,12 +700,8 @@ void trace_listen(int argc, char **argv) case 'D': daemon = 1; break; - case 0: - switch (option_index) { - case 2: - debug = 1; - break; - } + case OPT_debug: + debug = 1; break; default: usage(argv); diff --git a/trace-read.c b/trace-read.c index 63e2940..dfd945e 100644 --- a/trace-read.c +++ b/trace-read.c @@ -815,6 +815,12 @@ static void add_functions(struct pevent *pevent, const char *file) free(buf); } +enum { + OPT_cpu = 128, + OPT_events, + OPT_kallsyms, +}; + void trace_report (int argc, char **argv) { struct tracecmd_input *handle; @@ -848,10 +854,10 @@ void trace_report (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"cpu", required_argument, NULL, 0}, - {"events", no_argument, NULL, 0}, + {"cpu", required_argument, NULL, OPT_cpu}, + {"events", no_argument, NULL, OPT_events}, {"filter-test", no_argument, NULL, 'T'}, - {"kallsyms", required_argument, NULL, 0}, + {"kallsyms", required_argument, NULL, OPT_kallsyms}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -920,23 +926,14 @@ void trace_report (int argc, char **argv) case 'q': silence_warnings = 1; break; - case 0: - switch(option_index) { - case 0: /* cpu */ - parse_cpulist(optarg); - break; - case 1: /* events */ - print_events = 1; - break; - case 2: /* filter-test */ - test_filters = 1; - break; - case 3: /* kallsyms */ - functions = optarg; - break; - default: - usage(argv); - } + case OPT_cpu: + parse_cpulist(optarg); + break; + case OPT_events: + print_events = 1; + break; + case OPT_kallsyms: + functions = optarg; break; default: usage(argv); diff --git a/trace-record.c b/trace-record.c index 2611bfa..dd1f18c 100644 --- a/trace-record.c +++ b/trace-record.c @@ -1809,6 +1809,10 @@ static void record_all_events(void) listed_events = list; } +enum { + OPT_date = 128, +}; + void trace_record (int argc, char **argv) { const char *plugin = NULL; @@ -1862,7 +1866,7 @@ void trace_record (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"date", no_argument, NULL, 0}, + {"date", no_argument, NULL, OPT_date}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -2002,14 +2006,8 @@ void trace_record (int argc, char **argv) case 'i': ignore_event_not_found = 1; break; - case 0: - switch (option_index) { - case 0: /* date */ - date = 1; - break; - default: - usage(argv); - } + case OPT_date: + date = 1; break; default: usage(argv); diff --git a/trace-stack.c b/trace-stack.c index 3d9f392..52c30de 100644 --- a/trace-stack.c +++ b/trace-stack.c @@ -153,6 +153,12 @@ static void read_trace(void) fclose(fp); } +enum { + OPT_start = 128, + OPT_stop, + OPT_reset, +}; + void trace_stack (int argc, char **argv) { enum stack_type trace_type = STACK_REPORT; @@ -167,9 +173,9 @@ void trace_stack (int argc, char **argv) for (;;) { int option_index = 0; static struct option long_options[] = { - {"start", no_argument, NULL, 0}, - {"stop", no_argument, NULL, 0}, - {"reset", no_argument, NULL, 0}, + {"start", no_argument, NULL, OPT_start}, + {"stop", no_argument, NULL, OPT_stop}, + {"reset", no_argument, NULL, OPT_reset}, {"help", no_argument, NULL, '?'}, {NULL, 0, NULL, 0} }; @@ -183,20 +189,14 @@ void trace_stack (int argc, char **argv) case 'h': usage(argv); break; - case 0: - switch(option_index) { - case 0: - trace_type = STACK_START; - break; - case 1: - trace_type = STACK_STOP; - break; - case 2: - trace_type = STACK_RESET; - break; - default: - usage(argv); - } + case OPT_start: + trace_type = STACK_START; + break; + case OPT_stop: + trace_type = STACK_STOP; + break; + case OPT_reset: + trace_type = STACK_RESET; break; default: usage(argv); -- 1.7.4 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 3/3 V2] trace-cmd: fold option parsing 2011-03-14 2:42 ` [PATCH 3/3 V2] " Lai Jiangshan @ 2011-03-14 13:29 ` Steven Rostedt 0 siblings, 0 replies; 4+ messages in thread From: Steven Rostedt @ 2011-03-14 13:29 UTC (permalink / raw) To: Lai Jiangshan; +Cc: LKML On Mon, 2011-03-14 at 10:42 +0800, Lai Jiangshan wrote: > Signed-off-by: Lai Jiangshan <laijs@cn.fujitsu.com> > --- > trace-listen.c | 14 +++++++------- > trace-read.c | 37 +++++++++++++++++-------------------- > trace-record.c | 16 +++++++--------- > trace-stack.c | 34 +++++++++++++++++----------------- > 4 files changed, 48 insertions(+), 53 deletions(-) Thanks Lai, But I already applied your original changes, and then added a patch to convert the defines into enums. -- Steve > > diff --git a/trace-listen.c b/trace-listen.c > index e60c96b..e1cf9ea 100644 > --- a/trace-listen.c > +++ b/trace-listen.c > @@ -647,6 +647,10 @@ static void start_daemon(void) > die("starting daemon"); > } > > +enum { > + OPT_debug = 128, > +}; > + > void trace_listen(int argc, char **argv) > { > char *logfile = NULL; > @@ -666,7 +670,7 @@ void trace_listen(int argc, char **argv) > static struct option long_options[] = { > {"port", required_argument, NULL, 'p'}, > {"help", no_argument, NULL, '?'}, > - {"debug", no_argument, NULL, 0}, > + {"debug", no_argument, NULL, OPT_debug}, > {NULL, 0, NULL, 0} > }; > > @@ -696,12 +700,8 @@ void trace_listen(int argc, char **argv) > case 'D': > daemon = 1; > break; > - case 0: > - switch (option_index) { > - case 2: > - debug = 1; > - break; > - } > + case OPT_debug: > + debug = 1; > break; > default: > usage(argv); > diff --git a/trace-read.c b/trace-read.c > index 63e2940..dfd945e 100644 > --- a/trace-read.c > +++ b/trace-read.c > @@ -815,6 +815,12 @@ static void add_functions(struct pevent *pevent, const char *file) > free(buf); > } > > +enum { > + OPT_cpu = 128, > + OPT_events, > + OPT_kallsyms, > +}; > + > void trace_report (int argc, char **argv) > { > struct tracecmd_input *handle; > @@ -848,10 +854,10 @@ void trace_report (int argc, char **argv) > for (;;) { > int option_index = 0; > static struct option long_options[] = { > - {"cpu", required_argument, NULL, 0}, > - {"events", no_argument, NULL, 0}, > + {"cpu", required_argument, NULL, OPT_cpu}, > + {"events", no_argument, NULL, OPT_events}, > {"filter-test", no_argument, NULL, 'T'}, > - {"kallsyms", required_argument, NULL, 0}, > + {"kallsyms", required_argument, NULL, OPT_kallsyms}, > {"help", no_argument, NULL, '?'}, > {NULL, 0, NULL, 0} > }; > @@ -920,23 +926,14 @@ void trace_report (int argc, char **argv) > case 'q': > silence_warnings = 1; > break; > - case 0: > - switch(option_index) { > - case 0: /* cpu */ > - parse_cpulist(optarg); > - break; > - case 1: /* events */ > - print_events = 1; > - break; > - case 2: /* filter-test */ > - test_filters = 1; > - break; > - case 3: /* kallsyms */ > - functions = optarg; > - break; > - default: > - usage(argv); > - } > + case OPT_cpu: > + parse_cpulist(optarg); > + break; > + case OPT_events: > + print_events = 1; > + break; > + case OPT_kallsyms: > + functions = optarg; > break; > default: > usage(argv); > diff --git a/trace-record.c b/trace-record.c > index 2611bfa..dd1f18c 100644 > --- a/trace-record.c > +++ b/trace-record.c > @@ -1809,6 +1809,10 @@ static void record_all_events(void) > listed_events = list; > } > > +enum { > + OPT_date = 128, > +}; > + > void trace_record (int argc, char **argv) > { > const char *plugin = NULL; > @@ -1862,7 +1866,7 @@ void trace_record (int argc, char **argv) > for (;;) { > int option_index = 0; > static struct option long_options[] = { > - {"date", no_argument, NULL, 0}, > + {"date", no_argument, NULL, OPT_date}, > {"help", no_argument, NULL, '?'}, > {NULL, 0, NULL, 0} > }; > @@ -2002,14 +2006,8 @@ void trace_record (int argc, char **argv) > case 'i': > ignore_event_not_found = 1; > break; > - case 0: > - switch (option_index) { > - case 0: /* date */ > - date = 1; > - break; > - default: > - usage(argv); > - } > + case OPT_date: > + date = 1; > break; > default: > usage(argv); > diff --git a/trace-stack.c b/trace-stack.c > index 3d9f392..52c30de 100644 > --- a/trace-stack.c > +++ b/trace-stack.c > @@ -153,6 +153,12 @@ static void read_trace(void) > fclose(fp); > } > > +enum { > + OPT_start = 128, > + OPT_stop, > + OPT_reset, > +}; > + > void trace_stack (int argc, char **argv) > { > enum stack_type trace_type = STACK_REPORT; > @@ -167,9 +173,9 @@ void trace_stack (int argc, char **argv) > for (;;) { > int option_index = 0; > static struct option long_options[] = { > - {"start", no_argument, NULL, 0}, > - {"stop", no_argument, NULL, 0}, > - {"reset", no_argument, NULL, 0}, > + {"start", no_argument, NULL, OPT_start}, > + {"stop", no_argument, NULL, OPT_stop}, > + {"reset", no_argument, NULL, OPT_reset}, > {"help", no_argument, NULL, '?'}, > {NULL, 0, NULL, 0} > }; > @@ -183,20 +189,14 @@ void trace_stack (int argc, char **argv) > case 'h': > usage(argv); > break; > - case 0: > - switch(option_index) { > - case 0: > - trace_type = STACK_START; > - break; > - case 1: > - trace_type = STACK_STOP; > - break; > - case 2: > - trace_type = STACK_RESET; > - break; > - default: > - usage(argv); > - } > + case OPT_start: > + trace_type = STACK_START; > + break; > + case OPT_stop: > + trace_type = STACK_STOP; > + break; > + case OPT_reset: > + trace_type = STACK_RESET; > break; > default: > usage(argv); ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-14 13:29 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-03-10 9:58 [PATCH 3/3] trace-cmd: fold option parsing Lai Jiangshan 2011-03-10 13:16 ` Steven Rostedt 2011-03-14 2:42 ` [PATCH 3/3 V2] " Lai Jiangshan 2011-03-14 13:29 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox