* [PATCH 1/4] perf options: Introduce OPT_UINTEGER
2010-05-17 19:31 [GIT PULL 0/4] perf options type checking Arnaldo Carvalho de Melo
@ 2010-05-17 19:31 ` Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 2/4] perf options: Check v type in OPT_U?INTEGER Arnaldo Carvalho de Melo
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-05-17 19:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
For unsigned int options to be parsed, next patches will make use of it.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/util/parse-options.c | 22 ++++++++++++++++++++--
tools/perf/util/parse-options.h | 2 ++
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index b05d51d..36d955e 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -59,6 +59,7 @@ static int get_value(struct parse_opt_ctx_t *p,
case OPTION_GROUP:
case OPTION_STRING:
case OPTION_INTEGER:
+ case OPTION_UINTEGER:
case OPTION_LONG:
case OPTION_U64:
default:
@@ -126,6 +127,22 @@ static int get_value(struct parse_opt_ctx_t *p,
return opterror(opt, "expects a numerical value", flags);
return 0;
+ case OPTION_UINTEGER:
+ if (unset) {
+ *(unsigned int *)opt->value = 0;
+ return 0;
+ }
+ if (opt->flags & PARSE_OPT_OPTARG && !p->opt) {
+ *(unsigned int *)opt->value = opt->defval;
+ return 0;
+ }
+ if (get_arg(p, opt, flags, &arg))
+ return -1;
+ *(unsigned int *)opt->value = strtol(arg, (char **)&s, 10);
+ if (*s)
+ return opterror(opt, "expects a numerical value", flags);
+ return 0;
+
case OPTION_LONG:
if (unset) {
*(long *)opt->value = 0;
@@ -463,7 +480,10 @@ int usage_with_options_internal(const char * const *usagestr,
switch (opts->type) {
case OPTION_ARGUMENT:
break;
+ case OPTION_LONG:
+ case OPTION_U64:
case OPTION_INTEGER:
+ case OPTION_UINTEGER:
if (opts->flags & PARSE_OPT_OPTARG)
if (opts->long_name)
pos += fprintf(stderr, "[=<n>]");
@@ -503,8 +523,6 @@ int usage_with_options_internal(const char * const *usagestr,
case OPTION_INCR:
case OPTION_SET_INT:
case OPTION_SET_PTR:
- case OPTION_LONG:
- case OPTION_U64:
break;
}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index e301e96..c6aa4ee 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -18,6 +18,7 @@ enum parse_opt_type {
OPTION_LONG,
OPTION_CALLBACK,
OPTION_U64,
+ OPTION_UINTEGER,
};
enum parse_opt_flags {
@@ -101,6 +102,7 @@ struct option {
#define OPT_SET_INT(s, l, v, h, i) { .type = OPTION_SET_INT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (i) }
#define OPT_SET_PTR(s, l, v, h, p) { .type = OPTION_SET_PTR, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (p) }
#define OPT_INTEGER(s, l, v, h) { .type = OPTION_INTEGER, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
+#define OPT_UINTEGER(s, l, v, h) { .type = OPTION_UINTEGER, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_LONG(s, l, v, h) { .type = OPTION_LONG, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_U64(s, l, v, h) { .type = OPTION_U64, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_STRING(s, l, v, a, h) { .type = OPTION_STRING, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h) }
--
1.6.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 2/4] perf options: Check v type in OPT_U?INTEGER
2010-05-17 19:31 [GIT PULL 0/4] perf options type checking Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 1/4] perf options: Introduce OPT_UINTEGER Arnaldo Carvalho de Melo
@ 2010-05-17 19:31 ` Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 3/4] perf options: Type check OPT_BOOLEAN and fix the offenders Arnaldo Carvalho de Melo
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-05-17 19:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
To avoid problems like the one fixed by Stephane Eranian in 3de29ca, now
we'll got this instead:
bench/sched-messaging.c:259: error: negative width in bit-field ‘<anonymous>’
bench/sched-messaging.c:261: error: negative width in bit-field ‘<anonymous>’
Which is rather cryptic, but is how BUILD_BUG_ON_ZERO works, so kernel
hackers should be already used to this.
With it in place found some problems, fixed by changing the affected
variables to sensible types or changed some OPT_INTEGER to OPT_UINTEGER.
Next csets will go thru converting each of the remaining OPT_ so that
review can be made easier by grouping changes per type per patch.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/bench/sched-messaging.c | 6 ++----
tools/perf/builtin-record.c | 8 +++-----
tools/perf/builtin-sched.c | 6 +++---
tools/perf/builtin-top.c | 5 ++---
tools/perf/util/include/linux/kernel.h | 2 ++
tools/perf/util/parse-options.h | 8 ++++++--
6 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/tools/perf/bench/sched-messaging.c b/tools/perf/bench/sched-messaging.c
index da1b2e9..d1d1b30 100644
--- a/tools/perf/bench/sched-messaging.c
+++ b/tools/perf/bench/sched-messaging.c
@@ -256,10 +256,8 @@ static const struct option options[] = {
"Use pipe() instead of socketpair()"),
OPT_BOOLEAN('t', "thread", &thread_mode,
"Be multi thread instead of multi process"),
- OPT_INTEGER('g', "group", &num_groups,
- "Specify number of groups"),
- OPT_INTEGER('l', "loop", &loops,
- "Specify number of loops"),
+ OPT_UINTEGER('g', "group", &num_groups, "Specify number of groups"),
+ OPT_UINTEGER('l', "loop", &loops, "Specify number of loops"),
OPT_END()
};
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b93573c..cb46c7d 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -45,7 +45,7 @@ static int output;
static int pipe_output = 0;
static const char *output_name = "perf.data";
static int group = 0;
-static unsigned int realtime_prio = 0;
+static int realtime_prio = 0;
static bool raw_samples = false;
static bool system_wide = false;
static int profile_cpu = -1;
@@ -822,10 +822,8 @@ static const struct option options[] = {
"output file name"),
OPT_BOOLEAN('i', "no-inherit", &no_inherit,
"child tasks do not inherit counters"),
- OPT_INTEGER('F', "freq", &user_freq,
- "profile at this frequency"),
- OPT_INTEGER('m', "mmap-pages", &mmap_pages,
- "number of mmap data pages"),
+ OPT_UINTEGER('F', "freq", &user_freq, "profile at this frequency"),
+ OPT_UINTEGER('m', "mmap-pages", &mmap_pages, "number of mmap data pages"),
OPT_BOOLEAN('g', "call-graph", &call_graph,
"do call-graph (stack chain/backtrace) recording"),
OPT_INCR('v', "verbose", &verbose,
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index be7bc92..c80acdf 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -105,7 +105,7 @@ static u64 sum_runtime;
static u64 sum_fluct;
static u64 run_avg;
-static unsigned long replay_repeat = 10;
+static unsigned int replay_repeat = 10;
static unsigned long nr_timestamps;
static unsigned long nr_unordered_timestamps;
static unsigned long nr_state_machine_bugs;
@@ -1816,8 +1816,8 @@ static const char * const replay_usage[] = {
};
static const struct option replay_options[] = {
- OPT_INTEGER('r', "repeat", &replay_repeat,
- "repeat the workload replay N times (-1: infinite)"),
+ OPT_UINTEGER('r', "repeat", &replay_repeat,
+ "repeat the workload replay N times (-1: infinite)"),
OPT_INCR('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
OPT_BOOLEAN('D', "dump-raw-trace", &dump_trace,
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ed9b5b6..9f0cfa0 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -71,7 +71,7 @@ static int thread_num = 0;
static bool inherit = false;
static int profile_cpu = -1;
static int nr_cpus = 0;
-static unsigned int realtime_prio = 0;
+static int realtime_prio = 0;
static bool group = false;
static unsigned int page_size;
static unsigned int mmap_pages = 16;
@@ -1357,8 +1357,7 @@ static const struct option options[] = {
"file", "vmlinux pathname"),
OPT_BOOLEAN('K', "hide_kernel_symbols", &hide_kernel_symbols,
"hide kernel symbols"),
- OPT_INTEGER('m', "mmap-pages", &mmap_pages,
- "number of mmap data pages"),
+ OPT_UINTEGER('m', "mmap-pages", &mmap_pages, "number of mmap data pages"),
OPT_INTEGER('r', "realtime", &realtime_prio,
"collect data with this RT SCHED_FIFO priority"),
OPT_INTEGER('d', "delay", &delay_secs,
diff --git a/tools/perf/util/include/linux/kernel.h b/tools/perf/util/include/linux/kernel.h
index 388ab1b..1eb804f 100644
--- a/tools/perf/util/include/linux/kernel.h
+++ b/tools/perf/util/include/linux/kernel.h
@@ -28,6 +28,8 @@
(type *)((char *)__mptr - offsetof(type, member)); })
#endif
+#define BUILD_BUG_ON_ZERO(e) (sizeof(struct { int:-!!(e); }))
+
#ifndef max
#define max(x, y) ({ \
typeof(x) _max1 = (x); \
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index c6aa4ee..9ca348e 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -1,6 +1,8 @@
#ifndef __PERF_PARSE_OPTIONS_H
#define __PERF_PARSE_OPTIONS_H
+#include <linux/kernel.h>
+
enum parse_opt_type {
/* special types */
OPTION_END,
@@ -93,6 +95,8 @@ struct option {
intptr_t defval;
};
+#define check_vtype(v, type) ( BUILD_BUG_ON_ZERO(!__builtin_types_compatible_p(typeof(v), type)) + v )
+
#define OPT_END() { .type = OPTION_END }
#define OPT_ARGUMENT(l, h) { .type = OPTION_ARGUMENT, .long_name = (l), .help = (h) }
#define OPT_GROUP(h) { .type = OPTION_GROUP, .help = (h) }
@@ -101,8 +105,8 @@ struct option {
#define OPT_INCR(s, l, v, h) { .type = OPTION_INCR, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_SET_INT(s, l, v, h, i) { .type = OPTION_SET_INT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (i) }
#define OPT_SET_PTR(s, l, v, h, p) { .type = OPTION_SET_PTR, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (p) }
-#define OPT_INTEGER(s, l, v, h) { .type = OPTION_INTEGER, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
-#define OPT_UINTEGER(s, l, v, h) { .type = OPTION_UINTEGER, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
+#define OPT_INTEGER(s, l, v, h) { .type = OPTION_INTEGER, .short_name = (s), .long_name = (l), .value = check_vtype(v, int *), .help = (h) }
+#define OPT_UINTEGER(s, l, v, h) { .type = OPTION_UINTEGER, .short_name = (s), .long_name = (l), .value = check_vtype(v, unsigned int *), .help = (h) }
#define OPT_LONG(s, l, v, h) { .type = OPTION_LONG, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_U64(s, l, v, h) { .type = OPTION_U64, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_STRING(s, l, v, a, h) { .type = OPTION_STRING, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h) }
--
1.6.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 3/4] perf options: Type check OPT_BOOLEAN and fix the offenders
2010-05-17 19:31 [GIT PULL 0/4] perf options type checking Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 1/4] perf options: Introduce OPT_UINTEGER Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 2/4] perf options: Check v type in OPT_U?INTEGER Arnaldo Carvalho de Melo
@ 2010-05-17 19:31 ` Arnaldo Carvalho de Melo
2010-05-17 19:31 ` [PATCH 4/4] perf options: Type check all the remaining OPT_ variants Arnaldo Carvalho de Melo
2010-05-17 23:29 ` [GIT PULL 0/4] perf options type checking Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-05-17 19:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-kvm.c | 4 ++--
tools/perf/builtin-lock.c | 3 +--
tools/perf/builtin-test.c | 2 +-
tools/perf/perf.h | 3 ++-
tools/perf/util/parse-options.h | 3 ++-
5 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index a4c7cae..b1c6b38 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -22,8 +22,8 @@
static char *file_name;
static char name_buffer[256];
-int perf_host = 1;
-int perf_guest;
+bool perf_host = 1;
+bool perf_guest;
static const char * const kvm_usage[] = {
"perf kvm [<options>] {top|record|report|diff|buildid-list}",
diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index e18dfdc..821c158 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -792,8 +792,7 @@ static void print_result(void)
print_bad_events(bad, total);
}
-static int info_threads;
-static int info_map;
+static bool info_threads, info_map;
static void dump_threads(void)
{
diff --git a/tools/perf/builtin-test.c b/tools/perf/builtin-test.c
index 0339612..035b9fa 100644
--- a/tools/perf/builtin-test.c
+++ b/tools/perf/builtin-test.c
@@ -257,7 +257,7 @@ static const char * const test_usage[] = {
};
static const struct option test_options[] = {
- OPT_BOOLEAN('v', "verbose", &verbose,
+ OPT_INTEGER('v', "verbose", &verbose,
"be more verbose (show symbol address, etc)"),
OPT_END()
};
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 02821fe..ef7aa0a 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -80,6 +80,7 @@ void get_term_dimensions(struct winsize *ws);
#include "../../include/linux/perf_event.h"
#include "util/types.h"
+#include <stdbool.h>
/*
* prctl(PR_TASK_PERF_EVENTS_DISABLE) will (cheaply) disable all
@@ -131,6 +132,6 @@ struct ip_callchain {
u64 ips[0];
};
-extern int perf_host, perf_guest;
+extern bool perf_host, perf_guest;
#endif
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index 9ca348e..5838e2d 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -2,6 +2,7 @@
#define __PERF_PARSE_OPTIONS_H
#include <linux/kernel.h>
+#include <stdbool.h>
enum parse_opt_type {
/* special types */
@@ -101,7 +102,7 @@ struct option {
#define OPT_ARGUMENT(l, h) { .type = OPTION_ARGUMENT, .long_name = (l), .help = (h) }
#define OPT_GROUP(h) { .type = OPTION_GROUP, .help = (h) }
#define OPT_BIT(s, l, v, h, b) { .type = OPTION_BIT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (b) }
-#define OPT_BOOLEAN(s, l, v, h) { .type = OPTION_BOOLEAN, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
+#define OPT_BOOLEAN(s, l, v, h) { .type = OPTION_BOOLEAN, .short_name = (s), .long_name = (l), .value = check_vtype(v, bool *), .help = (h) }
#define OPT_INCR(s, l, v, h) { .type = OPTION_INCR, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
#define OPT_SET_INT(s, l, v, h, i) { .type = OPTION_SET_INT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (i) }
#define OPT_SET_PTR(s, l, v, h, p) { .type = OPTION_SET_PTR, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (p) }
--
1.6.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 4/4] perf options: Type check all the remaining OPT_ variants
2010-05-17 19:31 [GIT PULL 0/4] perf options type checking Arnaldo Carvalho de Melo
` (2 preceding siblings ...)
2010-05-17 19:31 ` [PATCH 3/4] perf options: Type check OPT_BOOLEAN and fix the offenders Arnaldo Carvalho de Melo
@ 2010-05-17 19:31 ` Arnaldo Carvalho de Melo
2010-05-17 23:29 ` [GIT PULL 0/4] perf options type checking Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2010-05-17 19:31 UTC (permalink / raw)
To: Ingo Molnar
Cc: linux-kernel, Arnaldo Carvalho de Melo,
Frédéric Weisbecker, Mike Galbraith, Paul Mackerras,
Peter Zijlstra, Stephane Eranian, Tom Zanussi
From: Arnaldo Carvalho de Melo <acme@redhat.com>
OPT_SET_INT was renamed to OPT_SET_UINT since the only use in these
tools is to set something that has an enum type, that is builtin
compatible with unsigned int.
Several string constifications were done to make OPT_STRING require a
const char * type.
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
LKML-Reference: <new-submission>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-bench.c | 4 ++--
tools/perf/builtin-help.c | 6 +++---
tools/perf/builtin-kvm.c | 2 +-
tools/perf/builtin-report.c | 4 ++--
tools/perf/builtin-sched.c | 2 +-
tools/perf/builtin-top.c | 2 +-
tools/perf/util/hist.c | 2 +-
tools/perf/util/parse-options.c | 10 +++++-----
tools/perf/util/parse-options.h | 16 ++++++++--------
tools/perf/util/sort.c | 8 ++++----
tools/perf/util/sort.h | 8 ++++----
tools/perf/util/symbol.h | 2 +-
12 files changed, 33 insertions(+), 33 deletions(-)
diff --git a/tools/perf/builtin-bench.c b/tools/perf/builtin-bench.c
index 4699677..fcb9626 100644
--- a/tools/perf/builtin-bench.c
+++ b/tools/perf/builtin-bench.c
@@ -95,7 +95,7 @@ static void dump_suites(int subsys_index)
return;
}
-static char *bench_format_str;
+static const char *bench_format_str;
int bench_format = BENCH_FORMAT_DEFAULT;
static const struct option bench_options[] = {
@@ -126,7 +126,7 @@ static void print_usage(void)
printf("\n");
}
-static int bench_str2int(char *str)
+static int bench_str2int(const char *str)
{
if (!str)
return BENCH_FORMAT_DEFAULT;
diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index 81e3ecc..6d5a8a7 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -33,10 +33,10 @@ static bool show_all = false;
static enum help_format help_format = HELP_FORMAT_MAN;
static struct option builtin_help_options[] = {
OPT_BOOLEAN('a', "all", &show_all, "print all available commands"),
- OPT_SET_INT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
- OPT_SET_INT('w', "web", &help_format, "show manual in web browser",
+ OPT_SET_UINT('m', "man", &help_format, "show man page", HELP_FORMAT_MAN),
+ OPT_SET_UINT('w', "web", &help_format, "show manual in web browser",
HELP_FORMAT_WEB),
- OPT_SET_INT('i', "info", &help_format, "show info page",
+ OPT_SET_UINT('i', "info", &help_format, "show info page",
HELP_FORMAT_INFO),
OPT_END(),
};
diff --git a/tools/perf/builtin-kvm.c b/tools/perf/builtin-kvm.c
index b1c6b38..34d1e85 100644
--- a/tools/perf/builtin-kvm.c
+++ b/tools/perf/builtin-kvm.c
@@ -19,7 +19,7 @@
#include <pthread.h>
#include <math.h>
-static char *file_name;
+static const char *file_name;
static char name_buffer[256];
bool perf_host = 1;
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 6826512..1d3c100 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -39,8 +39,8 @@ static bool dont_use_callchains;
static bool show_threads;
static struct perf_read_values show_threads_values;
-static char default_pretty_printing_style[] = "normal";
-static char *pretty_printing_style = default_pretty_printing_style;
+static const char default_pretty_printing_style[] = "normal";
+static const char *pretty_printing_style = default_pretty_printing_style;
static char callchain_default_opt[] = "fractal,0.5";
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index c80acdf..f67bce2 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -22,7 +22,7 @@
static char const *input_name = "perf.data";
static char default_sort_order[] = "avg, max, switch, runtime";
-static char *sort_order = default_sort_order;
+static const char *sort_order = default_sort_order;
static int profile_cpu = -1;
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 9f0cfa0..397290a 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -96,7 +96,7 @@ struct source_line {
struct source_line *next;
};
-static char *sym_filter = NULL;
+static const char *sym_filter = NULL;
struct sym_entry *sym_filter_entry = NULL;
struct sym_entry *sym_filter_entry_sched = NULL;
static int sym_pcnt_filter = 5;
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index f75c5f6..9a71c94 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -662,7 +662,7 @@ size_t hists__fprintf(struct hists *self, struct hists *pair,
long displacement = 0;
unsigned int width;
const char *sep = symbol_conf.field_sep;
- char *col_width = symbol_conf.col_width_list_str;
+ const char *col_width = symbol_conf.col_width_list_str;
init_rem_hits();
diff --git a/tools/perf/util/parse-options.c b/tools/perf/util/parse-options.c
index 36d955e..99d02aa 100644
--- a/tools/perf/util/parse-options.c
+++ b/tools/perf/util/parse-options.c
@@ -51,7 +51,7 @@ static int get_value(struct parse_opt_ctx_t *p,
case OPTION_BOOLEAN:
case OPTION_INCR:
case OPTION_BIT:
- case OPTION_SET_INT:
+ case OPTION_SET_UINT:
case OPTION_SET_PTR:
return opterror(opt, "takes no value", flags);
case OPTION_END:
@@ -83,8 +83,8 @@ static int get_value(struct parse_opt_ctx_t *p,
*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
return 0;
- case OPTION_SET_INT:
- *(int *)opt->value = unset ? 0 : opt->defval;
+ case OPTION_SET_UINT:
+ *(unsigned int *)opt->value = unset ? 0 : opt->defval;
return 0;
case OPTION_SET_PTR:
@@ -515,13 +515,13 @@ int usage_with_options_internal(const char * const *usagestr,
pos += fprintf(stderr, " ...");
}
break;
- default: /* OPTION_{BIT,BOOLEAN,SET_INT,SET_PTR} */
+ default: /* OPTION_{BIT,BOOLEAN,SET_UINT,SET_PTR} */
case OPTION_END:
case OPTION_GROUP:
case OPTION_BIT:
case OPTION_BOOLEAN:
case OPTION_INCR:
- case OPTION_SET_INT:
+ case OPTION_SET_UINT:
case OPTION_SET_PTR:
break;
}
diff --git a/tools/perf/util/parse-options.h b/tools/perf/util/parse-options.h
index 5838e2d..c7d72dc 100644
--- a/tools/perf/util/parse-options.h
+++ b/tools/perf/util/parse-options.h
@@ -13,7 +13,7 @@ enum parse_opt_type {
OPTION_BIT,
OPTION_BOOLEAN,
OPTION_INCR,
- OPTION_SET_INT,
+ OPTION_SET_UINT,
OPTION_SET_PTR,
/* options with arguments (usually) */
OPTION_STRING,
@@ -79,7 +79,7 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
*
* `defval`::
* default value to fill (*->value) with for PARSE_OPT_OPTARG.
- * OPTION_{BIT,SET_INT,SET_PTR} store the {mask,integer,pointer} to put in
+ * OPTION_{BIT,SET_UINT,SET_PTR} store the {mask,integer,pointer} to put in
* the value when met.
* CALLBACKS can use it like they want.
*/
@@ -101,16 +101,16 @@ struct option {
#define OPT_END() { .type = OPTION_END }
#define OPT_ARGUMENT(l, h) { .type = OPTION_ARGUMENT, .long_name = (l), .help = (h) }
#define OPT_GROUP(h) { .type = OPTION_GROUP, .help = (h) }
-#define OPT_BIT(s, l, v, h, b) { .type = OPTION_BIT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (b) }
+#define OPT_BIT(s, l, v, h, b) { .type = OPTION_BIT, .short_name = (s), .long_name = (l), .value = check_vtype(v, int *), .help = (h), .defval = (b) }
#define OPT_BOOLEAN(s, l, v, h) { .type = OPTION_BOOLEAN, .short_name = (s), .long_name = (l), .value = check_vtype(v, bool *), .help = (h) }
-#define OPT_INCR(s, l, v, h) { .type = OPTION_INCR, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
-#define OPT_SET_INT(s, l, v, h, i) { .type = OPTION_SET_INT, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (i) }
+#define OPT_INCR(s, l, v, h) { .type = OPTION_INCR, .short_name = (s), .long_name = (l), .value = check_vtype(v, int *), .help = (h) }
+#define OPT_SET_UINT(s, l, v, h, i) { .type = OPTION_SET_UINT, .short_name = (s), .long_name = (l), .value = check_vtype(v, unsigned int *), .help = (h), .defval = (i) }
#define OPT_SET_PTR(s, l, v, h, p) { .type = OPTION_SET_PTR, .short_name = (s), .long_name = (l), .value = (v), .help = (h), .defval = (p) }
#define OPT_INTEGER(s, l, v, h) { .type = OPTION_INTEGER, .short_name = (s), .long_name = (l), .value = check_vtype(v, int *), .help = (h) }
#define OPT_UINTEGER(s, l, v, h) { .type = OPTION_UINTEGER, .short_name = (s), .long_name = (l), .value = check_vtype(v, unsigned int *), .help = (h) }
-#define OPT_LONG(s, l, v, h) { .type = OPTION_LONG, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
-#define OPT_U64(s, l, v, h) { .type = OPTION_U64, .short_name = (s), .long_name = (l), .value = (v), .help = (h) }
-#define OPT_STRING(s, l, v, a, h) { .type = OPTION_STRING, .short_name = (s), .long_name = (l), .value = (v), (a), .help = (h) }
+#define OPT_LONG(s, l, v, h) { .type = OPTION_LONG, .short_name = (s), .long_name = (l), .value = check_vtype(v, long *), .help = (h) }
+#define OPT_U64(s, l, v, h) { .type = OPTION_U64, .short_name = (s), .long_name = (l), .value = check_vtype(v, u64 *), .help = (h) }
+#define OPT_STRING(s, l, v, a, h) { .type = OPTION_STRING, .short_name = (s), .long_name = (l), .value = check_vtype(v, const char **), (a), .help = (h) }
#define OPT_DATE(s, l, v, h) \
{ .type = OPTION_CALLBACK, .short_name = (s), .long_name = (l), .value = (v), .argh = "time", .help = (h), .callback = parse_opt_approxidate_cb }
#define OPT_CALLBACK(s, l, v, a, h, f) \
diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c
index da30b30..2316cb5 100644
--- a/tools/perf/util/sort.c
+++ b/tools/perf/util/sort.c
@@ -1,10 +1,10 @@
#include "sort.h"
regex_t parent_regex;
-char default_parent_pattern[] = "^sys_|^do_page_fault";
-char *parent_pattern = default_parent_pattern;
-char default_sort_order[] = "comm,dso,symbol";
-char *sort_order = default_sort_order;
+const char default_parent_pattern[] = "^sys_|^do_page_fault";
+const char *parent_pattern = default_parent_pattern;
+const char default_sort_order[] = "comm,dso,symbol";
+const char *sort_order = default_sort_order;
int sort__need_collapse = 0;
int sort__has_parent = 0;
diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h
index eab2e0b..0d61c40 100644
--- a/tools/perf/util/sort.h
+++ b/tools/perf/util/sort.h
@@ -25,10 +25,10 @@
#include "sort.h"
extern regex_t parent_regex;
-extern char *sort_order;
-extern char default_parent_pattern[];
-extern char *parent_pattern;
-extern char default_sort_order[];
+extern const char *sort_order;
+extern const char default_parent_pattern[];
+extern const char *parent_pattern;
+extern const char default_sort_order[];
extern int sort__need_collapse;
extern int sort__has_parent;
extern char *field_sep;
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index 6389d1a..032469e 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -78,7 +78,7 @@ struct symbol_conf {
*default_guest_kallsyms,
*default_guest_modules;
const char *guestmount;
- char *dso_list_str,
+ const char *dso_list_str,
*comm_list_str,
*sym_list_str,
*col_width_list_str;
--
1.6.2.5
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [GIT PULL 0/4] perf options type checking
2010-05-17 19:31 [GIT PULL 0/4] perf options type checking Arnaldo Carvalho de Melo
` (3 preceding siblings ...)
2010-05-17 19:31 ` [PATCH 4/4] perf options: Type check all the remaining OPT_ variants Arnaldo Carvalho de Melo
@ 2010-05-17 23:29 ` Ingo Molnar
4 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2010-05-17 23:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: linux-kernel, Frédéric Weisbecker, Mike Galbraith,
Paul Mackerras, Peter Zijlstra, Stephane Eranian, Tom Zanussi
* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> Hi Ingo,
>
> Please pull from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux-2.6 perf
>
> Regards,
>
> - Arnaldo
>
> Arnaldo Carvalho de Melo (4):
> perf options: Introduce OPT_UINTEGER
> perf options: Check v type in OPT_U?INTEGER
> perf options: Type check OPT_BOOLEAN and fix the offenders
> perf options: Type check all the remaining OPT_ variants
>
> tools/perf/bench/sched-messaging.c | 6 ++----
> tools/perf/builtin-bench.c | 4 ++--
> tools/perf/builtin-help.c | 6 +++---
> tools/perf/builtin-kvm.c | 6 +++---
> tools/perf/builtin-lock.c | 3 +--
> tools/perf/builtin-record.c | 8 +++-----
> tools/perf/builtin-report.c | 4 ++--
> tools/perf/builtin-sched.c | 8 ++++----
> tools/perf/builtin-test.c | 2 +-
> tools/perf/builtin-top.c | 7 +++----
> tools/perf/perf.h | 3 ++-
> tools/perf/util/hist.c | 2 +-
> tools/perf/util/include/linux/kernel.h | 2 ++
> tools/perf/util/parse-options.c | 32 +++++++++++++++++++++++++-------
> tools/perf/util/parse-options.h | 27 +++++++++++++++++----------
> tools/perf/util/sort.c | 8 ++++----
> tools/perf/util/sort.h | 8 ++++----
> tools/perf/util/symbol.h | 2 +-
> 18 files changed, 80 insertions(+), 58 deletions(-)
Pulled, thanks a lot Arnaldo!
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread