Skip to contentSkip to conversation

trace-cmd: fold option parsing

PATCH Superseded Lai Jiangshan 2 participants 4 messages Mar 10, 2011 – Mar 14, 2011

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Lai Jiangshan

author
#0 lore raw

PATCH 3/3 trace-cmd: fold option parsing

Signed-off-by Lai Jiangshan <[email protected]>
---
 trace-listen.c |   12 +++++-------
 trace-read.c   |   35 +++++++++++++++--------------------
 trace-record.c |   14 +++++---------
 trace-stack.c  |   32 +++++++++++++++-----------------
 4 files changed, 40 insertions(+), 53 deletions(-)
trace-listen.c +5 -7
index e60c96b..daaffca 100644
@@ -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);
trace-read.c +15 -20
index 932f5aa..018476d 100644
@@ -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);
trace-record.c +5 -9
index 2611bfa..1cf13b6 100644
@@ -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);
trace-stack.c +15 -17
index 3d9f392..24ba194 100644
@@ -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);
Signature · 1 line
1.7.4
#1 lore raw
Quoted 1 line · Lai Jiangshan
Signed-off-by Lai Jiangshan <[email protected]>

Thanks!

trace-read.c static void add_functions(struct pevent *pevent, const char *file) · line 818

Quoted 9 lines
trace-read.c +6 -0
+#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

author
#2 lore raw

PATCH v2 3/3 trace-cmd: fold option parsing

Signed-off-by Lai Jiangshan <[email protected]>
---
 trace-listen.c |   14 +++++++-------
 trace-read.c   |   37 +++++++++++++++++--------------------
 trace-record.c |   16 +++++++---------
 trace-stack.c  |   34 +++++++++++++++++-----------------
 4 files changed, 48 insertions(+), 53 deletions(-)
trace-listen.c +7 -7
index e60c96b..e1cf9ea 100644
@@ -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);
trace-read.c +17 -20
index 63e2940..dfd945e 100644
@@ -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);
trace-record.c +7 -9
index 2611bfa..dd1f18c 100644
@@ -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);
trace-stack.c +17 -17
index 3d9f392..52c30de 100644
@@ -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);
Signature · 1 line
1.7.4
#3 lore raw
Quoted 7 lines · Lai Jiangshan
Signed-off-by Lai Jiangshan <[email protected]>
---
 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

Quoted 203 lines
trace-listen.c +7 -7
index e60c96b..e1cf9ea 100644
@@ -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);
trace-read.c +17 -20
index 63e2940..dfd945e 100644
@@ -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);
trace-record.c +7 -9
index 2611bfa..dd1f18c 100644
@@ -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);
trace-stack.c +17 -17
index 3d9f392..52c30de 100644
@@ -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);