Skip to contentSkip to conversation

trace-cmd: correct cpu list parsing

PATCH Superseded Lai Jiangshan 2 participants 2 messages Mar 10, 2011

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Lai Jiangshan

author
#0 lore raw

PATCH 2/3 trace-cmd: correct cpu list parsing

Old parsing code is incorrect, when we use execute ./trace-cmd report --cpu 1,2-4,6,9-13

the result of filter_cpus will be:
1,2,1,2,3,4,6,9,5,6,7,8,9,10,11,12,13

The correct filter_cpus should be:
1,2,3,4,6,9,10,11,12,13,

I can change about 10 existed lines to make it correct, but it will be still bad readability, so I port the neat and good readability code:
linux_kernel_src/lib/bitmap.c:bitmap_parselist()
to here and replace the origin code

Signed-off-by Lai Jiangshan <[email protected]>
---
 trace-read.c |   76 ++++++++++++++-------------------------------------------
 1 files changed, 19 insertions(+), 57 deletions(-)
trace-read.c +19 -57
index 1cd9bd3..932f5aa 100644
@@ -738,9 +738,8 @@ static void sig_end(int sig)
 	exit(0);
 }
 
-static const char *inc_and_test_char(const char *p, const char *cpu_str)
+static const char *skip_space_and_test_digit(const char *p, const char *cpu_str)
 {
-	p++;
 	while (isspace(*p))
 		p++;
 	if (!isdigit(*p))

@@ -754,64 +753,27 @@ static void __add_cpu(int cpu)
 	filter_cpus = tracecmd_add_id(filter_cpus, cpu, nr_filter_cpus++);
 }
 
-static int process_cpu_str(const char *cpu_str)
+static void parse_cpulist(const char *cpu_str)
 {
-	const char *p = cpu_str;
-	int cpu, ncpu, ret_cpu = 1;
+	unsigned a, b;
+	const char *s = cpu_str;
 
 	do {
-		while (isspace(*p))
-			p++;
-
-		cpu = atoi(p);
-		__add_cpu(cpu);
-
- again:
-		while (isdigit(*p))
-			p++;
-		while (isspace(*p))
-			p++;
-
-		if (*p) {
-			ret_cpu = 0;
-			switch (*p) {
-			case '-':
-				p = inc_and_test_char(p, cpu_str);
-				ncpu = atoi(p);
-				if (ncpu < cpu)
-					die("range of cpu numbers must be lower to greater");
-				for (; cpu <= ncpu; cpu++)
-					__add_cpu(cpu);
-				break;
-
-			case ',':
-			case ':':
-				p = inc_and_test_char(p, cpu_str);
-				ncpu = atoi(p);
-				__add_cpu(ncpu);
-				break;
-			default:
-				die("invalid character '%c' in cpu string '%s'",
-				    *p, cpu_str);
-			}
-			goto again;
+		s = skip_space_and_test_digit(s, cpu_str);
+		b = a = strtoul(s, (char **)&s, 10);
+		if (*s == '-') {
+			s = skip_space_and_test_digit(s + 1, cpu_str);
+			b = strtoul(s, (char **)&s, 10);
 		}
-	} while (*p);
-
-	if (ret_cpu)
-		return cpu;
-
-	/* Return -1 if we added more than one CPU */
-	return -1;
-}
-
-static void add_cpu(const char *cpu_str)
-{
-	int cpu;
-
-	cpu = process_cpu_str(cpu_str);
-	if (cpu >= 0)
-		__add_cpu(cpu);
+		if (!(a <= b))
+			die("range of cpu numbers must be lower to greater");
+		while (a <= b) {
+			__add_cpu(a);
+			a++;
+		}
+		if (*s == ',' || *s == ':')
+			s++;
+	} while (*s != '\0');
 }
 
 static void read_file_fd(int fd, char *dst, int len)

@@ -958,7 +920,7 @@ void trace_report (int argc, char **argv)
 		case 0:
 			switch(option_index) {
 			case 0: /* cpu */
-				add_cpu(optarg);
+				parse_cpulist(optarg);
 				break;
 			case 1: /* events */
 				print_events = 1;
Signature · 1 line
1.7.4
#1 lore raw
Quoted 6 lines · Lai Jiangshan

Old parsing code is incorrect, when we use execute ./trace-cmd report --cpu 1,2-4,6,9-13

the result of filter_cpus will be:
1,2,1,2,3,4,6,9,5,6,7,8,9,10,11,12,13

Quoted 7 lines

The correct filter_cpus should be:
1,2,3,4,6,9,10,11,12,13,

I can change about 10 existed lines to make it correct, but it will be still bad readability, so I port the neat and good readability code:
linux_kernel_src/lib/bitmap.c:bitmap_parselist()
to here and replace the origin code

Yeah that code never got much love. I added it as a hack as the first filter to use. And never really spent much time in fixing it. As it goes, "if it works for what I need it for, it's good enough" ;)

-- Steve