From c506c96b61fa96c9a52ad4d25e895e45c1692650 Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 11 Dec 2013 09:15:00 -0300 Subject: [PATCH 01/20] tools lib symbol: Start carving out symbol parsing routines from perf Eventually this should be useful to other tools/ living utilities. For now don't try to build any .a, just trying the minimal approach of separating existing code into multiple .c files that can then be included wherever they are needed, using whatever build machinery already in place. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-pfa8i5zpf4bf9rcccryi0lt3@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/symbol/kallsyms.c | 58 ++++++++++++++++++++++++++++++ tools/lib/symbol/kallsyms.h | 24 +++++++++++++ tools/perf/MANIFEST | 2 ++ tools/perf/Makefile.perf | 5 +++ tools/perf/util/event.c | 1 + tools/perf/util/machine.c | 1 + tools/perf/util/symbol-elf.c | 1 + tools/perf/util/symbol.c | 69 +----------------------------------- tools/perf/util/symbol.h | 3 -- 9 files changed, 93 insertions(+), 71 deletions(-) create mode 100644 tools/lib/symbol/kallsyms.c create mode 100644 tools/lib/symbol/kallsyms.h diff --git a/tools/lib/symbol/kallsyms.c b/tools/lib/symbol/kallsyms.c new file mode 100644 index 000000000000..18bc271a4bbc --- /dev/null +++ b/tools/lib/symbol/kallsyms.c @@ -0,0 +1,58 @@ +#include "symbol/kallsyms.h" +#include +#include + +int kallsyms__parse(const char *filename, void *arg, + int (*process_symbol)(void *arg, const char *name, + char type, u64 start)) +{ + char *line = NULL; + size_t n; + int err = -1; + FILE *file = fopen(filename, "r"); + + if (file == NULL) + goto out_failure; + + err = 0; + + while (!feof(file)) { + u64 start; + int line_len, len; + char symbol_type; + char *symbol_name; + + line_len = getline(&line, &n, file); + if (line_len < 0 || !line) + break; + + line[--line_len] = '\0'; /* \n */ + + len = hex2u64(line, &start); + + len++; + if (len + 2 >= line_len) + continue; + + symbol_type = line[len]; + len += 2; + symbol_name = line + len; + len = line_len - len; + + if (len >= KSYM_NAME_LEN) { + err = -1; + break; + } + + err = process_symbol(arg, symbol_name, symbol_type, start); + if (err) + break; + } + + free(line); + fclose(file); + return err; + +out_failure: + return -1; +} diff --git a/tools/lib/symbol/kallsyms.h b/tools/lib/symbol/kallsyms.h new file mode 100644 index 000000000000..6084f5e18b3c --- /dev/null +++ b/tools/lib/symbol/kallsyms.h @@ -0,0 +1,24 @@ +#ifndef __TOOLS_KALLSYMS_H_ +#define __TOOLS_KALLSYMS_H_ 1 + +#include +#include +#include + +#ifndef KSYM_NAME_LEN +#define KSYM_NAME_LEN 256 +#endif + +static inline u8 kallsyms2elf_type(char type) +{ + if (type == 'W') + return STB_WEAK; + + return isupper(type) ? STB_GLOBAL : STB_LOCAL; +} + +int kallsyms__parse(const char *filename, void *arg, + int (*process_symbol)(void *arg, const char *name, + char type, u64 start)); + +#endif /* __TOOLS_KALLSYMS_H_ */ diff --git a/tools/perf/MANIFEST b/tools/perf/MANIFEST index 025de796067c..3170a7ff5782 100644 --- a/tools/perf/MANIFEST +++ b/tools/perf/MANIFEST @@ -2,6 +2,8 @@ tools/perf tools/scripts tools/lib/traceevent tools/lib/lk +tools/lib/symbol/kallsyms.c +tools/lib/symbol/kallsyms.h include/linux/const.h include/linux/perf_event.h include/linux/rbtree.h diff --git a/tools/perf/Makefile.perf b/tools/perf/Makefile.perf index 9a8cf376f874..fad61079e795 100644 --- a/tools/perf/Makefile.perf +++ b/tools/perf/Makefile.perf @@ -202,6 +202,7 @@ $(OUTPUT)util/pmu.o: $(OUTPUT)util/pmu-flex.c $(OUTPUT)util/pmu-bison.c LIB_FILE=$(OUTPUT)libperf.a +LIB_H += ../lib/symbol/kallsyms.h LIB_H += ../../include/uapi/linux/perf_event.h LIB_H += ../../include/linux/rbtree.h LIB_H += ../../include/linux/list.h @@ -312,6 +313,7 @@ LIB_OBJS += $(OUTPUT)util/evlist.o LIB_OBJS += $(OUTPUT)util/evsel.o LIB_OBJS += $(OUTPUT)util/exec_cmd.o LIB_OBJS += $(OUTPUT)util/help.o +LIB_OBJS += $(OUTPUT)util/kallsyms.o LIB_OBJS += $(OUTPUT)util/levenshtein.o LIB_OBJS += $(OUTPUT)util/parse-options.o LIB_OBJS += $(OUTPUT)util/parse-events.o @@ -672,6 +674,9 @@ $(OUTPUT)ui/browsers/map.o: ui/browsers/map.c $(OUTPUT)PERF-CFLAGS $(OUTPUT)ui/browsers/scripts.o: ui/browsers/scripts.c $(OUTPUT)PERF-CFLAGS $(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) -DENABLE_SLFUTURE_CONST $< +$(OUTPUT)util/kallsyms.o: ../lib/symbol/kallsyms.c $(OUTPUT)PERF-CFLAGS + $(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) $< + $(OUTPUT)util/rbtree.o: ../../lib/rbtree.c $(OUTPUT)PERF-CFLAGS $(QUIET_CC)$(CC) -o $@ -c $(CFLAGS) -Wno-unused-parameter -DETC_PERFCONFIG='"$(ETC_PERFCONFIG_SQ)"' $< diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c index c77814bf01e1..694876877ae2 100644 --- a/tools/perf/util/event.c +++ b/tools/perf/util/event.c @@ -7,6 +7,7 @@ #include "strlist.h" #include "thread.h" #include "thread_map.h" +#include "symbol/kallsyms.h" static const char *perf_event__names[] = { [0] = "TOTAL", diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 751454bcde69..c78cc84f433e 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -9,6 +9,7 @@ #include "strlist.h" #include "thread.h" #include +#include #include "unwind.h" int machine__init(struct machine *machine, const char *root_dir, pid_t pid) diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c index eed0b96302af..bf0ce29567b6 100644 --- a/tools/perf/util/symbol-elf.c +++ b/tools/perf/util/symbol-elf.c @@ -6,6 +6,7 @@ #include #include "symbol.h" +#include #include "debug.h" #ifndef HAVE_ELF_GETPHDRNUM_SUPPORT diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c index e377c2e96191..61eb1cddf01a 100644 --- a/tools/perf/util/symbol.c +++ b/tools/perf/util/symbol.c @@ -18,12 +18,9 @@ #include #include +#include #include -#ifndef KSYM_NAME_LEN -#define KSYM_NAME_LEN 256 -#endif - static int dso__load_kernel_sym(struct dso *dso, struct map *map, symbol_filter_t filter); static int dso__load_guest_kernel_sym(struct dso *dso, struct map *map, @@ -446,62 +443,6 @@ size_t dso__fprintf_symbols_by_name(struct dso *dso, return ret; } -int kallsyms__parse(const char *filename, void *arg, - int (*process_symbol)(void *arg, const char *name, - char type, u64 start)) -{ - char *line = NULL; - size_t n; - int err = -1; - FILE *file = fopen(filename, "r"); - - if (file == NULL) - goto out_failure; - - err = 0; - - while (!feof(file)) { - u64 start; - int line_len, len; - char symbol_type; - char *symbol_name; - - line_len = getline(&line, &n, file); - if (line_len < 0 || !line) - break; - - line[--line_len] = '\0'; /* \n */ - - len = hex2u64(line, &start); - - len++; - if (len + 2 >= line_len) - continue; - - symbol_type = line[len]; - len += 2; - symbol_name = line + len; - len = line_len - len; - - if (len >= KSYM_NAME_LEN) { - err = -1; - break; - } - - err = process_symbol(arg, symbol_name, - symbol_type, start); - if (err) - break; - } - - free(line); - fclose(file); - return err; - -out_failure: - return -1; -} - int modules__parse(const char *filename, void *arg, int (*process_module)(void *arg, const char *name, u64 start)) @@ -565,14 +506,6 @@ struct process_kallsyms_args { struct dso *dso; }; -static u8 kallsyms2elf_type(char type) -{ - if (type == 'W') - return STB_WEAK; - - return isupper(type) ? STB_GLOBAL : STB_LOCAL; -} - bool symbol__is_idle(struct symbol *sym) { const char * const idle_symbols[] = { diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index 6de9c2b8a601..8a9d910c5345 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -221,9 +221,6 @@ struct symbol *dso__first_symbol(struct dso *dso, enum map_type type); int filename__read_build_id(const char *filename, void *bf, size_t size); int sysfs__read_build_id(const char *filename, void *bf, size_t size); -int kallsyms__parse(const char *filename, void *arg, - int (*process_symbol)(void *arg, const char *name, - char type, u64 start)); int modules__parse(const char *filename, void *arg, int (*process_module)(void *arg, const char *name, u64 start)); From 1a47245d2f3bf6276c95cd37901b562962d6ae47 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 11 Dec 2013 14:36:23 +0200 Subject: [PATCH 02/20] perf tools: Add perf_event_paranoid() Add a function to return the value of /proc/sys/kernel/perf_event_paranoid. This will be used to determine default values for mmap size because perf is not subject to mmap limits when perf_event_paranoid is less than zero. Signed-off-by: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386765443-26966-12-git-send-email-alexander.shishkin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 3 +-- tools/perf/util/util.c | 19 +++++++++++++++++++ tools/perf/util/util.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index af250556b33f..2eb7378e4c40 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1191,8 +1191,7 @@ int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused, "Error:\t%s.\n" "Hint:\tCheck /proc/sys/kernel/perf_event_paranoid setting.", emsg); - if (filename__read_int("/proc/sys/kernel/perf_event_paranoid", &value)) - break; + value = perf_event_paranoid(); printed += scnprintf(buf + printed, size - printed, "\nHint:\t"); diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index 4a57609c0b43..8f63dba212d7 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -1,5 +1,6 @@ #include "../perf.h" #include "util.h" +#include "fs.h" #include #ifdef HAVE_BACKTRACE_SUPPORT #include @@ -8,6 +9,7 @@ #include #include #include +#include #include /* @@ -496,3 +498,20 @@ const char *get_filename_for_perf_kvm(void) return filename; } + +int perf_event_paranoid(void) +{ + char path[PATH_MAX]; + const char *procfs = procfs__mountpoint(); + int value; + + if (!procfs) + return INT_MAX; + + scnprintf(path, PATH_MAX, "%s/sys/kernel/perf_event_paranoid", procfs); + + if (filename__read_int(path, &value)) + return INT_MAX; + + return value; +} diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index 0171213d1d4d..1e7d4136cc82 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -321,6 +321,7 @@ void free_srcline(char *srcline); int filename__read_int(const char *filename, int *value); int filename__read_str(const char *filename, char **buf, size_t *sizep); +int perf_event_paranoid(void); const char *get_filename_for_perf_kvm(void); #endif /* GIT_COMPAT_UTIL_H */ From d645c442e68d24e64c46845bc8bb5d5a0a70b249 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 11 Dec 2013 14:36:28 +0200 Subject: [PATCH 03/20] perf header: Allow header->data_offset to be predetermined It will be necessary to predetermine header->data_offset to allow space for attributes that are added later. Consequently, do not change header->data_offset if it is non-zero. Signed-off-by: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386765443-26966-17-git-send-email-alexander.shishkin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/header.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/perf/util/header.c b/tools/perf/util/header.c index 0bb830f6b49c..61c54213704b 100644 --- a/tools/perf/util/header.c +++ b/tools/perf/util/header.c @@ -2327,7 +2327,8 @@ int perf_session__write_header(struct perf_session *session, } } - header->data_offset = lseek(fd, 0, SEEK_CUR); + if (!header->data_offset) + header->data_offset = lseek(fd, 0, SEEK_CUR); header->feat_offset = header->data_offset + header->data_size; if (at_exit) { From c09ec622629eeb4b7877646a42852e7156363425 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 11 Dec 2013 14:36:29 +0200 Subject: [PATCH 04/20] perf evlist: Add can_select_event() method Add a function to determine whether an event can be selected. This function is needed to allow a tool to automatically select additional events, but only if they are available. Signed-off-by: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386765443-26966-18-git-send-email-alexander.shishkin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.h | 2 ++ tools/perf/util/record.c | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 649d6ea98a84..8a04aae95a18 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -193,4 +193,6 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, pc->data_tail = tail; } +bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str); + #endif /* __PERF_EVLIST_H */ diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c index c8845b107f60..e5104538c354 100644 --- a/tools/perf/util/record.c +++ b/tools/perf/util/record.c @@ -177,3 +177,40 @@ int perf_record_opts__config(struct perf_record_opts *opts) { return perf_record_opts__config_freq(opts); } + +bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str) +{ + struct perf_evlist *temp_evlist; + struct perf_evsel *evsel; + int err, fd, cpu; + bool ret = false; + + temp_evlist = perf_evlist__new(); + if (!temp_evlist) + return false; + + err = parse_events(temp_evlist, str); + if (err) + goto out_delete; + + evsel = perf_evlist__last(temp_evlist); + + if (!evlist || cpu_map__empty(evlist->cpus)) { + struct cpu_map *cpus = cpu_map__new(NULL); + + cpu = cpus ? cpus->map[0] : 0; + cpu_map__delete(cpus); + } else { + cpu = evlist->cpus->map[0]; + } + + fd = sys_perf_event_open(&evsel->attr, -1, cpu, -1, 0); + if (fd >= 0) { + close(fd); + ret = true; + } + +out_delete: + perf_evlist__delete(temp_evlist); + return ret; +} From 71db07b12eace6a3619335d03eaf3cbe2de131ed Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 11 Dec 2013 14:36:32 +0200 Subject: [PATCH 05/20] perf tools: Move mem_bswap32/64 to util.c Move functions mem_bswap_32() and mem_bswap_64() so they can be reused. Signed-off-by: Adrian Hunter Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386765443-26966-21-git-send-email-alexander.shishkin@linux.intel.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/session.c | 21 --------------------- tools/perf/util/session.h | 2 -- tools/perf/util/util.c | 22 ++++++++++++++++++++++ tools/perf/util/util.h | 3 +++ 4 files changed, 25 insertions(+), 23 deletions(-) diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index e748f29c53cf..989b2e377626 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -247,27 +247,6 @@ void perf_tool__fill_defaults(struct perf_tool *tool) } } -void mem_bswap_32(void *src, int byte_size) -{ - u32 *m = src; - while (byte_size > 0) { - *m = bswap_32(*m); - byte_size -= sizeof(u32); - ++m; - } -} - -void mem_bswap_64(void *src, int byte_size) -{ - u64 *m = src; - - while (byte_size > 0) { - *m = bswap_64(*m); - byte_size -= sizeof(u64); - ++m; - } -} - static void swap_sample_id_all(union perf_event *event, void *data) { void *end = (void *) event + event->header.size; diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h index 2a3955ea4fd8..9c25d49900af 100644 --- a/tools/perf/util/session.h +++ b/tools/perf/util/session.h @@ -74,8 +74,6 @@ int perf_session__resolve_callchain(struct perf_session *session, bool perf_session__has_traces(struct perf_session *session, const char *msg); -void mem_bswap_64(void *src, int byte_size); -void mem_bswap_32(void *src, int byte_size); void perf_event__attr_swap(struct perf_event_attr *attr); int perf_session__create_kernel_maps(struct perf_session *session); diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c index 8f63dba212d7..42ad667bb317 100644 --- a/tools/perf/util/util.c +++ b/tools/perf/util/util.c @@ -10,6 +10,7 @@ #include #include #include +#include #include /* @@ -515,3 +516,24 @@ int perf_event_paranoid(void) return value; } + +void mem_bswap_32(void *src, int byte_size) +{ + u32 *m = src; + while (byte_size > 0) { + *m = bswap_32(*m); + byte_size -= sizeof(u32); + ++m; + } +} + +void mem_bswap_64(void *src, int byte_size) +{ + u64 *m = src; + + while (byte_size > 0) { + *m = bswap_64(*m); + byte_size -= sizeof(u64); + ++m; + } +} diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h index 1e7d4136cc82..a1eea3e809a3 100644 --- a/tools/perf/util/util.h +++ b/tools/perf/util/util.h @@ -323,5 +323,8 @@ int filename__read_int(const char *filename, int *value); int filename__read_str(const char *filename, char **buf, size_t *sizep); int perf_event_paranoid(void); +void mem_bswap_64(void *src, int byte_size); +void mem_bswap_32(void *src, int byte_size); + const char *get_filename_for_perf_kvm(void); #endif /* GIT_COMPAT_UTIL_H */ From 8d00be815c05ed0f0202f606bab4e54f98fd3b30 Mon Sep 17 00:00:00 2001 From: David Ahern Date: Tue, 10 Dec 2013 21:35:38 -0700 Subject: [PATCH 06/20] perf tools: Fix inverted error verification bug in thread__fork Commit 1902efe7f for the new comm infra added the wrong check for return code on thread__set_comm. err == 0 is normal, so don't return at that point unless err != 0. Signed-off-by: David Ahern Cc: Frederic Weisbecker Link: http://lkml.kernel.org/r/1386736538-23525-1-git-send-email-dsahern@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 49eaf1d7d89d..e3948612543e 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -126,7 +126,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp) if (!comm) return -ENOMEM; err = thread__set_comm(thread, comm, timestamp); - if (!err) + if (err) return err; thread->comm_set = true; } From a025e4f0d8a92b38539d39b495b530015296b4d9 Mon Sep 17 00:00:00 2001 From: Adrian Hunter Date: Wed, 11 Dec 2013 14:36:35 +0200 Subject: [PATCH 07/20] perf evlist: Add perf_evlist__to_front() Add a function to move a selected event to the front of the list. This is needed because it is not possible to use the PERF_EVENT_IOC_SET_OUTPUT IOCTL from an Instruction Tracing event to a non-Instruction Tracing event. Thus the Instruction Tracing event must come first. Cc: Andi Kleen Cc: David Ahern Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Mike Galbraith Cc: Namhyung Kim Cc: Paul Mackerras Cc: Peter Zijlstra Cc: Stephane Eranian Link: http://lkml.kernel.org/r/1386765443-26966-24-git-send-email-alexander.shishkin@linux.intel.com Signed-off-by: Adrian Hunter Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/evlist.c | 17 +++++++++++++++++ tools/perf/util/evlist.h | 3 +++ 2 files changed, 20 insertions(+) diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c index 2eb7378e4c40..0b31cee34874 100644 --- a/tools/perf/util/evlist.c +++ b/tools/perf/util/evlist.c @@ -1212,3 +1212,20 @@ int perf_evlist__strerror_open(struct perf_evlist *evlist __maybe_unused, return 0; } + +void perf_evlist__to_front(struct perf_evlist *evlist, + struct perf_evsel *move_evsel) +{ + struct perf_evsel *evsel, *n; + LIST_HEAD(move); + + if (move_evsel == perf_evlist__first(evlist)) + return; + + list_for_each_entry_safe(evsel, n, &evlist->entries, node) { + if (evsel->leader == move_evsel->leader) + list_move_tail(&evsel->node, &move); + } + + list_splice(&move, &evlist->entries); +} diff --git a/tools/perf/util/evlist.h b/tools/perf/util/evlist.h index 8a04aae95a18..9f64ede3ecbd 100644 --- a/tools/perf/util/evlist.h +++ b/tools/perf/util/evlist.h @@ -194,5 +194,8 @@ static inline void perf_mmap__write_tail(struct perf_mmap *md, } bool perf_evlist__can_select_event(struct perf_evlist *evlist, const char *str); +void perf_evlist__to_front(struct perf_evlist *evlist, + struct perf_evsel *move_evsel); + #endif /* __PERF_EVLIST_H */ From 8f2f5ada719560954174da30ce0a67261c616e39 Mon Sep 17 00:00:00 2001 From: Ramkumar Ramachandra Date: Wed, 11 Dec 2013 16:04:15 +0530 Subject: [PATCH 08/20] perf completion: Complete 'perf kvm' Currently, there is no way to enumerate the subcommands under 'perf kvm', so hardcode them. Signed-off-by: Ramkumar Ramachandra Link: http://lkml.kernel.org/r/1386758056-24618-2-git-send-email-artagnon@gmail.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/perf-completion.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/perf/perf-completion.sh b/tools/perf/perf-completion.sh index 49494882d9bb..496e2abb5482 100644 --- a/tools/perf/perf-completion.sh +++ b/tools/perf/perf-completion.sh @@ -121,6 +121,10 @@ __perf_main () elif [[ $prev == "-e" && "${words[1]}" == @(record|stat|top) ]]; then evts=$($cmd list --raw-dump) __perfcomp_colon "$evts" "$cur" + # List subcommands for 'perf kvm' + elif [[ $prev == "kvm" ]]; then + subcmds="top record report diff buildid-list stat" + __perfcomp_colon "$subcmds" "$cur" # List long option names elif [[ $cur == --* ]]; then subcmd=${words[1]} From 9451a2fd78c785445afe0f6966b2043c3ee187ca Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:04 +0900 Subject: [PATCH 09/20] tools lib traceevent: Get rid of malloc_or_die() in show_error() Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-2-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index ab402fb2dcf7..d4b0bac80dc8 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -56,7 +56,21 @@ static void show_error(char **error_str, const char *fmt, ...) index = pevent_get_input_buf_ptr(); len = input ? strlen(input) : 0; - error = malloc_or_die(MAX_ERR_STR_SIZE + (len*2) + 3); + error = malloc(MAX_ERR_STR_SIZE + (len*2) + 3); + if (error == NULL) { + /* + * Maybe it's due to len is too long. + * Retry without the input buffer part. + */ + len = 0; + + error = malloc(MAX_ERR_STR_SIZE); + if (error == NULL) { + /* no memory */ + *error_str = NULL; + return; + } + } if (len) { strcpy(error, input); From ef3072cd1d5c2ea229f7abf8d6475e0c200eeb71 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:05 +0900 Subject: [PATCH 10/20] tools lib traceevent: Get rid of die in add_filter_type() The realloc() should check return value and not to overwrite previous pointer in case of error. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-3-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index d4b0bac80dc8..767de4f1e8ee 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -161,11 +161,13 @@ add_filter_type(struct event_filter *filter, int id) if (filter_type) return filter_type; - filter->event_filters = realloc(filter->event_filters, - sizeof(*filter->event_filters) * - (filter->filters + 1)); - if (!filter->event_filters) - die("Could not allocate filter"); + filter_type = realloc(filter->event_filters, + sizeof(*filter->event_filters) * + (filter->filters + 1)); + if (!filter_type) + return NULL; + + filter->event_filters = filter_type; for (i = 0; i < filter->filters; i++) { if (filter->event_filters[i].event_id > id) @@ -1180,6 +1182,12 @@ static int filter_event(struct event_filter *filter, } filter_type = add_filter_type(filter, event->id); + if (filter_type == NULL) { + show_error(error_str, "failed to add a new filter: %s", + filter_str ? filter_str : "true"); + return -1; + } + if (filter_type->filter) free_arg(filter_type->filter); filter_type->filter = arg; @@ -1417,6 +1425,9 @@ static int copy_filter_type(struct event_filter *filter, arg->boolean.value = 0; filter_type = add_filter_type(filter, event->id); + if (filter_type == NULL) + return -1; + filter_type->filter = arg; free(str); From 2e4eb10d7e59df71ab649343b3f1bff9259da15d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:06 +0900 Subject: [PATCH 11/20] tools lib traceevent: Get rid of malloc_or_die() allocate_arg() Also check return value and handle it. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-4-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 48 ++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 767de4f1e8ee..ab9cefe320b4 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -211,12 +211,7 @@ struct event_filter *pevent_filter_alloc(struct pevent *pevent) static struct filter_arg *allocate_arg(void) { - struct filter_arg *arg; - - arg = malloc_or_die(sizeof(*arg)); - memset(arg, 0, sizeof(*arg)); - - return arg; + return calloc(1, sizeof(struct filter_arg)); } static void free_arg(struct filter_arg *arg) @@ -369,6 +364,10 @@ create_arg_item(struct event_format *event, const char *token, struct filter_arg *arg; arg = allocate_arg(); + if (arg == NULL) { + show_error(error_str, "failed to allocate filter arg"); + return NULL; + } switch (type) { @@ -422,6 +421,9 @@ create_arg_op(enum filter_op_type btype) struct filter_arg *arg; arg = allocate_arg(); + if (!arg) + return NULL; + arg->type = FILTER_ARG_OP; arg->op.type = btype; @@ -434,6 +436,9 @@ create_arg_exp(enum filter_exp_type etype) struct filter_arg *arg; arg = allocate_arg(); + if (!arg) + return NULL; + arg->type = FILTER_ARG_EXP; arg->op.type = etype; @@ -446,6 +451,9 @@ create_arg_cmp(enum filter_exp_type etype) struct filter_arg *arg; arg = allocate_arg(); + if (!arg) + return NULL; + /* Use NUM and change if necessary */ arg->type = FILTER_ARG_NUM; arg->op.type = etype; @@ -909,8 +917,10 @@ static struct filter_arg *collapse_tree(struct filter_arg *arg) case FILTER_VAL_FALSE: free_arg(arg); arg = allocate_arg(); - arg->type = FILTER_ARG_BOOLEAN; - arg->boolean.value = ret == FILTER_VAL_TRUE; + if (arg) { + arg->type = FILTER_ARG_BOOLEAN; + arg->boolean.value = ret == FILTER_VAL_TRUE; + } } return arg; @@ -1057,6 +1067,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, switch (op_type) { case OP_BOOL: arg = create_arg_op(btype); + if (arg == NULL) + goto fail_alloc; if (current_op) ret = add_left(arg, current_op); else @@ -1067,6 +1079,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, case OP_NOT: arg = create_arg_op(btype); + if (arg == NULL) + goto fail_alloc; if (current_op) ret = add_right(current_op, arg, error_str); if (ret < 0) @@ -1086,6 +1100,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, arg = create_arg_exp(etype); else arg = create_arg_cmp(ctype); + if (arg == NULL) + goto fail_alloc; if (current_op) ret = add_right(current_op, arg, error_str); @@ -1119,11 +1135,16 @@ process_filter(struct event_format *event, struct filter_arg **parg, current_op = current_exp; current_op = collapse_tree(current_op); + if (current_op == NULL) + goto fail_alloc; *parg = current_op; return 0; + fail_alloc: + show_error(error_str, "failed to allocate filter arg"); + goto fail; fail_print: show_error(error_str, "Syntax error"); fail: @@ -1154,6 +1175,10 @@ process_event(struct event_format *event, const char *filter_str, /* If parg is NULL, then make it into FALSE */ if (!*parg) { *parg = allocate_arg(); + if (*parg == NULL) { + show_error(error_str, "failed to allocate filter arg"); + return -1; + } (*parg)->type = FILTER_ARG_BOOLEAN; (*parg)->boolean.value = FILTER_FALSE; } @@ -1177,6 +1202,10 @@ static int filter_event(struct event_filter *filter, } else { /* just add a TRUE arg */ arg = allocate_arg(); + if (arg == NULL) { + show_error(error_str, "failed to allocate filter arg"); + return -1; + } arg->type = FILTER_ARG_BOOLEAN; arg->boolean.value = FILTER_TRUE; } @@ -1418,6 +1447,9 @@ static int copy_filter_type(struct event_filter *filter, if (strcmp(str, "TRUE") == 0 || strcmp(str, "FALSE") == 0) { /* Add trivial event */ arg = allocate_arg(); + if (arg == NULL) + return -1; + arg->type = FILTER_ARG_BOOLEAN; if (strcmp(str, "TRUE") == 0) arg->boolean.value = 1; From 91dfa49bdd8ef9600d850ef68ec892eb70824e3d Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:07 +0900 Subject: [PATCH 12/20] tools lib traceevent: Get rid of malloc_or_die() in read_token() Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-5-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index ab9cefe320b4..246ee81e1f93 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -109,7 +109,11 @@ static enum event_type read_token(char **tok) (strcmp(token, "=") == 0 || strcmp(token, "!") == 0) && pevent_peek_char() == '~') { /* append it */ - *tok = malloc_or_die(3); + *tok = malloc(3); + if (*tok == NULL) { + free_token(token); + return EVENT_ERROR; + } sprintf(*tok, "%c%c", *token, '~'); free_token(token); /* Now remove the '~' from the buffer */ @@ -1123,6 +1127,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, break; case EVENT_NONE: break; + case EVENT_ERROR: + goto fail_alloc; default: goto fail_print; } From 605b8fda958a578e0a50ed1df3cac5a12f1fe8dc Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:08 +0900 Subject: [PATCH 13/20] tools lib traceevent: Get rid of malloc_or_die() in find_event() Make it return pevent_errno to distinguish malloc allocation failure. Since it'll be returned to user later, add more error code. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-6-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 4 +++- tools/lib/traceevent/parse-filter.c | 27 +++++++++++++++++++-------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 6e23f197175f..abdfd3c606ed 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -356,7 +356,9 @@ enum pevent_flag { _PE(READ_FORMAT_FAILED, "failed to read event format"), \ _PE(READ_PRINT_FAILED, "failed to read event print fmt"), \ _PE(OLD_FTRACE_ARG_FAILED,"failed to allocate field name for ftrace"),\ - _PE(INVALID_ARG_TYPE, "invalid argument type") + _PE(INVALID_ARG_TYPE, "invalid argument type"), \ + _PE(INVALID_EVENT_NAME, "invalid event name"), \ + _PE(EVENT_NOT_FOUND, "No event found") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 246ee81e1f93..a0ab040e8f71 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -287,7 +287,7 @@ static int event_match(struct event_format *event, !regexec(ereg, event->name, 0, NULL, 0); } -static int +static enum pevent_errno find_event(struct pevent *pevent, struct event_list **events, char *sys_name, char *event_name) { @@ -306,23 +306,31 @@ find_event(struct pevent *pevent, struct event_list **events, sys_name = NULL; } - reg = malloc_or_die(strlen(event_name) + 3); + reg = malloc(strlen(event_name) + 3); + if (reg == NULL) + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + sprintf(reg, "^%s$", event_name); ret = regcomp(&ereg, reg, REG_ICASE|REG_NOSUB); free(reg); if (ret) - return -1; + return PEVENT_ERRNO__INVALID_EVENT_NAME; if (sys_name) { - reg = malloc_or_die(strlen(sys_name) + 3); + reg = malloc(strlen(sys_name) + 3); + if (reg == NULL) { + regfree(&ereg); + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + } + sprintf(reg, "^%s$", sys_name); ret = regcomp(&sreg, reg, REG_ICASE|REG_NOSUB); free(reg); if (ret) { regfree(&ereg); - return -1; + return PEVENT_ERRNO__INVALID_EVENT_NAME; } } @@ -342,9 +350,9 @@ find_event(struct pevent *pevent, struct event_list **events, regfree(&sreg); if (!match) - return -1; + return PEVENT_ERRNO__EVENT_NOT_FOUND; if (fail) - return -2; + return PEVENT_ERRNO__MEM_ALLOC_FAILED; return 0; } @@ -1312,7 +1320,10 @@ int pevent_filter_add_filter_str(struct event_filter *filter, /* Find this event */ ret = find_event(pevent, &events, strim(sys_name), strim(event_name)); if (ret < 0) { - if (event_name) + if (ret == PEVENT_ERRNO__MEM_ALLOC_FAILED) + show_error(error_str, + "Memory allocation failure"); + else if (event_name) show_error(error_str, "No event found under '%s.%s'", sys_name, event_name); From 02d62d6d17b9b718be2878477cdcae95df0d5b4e Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:09 +0900 Subject: [PATCH 14/20] tools lib traceevent: Get rid of die() in add_right() Refactor it to return appropriate pevent_errno value. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-7-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 8 ++++++- tools/lib/traceevent/parse-filter.c | 34 ++++++++++++++++------------- 2 files changed, 26 insertions(+), 16 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index abdfd3c606ed..89e4dfd40db6 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -358,7 +358,13 @@ enum pevent_flag { _PE(OLD_FTRACE_ARG_FAILED,"failed to allocate field name for ftrace"),\ _PE(INVALID_ARG_TYPE, "invalid argument type"), \ _PE(INVALID_EVENT_NAME, "invalid event name"), \ - _PE(EVENT_NOT_FOUND, "No event found") + _PE(EVENT_NOT_FOUND, "no event found"), \ + _PE(SYNTAX_ERROR, "syntax error"), \ + _PE(ILLEGAL_RVALUE, "illegal rvalue"), \ + _PE(ILLEGAL_LVALUE, "illegal lvalue for string comparison"), \ + _PE(INVALID_REGEX, "regex did not compute"), \ + _PE(ILLEGAL_STRING_CMP, "illegal comparison for string"), \ + _PE(ILLEGAL_INTEGER_CMP,"illegal comparison for integer") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index a0ab040e8f71..c08ce594cabe 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -473,8 +473,8 @@ create_arg_cmp(enum filter_exp_type etype) return arg; } -static int add_right(struct filter_arg *op, struct filter_arg *arg, - char **error_str) +static enum pevent_errno +add_right(struct filter_arg *op, struct filter_arg *arg, char **error_str) { struct filter_arg *left; char *str; @@ -505,9 +505,8 @@ static int add_right(struct filter_arg *op, struct filter_arg *arg, case FILTER_ARG_FIELD: break; default: - show_error(error_str, - "Illegal rvalue"); - return -1; + show_error(error_str, "Illegal rvalue"); + return PEVENT_ERRNO__ILLEGAL_RVALUE; } /* @@ -554,7 +553,7 @@ static int add_right(struct filter_arg *op, struct filter_arg *arg, if (left->type != FILTER_ARG_FIELD) { show_error(error_str, "Illegal lvalue for string comparison"); - return -1; + return PEVENT_ERRNO__ILLEGAL_LVALUE; } /* Make sure this is a valid string compare */ @@ -573,25 +572,31 @@ static int add_right(struct filter_arg *op, struct filter_arg *arg, show_error(error_str, "RegEx '%s' did not compute", str); - return -1; + return PEVENT_ERRNO__INVALID_REGEX; } break; default: show_error(error_str, "Illegal comparison for string"); - return -1; + return PEVENT_ERRNO__ILLEGAL_STRING_CMP; } op->type = FILTER_ARG_STR; op->str.type = op_type; op->str.field = left->field.field; op->str.val = strdup(str); - if (!op->str.val) - die("malloc string"); + if (!op->str.val) { + show_error(error_str, "Failed to allocate string filter"); + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + } /* * Need a buffer to copy data for tests */ - op->str.buffer = malloc_or_die(op->str.field->size + 1); + op->str.buffer = malloc(op->str.field->size + 1); + if (!op->str.buffer) { + show_error(error_str, "Failed to allocate string filter"); + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + } /* Null terminate this buffer */ op->str.buffer[op->str.field->size] = 0; @@ -609,7 +614,7 @@ static int add_right(struct filter_arg *op, struct filter_arg *arg, case FILTER_CMP_NOT_REGEX: show_error(error_str, "Op not allowed with integers"); - return -1; + return PEVENT_ERRNO__ILLEGAL_INTEGER_CMP; default: break; @@ -629,9 +634,8 @@ static int add_right(struct filter_arg *op, struct filter_arg *arg, return 0; out_fail: - show_error(error_str, - "Syntax error"); - return -1; + show_error(error_str, "Syntax error"); + return PEVENT_ERRNO__SYNTAX_ERROR; } static struct filter_arg * From ff533fc058975579dffbb62a731f63911ae714be Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:10 +0900 Subject: [PATCH 15/20] tools lib traceevent: Make add_left() return pevent_errno So that it can propagate error properly. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-8-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/parse-filter.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index c08ce594cabe..774c3e4c1d9f 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -648,7 +648,7 @@ rotate_op_right(struct filter_arg *a, struct filter_arg *b) return arg; } -static int add_left(struct filter_arg *op, struct filter_arg *arg) +static enum pevent_errno add_left(struct filter_arg *op, struct filter_arg *arg) { switch (op->type) { case FILTER_ARG_EXP: @@ -667,11 +667,11 @@ static int add_left(struct filter_arg *op, struct filter_arg *arg) /* left arg of compares must be a field */ if (arg->type != FILTER_ARG_FIELD && arg->type != FILTER_ARG_BOOLEAN) - return -1; + return PEVENT_ERRNO__INVALID_ARG_TYPE; op->num.left = arg; break; default: - return -1; + return PEVENT_ERRNO__INVALID_ARG_TYPE; } return 0; } From 7bb73553e2490ac6667387ee723e0faa61e9d999 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:11 +0900 Subject: [PATCH 16/20] tools lib traceevent: Get rid of die() in reparent_op_arg() To do that, make the function returns the error code. Also pass error_str so that it can set proper error message when error occurred. Signed-off-by: Namhyung Kim Reviewed-by: Steven Rostedt Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-9-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 5 +- tools/lib/traceevent/parse-filter.c | 94 ++++++++++++++++++----------- 2 files changed, 64 insertions(+), 35 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 89e4dfd40db6..5e4392d8e2d4 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -364,7 +364,10 @@ enum pevent_flag { _PE(ILLEGAL_LVALUE, "illegal lvalue for string comparison"), \ _PE(INVALID_REGEX, "regex did not compute"), \ _PE(ILLEGAL_STRING_CMP, "illegal comparison for string"), \ - _PE(ILLEGAL_INTEGER_CMP,"illegal comparison for integer") + _PE(ILLEGAL_INTEGER_CMP,"illegal comparison for integer"), \ + _PE(REPARENT_NOT_OP, "cannot reparent other than OP"), \ + _PE(REPARENT_FAILED, "failed to reparent filter OP"), \ + _PE(BAD_FILTER_ARG, "bad arg in filter tree") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 774c3e4c1d9f..9b05892566e0 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -784,15 +784,18 @@ enum filter_vals { FILTER_VAL_TRUE, }; -void reparent_op_arg(struct filter_arg *parent, struct filter_arg *old_child, - struct filter_arg *arg) +static enum pevent_errno +reparent_op_arg(struct filter_arg *parent, struct filter_arg *old_child, + struct filter_arg *arg, char **error_str) { struct filter_arg *other_child; struct filter_arg **ptr; if (parent->type != FILTER_ARG_OP && - arg->type != FILTER_ARG_OP) - die("can not reparent other than OP"); + arg->type != FILTER_ARG_OP) { + show_error(error_str, "can not reparent other than OP"); + return PEVENT_ERRNO__REPARENT_NOT_OP; + } /* Get the sibling */ if (old_child->op.right == arg) { @@ -801,8 +804,10 @@ void reparent_op_arg(struct filter_arg *parent, struct filter_arg *old_child, } else if (old_child->op.left == arg) { ptr = &old_child->op.left; other_child = old_child->op.right; - } else - die("Error in reparent op, find other child"); + } else { + show_error(error_str, "Error in reparent op, find other child"); + return PEVENT_ERRNO__REPARENT_FAILED; + } /* Detach arg from old_child */ *ptr = NULL; @@ -813,23 +818,29 @@ void reparent_op_arg(struct filter_arg *parent, struct filter_arg *old_child, *parent = *arg; /* Free arg without recussion */ free(arg); - return; + return 0; } if (parent->op.right == old_child) ptr = &parent->op.right; else if (parent->op.left == old_child) ptr = &parent->op.left; - else - die("Error in reparent op"); + else { + show_error(error_str, "Error in reparent op"); + return PEVENT_ERRNO__REPARENT_FAILED; + } + *ptr = arg; free_arg(old_child); + return 0; } -enum filter_vals test_arg(struct filter_arg *parent, struct filter_arg *arg) +/* Returns either filter_vals (success) or pevent_errno (failfure) */ +static int test_arg(struct filter_arg *parent, struct filter_arg *arg, + char **error_str) { - enum filter_vals lval, rval; + int lval, rval; switch (arg->type) { @@ -844,63 +855,68 @@ enum filter_vals test_arg(struct filter_arg *parent, struct filter_arg *arg) return FILTER_VAL_NORM; case FILTER_ARG_EXP: - lval = test_arg(arg, arg->exp.left); + lval = test_arg(arg, arg->exp.left, error_str); if (lval != FILTER_VAL_NORM) return lval; - rval = test_arg(arg, arg->exp.right); + rval = test_arg(arg, arg->exp.right, error_str); if (rval != FILTER_VAL_NORM) return rval; return FILTER_VAL_NORM; case FILTER_ARG_NUM: - lval = test_arg(arg, arg->num.left); + lval = test_arg(arg, arg->num.left, error_str); if (lval != FILTER_VAL_NORM) return lval; - rval = test_arg(arg, arg->num.right); + rval = test_arg(arg, arg->num.right, error_str); if (rval != FILTER_VAL_NORM) return rval; return FILTER_VAL_NORM; case FILTER_ARG_OP: if (arg->op.type != FILTER_OP_NOT) { - lval = test_arg(arg, arg->op.left); + lval = test_arg(arg, arg->op.left, error_str); switch (lval) { case FILTER_VAL_NORM: break; case FILTER_VAL_TRUE: if (arg->op.type == FILTER_OP_OR) return FILTER_VAL_TRUE; - rval = test_arg(arg, arg->op.right); + rval = test_arg(arg, arg->op.right, error_str); if (rval != FILTER_VAL_NORM) return rval; - reparent_op_arg(parent, arg, arg->op.right); - return FILTER_VAL_NORM; + return reparent_op_arg(parent, arg, arg->op.right, + error_str); case FILTER_VAL_FALSE: if (arg->op.type == FILTER_OP_AND) return FILTER_VAL_FALSE; - rval = test_arg(arg, arg->op.right); + rval = test_arg(arg, arg->op.right, error_str); if (rval != FILTER_VAL_NORM) return rval; - reparent_op_arg(parent, arg, arg->op.right); - return FILTER_VAL_NORM; + return reparent_op_arg(parent, arg, arg->op.right, + error_str); + + default: + return lval; } } - rval = test_arg(arg, arg->op.right); + rval = test_arg(arg, arg->op.right, error_str); switch (rval) { case FILTER_VAL_NORM: + default: break; + case FILTER_VAL_TRUE: if (arg->op.type == FILTER_OP_OR) return FILTER_VAL_TRUE; if (arg->op.type == FILTER_OP_NOT) return FILTER_VAL_FALSE; - reparent_op_arg(parent, arg, arg->op.left); - return FILTER_VAL_NORM; + return reparent_op_arg(parent, arg, arg->op.left, + error_str); case FILTER_VAL_FALSE: if (arg->op.type == FILTER_OP_AND) @@ -908,26 +924,27 @@ enum filter_vals test_arg(struct filter_arg *parent, struct filter_arg *arg) if (arg->op.type == FILTER_OP_NOT) return FILTER_VAL_TRUE; - reparent_op_arg(parent, arg, arg->op.left); - return FILTER_VAL_NORM; + return reparent_op_arg(parent, arg, arg->op.left, + error_str); } - return FILTER_VAL_NORM; + return rval; default: - die("bad arg in filter tree"); + show_error(error_str, "bad arg in filter tree"); + return PEVENT_ERRNO__BAD_FILTER_ARG; } return FILTER_VAL_NORM; } /* Remove any unknown event fields */ -static struct filter_arg *collapse_tree(struct filter_arg *arg) +static struct filter_arg *collapse_tree(struct filter_arg *arg, char **error_str) { enum filter_vals ret; - ret = test_arg(arg, arg); + ret = test_arg(arg, arg, error_str); switch (ret) { case FILTER_VAL_NORM: - return arg; + break; case FILTER_VAL_TRUE: case FILTER_VAL_FALSE: @@ -936,7 +953,16 @@ static struct filter_arg *collapse_tree(struct filter_arg *arg) if (arg) { arg->type = FILTER_ARG_BOOLEAN; arg->boolean.value = ret == FILTER_VAL_TRUE; + } else { + show_error(error_str, "Failed to allocate filter arg"); } + break; + + default: + /* test_arg() already set the error_str */ + free_arg(arg); + arg = NULL; + break; } return arg; @@ -1152,9 +1178,9 @@ process_filter(struct event_format *event, struct filter_arg **parg, if (!current_op) current_op = current_exp; - current_op = collapse_tree(current_op); + current_op = collapse_tree(current_op, error_str); if (current_op == NULL) - goto fail_alloc; + goto fail; *parg = current_op; From c8ea690dd0d1385a766d68c51832497181e013b8 Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:12 +0900 Subject: [PATCH 17/20] tools lib traceevent: Refactor create_arg_item() So that it can return a proper pevent_errno value. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-10-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 3 ++- tools/lib/traceevent/parse-filter.c | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 5e4392d8e2d4..57b66aed8122 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -367,7 +367,8 @@ enum pevent_flag { _PE(ILLEGAL_INTEGER_CMP,"illegal comparison for integer"), \ _PE(REPARENT_NOT_OP, "cannot reparent other than OP"), \ _PE(REPARENT_FAILED, "failed to reparent filter OP"), \ - _PE(BAD_FILTER_ARG, "bad arg in filter tree") + _PE(BAD_FILTER_ARG, "bad arg in filter tree"), \ + _PE(UNEXPECTED_TYPE, "unexpected type (not a value)") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 9b05892566e0..8d71208f0131 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -368,9 +368,9 @@ static void free_events(struct event_list *events) } } -static struct filter_arg * +static enum pevent_errno create_arg_item(struct event_format *event, const char *token, - enum event_type type, char **error_str) + enum event_type type, struct filter_arg **parg, char **error_str) { struct format_field *field; struct filter_arg *arg; @@ -378,7 +378,7 @@ create_arg_item(struct event_format *event, const char *token, arg = allocate_arg(); if (arg == NULL) { show_error(error_str, "failed to allocate filter arg"); - return NULL; + return PEVENT_ERRNO__MEM_ALLOC_FAILED; } switch (type) { @@ -392,7 +392,7 @@ create_arg_item(struct event_format *event, const char *token, if (!arg->value.str) { free_arg(arg); show_error(error_str, "failed to allocate string filter arg"); - return NULL; + return PEVENT_ERRNO__MEM_ALLOC_FAILED; } break; case EVENT_ITEM: @@ -420,11 +420,11 @@ create_arg_item(struct event_format *event, const char *token, break; default: free_arg(arg); - show_error(error_str, "expected a value but found %s", - token); - return NULL; + show_error(error_str, "expected a value but found %s", token); + return PEVENT_ERRNO__UNEXPECTED_TYPE; } - return arg; + *parg = arg; + return 0; } static struct filter_arg * @@ -993,8 +993,8 @@ process_filter(struct event_format *event, struct filter_arg **parg, case EVENT_SQUOTE: case EVENT_DQUOTE: case EVENT_ITEM: - arg = create_arg_item(event, token, type, error_str); - if (!arg) + ret = create_arg_item(event, token, type, &arg, error_str); + if (ret < 0) goto fail; if (!left_item) left_item = arg; From 42d6194d133cbaf12f34cbdc4111bd8f7dc0ed2a Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:13 +0900 Subject: [PATCH 18/20] tools lib traceevent: Refactor process_filter() So that it can return a proper pevent_errno value. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-11-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 6 ++- tools/lib/traceevent/parse-filter.c | 64 +++++++++++++++++------------ 2 files changed, 42 insertions(+), 28 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 57b66aed8122..da942d59cc3a 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -368,7 +368,11 @@ enum pevent_flag { _PE(REPARENT_NOT_OP, "cannot reparent other than OP"), \ _PE(REPARENT_FAILED, "failed to reparent filter OP"), \ _PE(BAD_FILTER_ARG, "bad arg in filter tree"), \ - _PE(UNEXPECTED_TYPE, "unexpected type (not a value)") + _PE(UNEXPECTED_TYPE, "unexpected type (not a value)"), \ + _PE(ILLEGAL_TOKEN, "illegal token"), \ + _PE(INVALID_PAREN, "open parenthesis cannot come here"), \ + _PE(UNBALANCED_PAREN, "unbalanced number of parenthesis"), \ + _PE(UNKNOWN_TOKEN, "unknown token") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 8d71208f0131..5aa5012a17ee 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -937,9 +937,10 @@ static int test_arg(struct filter_arg *parent, struct filter_arg *arg, } /* Remove any unknown event fields */ -static struct filter_arg *collapse_tree(struct filter_arg *arg, char **error_str) +static int collapse_tree(struct filter_arg *arg, + struct filter_arg **arg_collapsed, char **error_str) { - enum filter_vals ret; + int ret; ret = test_arg(arg, arg, error_str); switch (ret) { @@ -955,6 +956,7 @@ static struct filter_arg *collapse_tree(struct filter_arg *arg, char **error_str arg->boolean.value = ret == FILTER_VAL_TRUE; } else { show_error(error_str, "Failed to allocate filter arg"); + ret = PEVENT_ERRNO__MEM_ALLOC_FAILED; } break; @@ -965,10 +967,11 @@ static struct filter_arg *collapse_tree(struct filter_arg *arg, char **error_str break; } - return arg; + *arg_collapsed = arg; + return ret; } -static int +static enum pevent_errno process_filter(struct event_format *event, struct filter_arg **parg, char **error_str, int not) { @@ -982,7 +985,7 @@ process_filter(struct event_format *event, struct filter_arg **parg, enum filter_op_type btype; enum filter_exp_type etype; enum filter_cmp_type ctype; - int ret; + enum pevent_errno ret; *parg = NULL; @@ -1007,20 +1010,20 @@ process_filter(struct event_format *event, struct filter_arg **parg, if (not) { arg = NULL; if (current_op) - goto fail_print; + goto fail_syntax; free(token); *parg = current_exp; return 0; } } else - goto fail_print; + goto fail_syntax; arg = NULL; break; case EVENT_DELIM: if (*token == ',') { - show_error(error_str, - "Illegal token ','"); + show_error(error_str, "Illegal token ','"); + ret = PEVENT_ERRNO__ILLEGAL_TOKEN; goto fail; } @@ -1028,19 +1031,23 @@ process_filter(struct event_format *event, struct filter_arg **parg, if (left_item) { show_error(error_str, "Open paren can not come after item"); + ret = PEVENT_ERRNO__INVALID_PAREN; goto fail; } if (current_exp) { show_error(error_str, "Open paren can not come after expression"); + ret = PEVENT_ERRNO__INVALID_PAREN; goto fail; } ret = process_filter(event, &arg, error_str, 0); - if (ret != 1) { - if (ret == 0) + if (ret != PEVENT_ERRNO__UNBALANCED_PAREN) { + if (ret == 0) { show_error(error_str, "Unbalanced number of '('"); + ret = PEVENT_ERRNO__UNBALANCED_PAREN; + } goto fail; } ret = 0; @@ -1048,7 +1055,7 @@ process_filter(struct event_format *event, struct filter_arg **parg, /* A not wants just one expression */ if (not) { if (current_op) - goto fail_print; + goto fail_syntax; *parg = arg; return 0; } @@ -1063,19 +1070,19 @@ process_filter(struct event_format *event, struct filter_arg **parg, } else { /* ')' */ if (!current_op && !current_exp) - goto fail_print; + goto fail_syntax; /* Make sure everything is finished at this level */ if (current_exp && !check_op_done(current_exp)) - goto fail_print; + goto fail_syntax; if (current_op && !check_op_done(current_op)) - goto fail_print; + goto fail_syntax; if (current_op) *parg = current_op; else *parg = current_exp; - return 1; + return PEVENT_ERRNO__UNBALANCED_PAREN; } break; @@ -1087,21 +1094,22 @@ process_filter(struct event_format *event, struct filter_arg **parg, case OP_BOOL: /* Logic ops need a left expression */ if (!current_exp && !current_op) - goto fail_print; + goto fail_syntax; /* fall through */ case OP_NOT: /* logic only processes ops and exp */ if (left_item) - goto fail_print; + goto fail_syntax; break; case OP_EXP: case OP_CMP: if (!left_item) - goto fail_print; + goto fail_syntax; break; case OP_NONE: show_error(error_str, "Unknown op token %s", token); + ret = PEVENT_ERRNO__UNKNOWN_TOKEN; goto fail; } @@ -1152,7 +1160,7 @@ process_filter(struct event_format *event, struct filter_arg **parg, ret = add_left(arg, left_item); if (ret < 0) { arg = NULL; - goto fail_print; + goto fail_syntax; } current_exp = arg; break; @@ -1161,25 +1169,25 @@ process_filter(struct event_format *event, struct filter_arg **parg, } arg = NULL; if (ret < 0) - goto fail_print; + goto fail_syntax; break; case EVENT_NONE: break; case EVENT_ERROR: goto fail_alloc; default: - goto fail_print; + goto fail_syntax; } } while (type != EVENT_NONE); if (!current_op && !current_exp) - goto fail_print; + goto fail_syntax; if (!current_op) current_op = current_exp; - current_op = collapse_tree(current_op, error_str); - if (current_op == NULL) + ret = collapse_tree(current_op, parg, error_str); + if (ret < 0) goto fail; *parg = current_op; @@ -1188,15 +1196,17 @@ process_filter(struct event_format *event, struct filter_arg **parg, fail_alloc: show_error(error_str, "failed to allocate filter arg"); + ret = PEVENT_ERRNO__MEM_ALLOC_FAILED; goto fail; - fail_print: + fail_syntax: show_error(error_str, "Syntax error"); + ret = PEVENT_ERRNO__SYNTAX_ERROR; fail: free_arg(current_op); free_arg(current_exp); free_arg(arg); free(token); - return -1; + return ret; } static int From 69c770a690422c6cdc4ea52d9edbba7c20cd1aff Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:14 +0900 Subject: [PATCH 19/20] tools lib traceevent: Make pevent_filter_add_filter_str() return pevent_errno Refactor the pevent_filter_add_filter_str() to return a proper error code and get rid of the third error_str argument. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-12-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 8 +-- tools/lib/traceevent/parse-filter.c | 78 +++++++++-------------------- 2 files changed, 27 insertions(+), 59 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index da942d59cc3a..089964e56ed4 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -372,7 +372,8 @@ enum pevent_flag { _PE(ILLEGAL_TOKEN, "illegal token"), \ _PE(INVALID_PAREN, "open parenthesis cannot come here"), \ _PE(UNBALANCED_PAREN, "unbalanced number of parenthesis"), \ - _PE(UNKNOWN_TOKEN, "unknown token") + _PE(UNKNOWN_TOKEN, "unknown token"), \ + _PE(FILTER_NOT_FOUND, "no filter found") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code @@ -863,9 +864,8 @@ enum filter_trivial_type { FILTER_TRIVIAL_BOTH, }; -int pevent_filter_add_filter_str(struct event_filter *filter, - const char *filter_str, - char **error_str); +enum pevent_errno pevent_filter_add_filter_str(struct event_filter *filter, + const char *filter_str); int pevent_filter_match(struct event_filter *filter, diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 5aa5012a17ee..78440d73e0ad 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -1209,7 +1209,7 @@ process_filter(struct event_format *event, struct filter_arg **parg, return ret; } -static int +static enum pevent_errno process_event(struct event_format *event, const char *filter_str, struct filter_arg **parg, char **error_str) { @@ -1218,21 +1218,15 @@ process_event(struct event_format *event, const char *filter_str, pevent_buffer_init(filter_str, strlen(filter_str)); ret = process_filter(event, parg, error_str, 0); - if (ret == 1) { - show_error(error_str, - "Unbalanced number of ')'"); - return -1; - } if (ret < 0) return ret; /* If parg is NULL, then make it into FALSE */ if (!*parg) { *parg = allocate_arg(); - if (*parg == NULL) { - show_error(error_str, "failed to allocate filter arg"); - return -1; - } + if (*parg == NULL) + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + (*parg)->type = FILTER_ARG_BOOLEAN; (*parg)->boolean.value = FILTER_FALSE; } @@ -1240,13 +1234,13 @@ process_event(struct event_format *event, const char *filter_str, return 0; } -static int filter_event(struct event_filter *filter, - struct event_format *event, - const char *filter_str, char **error_str) +static enum pevent_errno +filter_event(struct event_filter *filter, struct event_format *event, + const char *filter_str, char **error_str) { struct filter_type *filter_type; struct filter_arg *arg; - int ret; + enum pevent_errno ret; if (filter_str) { ret = process_event(event, filter_str, &arg, error_str); @@ -1256,20 +1250,16 @@ static int filter_event(struct event_filter *filter, } else { /* just add a TRUE arg */ arg = allocate_arg(); - if (arg == NULL) { - show_error(error_str, "failed to allocate filter arg"); - return -1; - } + if (arg == NULL) + return PEVENT_ERRNO__MEM_ALLOC_FAILED; + arg->type = FILTER_ARG_BOOLEAN; arg->boolean.value = FILTER_TRUE; } filter_type = add_filter_type(filter, event->id); - if (filter_type == NULL) { - show_error(error_str, "failed to add a new filter: %s", - filter_str ? filter_str : "true"); - return -1; - } + if (filter_type == NULL) + return PEVENT_ERRNO__MEM_ALLOC_FAILED; if (filter_type->filter) free_arg(filter_type->filter); @@ -1282,18 +1272,12 @@ static int filter_event(struct event_filter *filter, * pevent_filter_add_filter_str - add a new filter * @filter: the event filter to add to * @filter_str: the filter string that contains the filter - * @error_str: string containing reason for failed filter * - * Returns 0 if the filter was successfully added - * -1 if there was an error. - * - * On error, if @error_str points to a string pointer, - * it is set to the reason that the filter failed. - * This string must be freed with "free". + * Returns 0 if the filter was successfully added or a + * negative error code. */ -int pevent_filter_add_filter_str(struct event_filter *filter, - const char *filter_str, - char **error_str) +enum pevent_errno pevent_filter_add_filter_str(struct event_filter *filter, + const char *filter_str) { struct pevent *pevent = filter->pevent; struct event_list *event; @@ -1304,23 +1288,20 @@ int pevent_filter_add_filter_str(struct event_filter *filter, char *event_name = NULL; char *sys_name = NULL; char *sp; - int rtn = 0; + enum pevent_errno rtn = 0; /* PEVENT_ERRNO__SUCCESS */ int len; int ret; + char *error_str = NULL; /* clear buffer to reset show error */ pevent_buffer_init("", 0); - if (error_str) - *error_str = NULL; - filter_start = strchr(filter_str, ':'); if (filter_start) len = filter_start - filter_str; else len = strlen(filter_str); - do { next_event = strchr(filter_str, ','); if (next_event && @@ -1333,10 +1314,9 @@ int pevent_filter_add_filter_str(struct event_filter *filter, this_event = malloc(len + 1); if (this_event == NULL) { - show_error(error_str, "Memory allocation failure"); /* This can only happen when events is NULL, but still */ free_events(events); - return -1; + return PEVENT_ERRNO__MEM_ALLOC_FAILED; } memcpy(this_event, filter_str, len); this_event[len] = 0; @@ -1350,30 +1330,18 @@ int pevent_filter_add_filter_str(struct event_filter *filter, event_name = strtok_r(NULL, "/", &sp); if (!sys_name) { - show_error(error_str, "No filter found"); /* This can only happen when events is NULL, but still */ free_events(events); free(this_event); - return -1; + return PEVENT_ERRNO__FILTER_NOT_FOUND; } /* Find this event */ ret = find_event(pevent, &events, strim(sys_name), strim(event_name)); if (ret < 0) { - if (ret == PEVENT_ERRNO__MEM_ALLOC_FAILED) - show_error(error_str, - "Memory allocation failure"); - else if (event_name) - show_error(error_str, - "No event found under '%s.%s'", - sys_name, event_name); - else - show_error(error_str, - "No event found under '%s'", - sys_name); free_events(events); free(this_event); - return -1; + return ret; } free(this_event); } while (filter_str); @@ -1385,7 +1353,7 @@ int pevent_filter_add_filter_str(struct event_filter *filter, /* filter starts here */ for (event = events; event; event = event->next) { ret = filter_event(filter, event->event, filter_start, - error_str); + &error_str); /* Failures are returned if a parse error happened */ if (ret < 0) rtn = ret; From 41e12e580a7b0c151199f927193548b84d3e874c Mon Sep 17 00:00:00 2001 From: Namhyung Kim Date: Thu, 12 Dec 2013 16:36:15 +0900 Subject: [PATCH 20/20] tools lib traceevent: Refactor pevent_filter_match() to get rid of die() The test_filter() function is for testing given filter is matched to a given record. However it doesn't handle error cases properly so add a new argument err to save error info during the test and also pass it to internal test functions. The return value of pevent_filter_match() also converted to pevent_errno to indicate an exact error case. Signed-off-by: Namhyung Kim Cc: Frederic Weisbecker Cc: Ingo Molnar Cc: Jiri Olsa Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Steven Rostedt Link: http://lkml.kernel.org/r/1386833777-3790-13-git-send-email-namhyung@kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/lib/traceevent/event-parse.h | 21 +++-- tools/lib/traceevent/parse-filter.c | 135 +++++++++++++++++----------- 2 files changed, 99 insertions(+), 57 deletions(-) diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index 089964e56ed4..3ad784f5f647 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -357,6 +357,8 @@ enum pevent_flag { _PE(READ_PRINT_FAILED, "failed to read event print fmt"), \ _PE(OLD_FTRACE_ARG_FAILED,"failed to allocate field name for ftrace"),\ _PE(INVALID_ARG_TYPE, "invalid argument type"), \ + _PE(INVALID_EXP_TYPE, "invalid expression type"), \ + _PE(INVALID_OP_TYPE, "invalid operator type"), \ _PE(INVALID_EVENT_NAME, "invalid event name"), \ _PE(EVENT_NOT_FOUND, "no event found"), \ _PE(SYNTAX_ERROR, "syntax error"), \ @@ -373,12 +375,16 @@ enum pevent_flag { _PE(INVALID_PAREN, "open parenthesis cannot come here"), \ _PE(UNBALANCED_PAREN, "unbalanced number of parenthesis"), \ _PE(UNKNOWN_TOKEN, "unknown token"), \ - _PE(FILTER_NOT_FOUND, "no filter found") + _PE(FILTER_NOT_FOUND, "no filter found"), \ + _PE(NOT_A_NUMBER, "must have number field"), \ + _PE(NO_FILTER, "no filters exists"), \ + _PE(FILTER_MISS, "record does not match to filter") #undef _PE #define _PE(__code, __str) PEVENT_ERRNO__ ## __code enum pevent_errno { PEVENT_ERRNO__SUCCESS = 0, + PEVENT_ERRNO__FILTER_MATCH = PEVENT_ERRNO__SUCCESS, /* * Choose an arbitrary negative big number not to clash with standard @@ -853,10 +859,11 @@ struct event_filter { struct event_filter *pevent_filter_alloc(struct pevent *pevent); -#define FILTER_NONE -2 -#define FILTER_NOEXIST -1 -#define FILTER_MISS 0 -#define FILTER_MATCH 1 +/* for backward compatibility */ +#define FILTER_NONE PEVENT_ERRNO__FILTER_NOT_FOUND +#define FILTER_NOEXIST PEVENT_ERRNO__NO_FILTER +#define FILTER_MISS PEVENT_ERRNO__FILTER_MISS +#define FILTER_MATCH PEVENT_ERRNO__FILTER_MATCH enum filter_trivial_type { FILTER_TRIVIAL_FALSE, @@ -868,8 +875,8 @@ enum pevent_errno pevent_filter_add_filter_str(struct event_filter *filter, const char *filter_str); -int pevent_filter_match(struct event_filter *filter, - struct pevent_record *record); +enum pevent_errno pevent_filter_match(struct event_filter *filter, + struct pevent_record *record); int pevent_event_filtered(struct event_filter *filter, int event_id); diff --git a/tools/lib/traceevent/parse-filter.c b/tools/lib/traceevent/parse-filter.c index 78440d73e0ad..9303c55128db 100644 --- a/tools/lib/traceevent/parse-filter.c +++ b/tools/lib/traceevent/parse-filter.c @@ -1678,8 +1678,8 @@ int pevent_filter_event_has_trivial(struct event_filter *filter, } } -static int test_filter(struct event_format *event, - struct filter_arg *arg, struct pevent_record *record); +static int test_filter(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err); static const char * get_comm(struct event_format *event, struct pevent_record *record) @@ -1725,15 +1725,24 @@ get_value(struct event_format *event, } static unsigned long long -get_arg_value(struct event_format *event, struct filter_arg *arg, struct pevent_record *record); +get_arg_value(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err); static unsigned long long -get_exp_value(struct event_format *event, struct filter_arg *arg, struct pevent_record *record) +get_exp_value(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { unsigned long long lval, rval; - lval = get_arg_value(event, arg->exp.left, record); - rval = get_arg_value(event, arg->exp.right, record); + lval = get_arg_value(event, arg->exp.left, record, err); + rval = get_arg_value(event, arg->exp.right, record, err); + + if (*err) { + /* + * There was an error, no need to process anymore. + */ + return 0; + } switch (arg->exp.type) { case FILTER_EXP_ADD: @@ -1768,39 +1777,51 @@ get_exp_value(struct event_format *event, struct filter_arg *arg, struct pevent_ case FILTER_EXP_NOT: default: - die("error in exp"); + if (!*err) + *err = PEVENT_ERRNO__INVALID_EXP_TYPE; } return 0; } static unsigned long long -get_arg_value(struct event_format *event, struct filter_arg *arg, struct pevent_record *record) +get_arg_value(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { switch (arg->type) { case FILTER_ARG_FIELD: return get_value(event, arg->field.field, record); case FILTER_ARG_VALUE: - if (arg->value.type != FILTER_NUMBER) - die("must have number field!"); + if (arg->value.type != FILTER_NUMBER) { + if (!*err) + *err = PEVENT_ERRNO__NOT_A_NUMBER; + } return arg->value.val; case FILTER_ARG_EXP: - return get_exp_value(event, arg, record); + return get_exp_value(event, arg, record, err); default: - die("oops in filter"); + if (!*err) + *err = PEVENT_ERRNO__INVALID_ARG_TYPE; } return 0; } -static int test_num(struct event_format *event, - struct filter_arg *arg, struct pevent_record *record) +static int test_num(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { unsigned long long lval, rval; - lval = get_arg_value(event, arg->num.left, record); - rval = get_arg_value(event, arg->num.right, record); + lval = get_arg_value(event, arg->num.left, record, err); + rval = get_arg_value(event, arg->num.right, record, err); + + if (*err) { + /* + * There was an error, no need to process anymore. + */ + return 0; + } switch (arg->num.type) { case FILTER_CMP_EQ: @@ -1822,7 +1843,8 @@ static int test_num(struct event_format *event, return lval <= rval; default: - /* ?? */ + if (!*err) + *err = PEVENT_ERRNO__ILLEGAL_INTEGER_CMP; return 0; } } @@ -1869,8 +1891,8 @@ static const char *get_field_str(struct filter_arg *arg, struct pevent_record *r return val; } -static int test_str(struct event_format *event, - struct filter_arg *arg, struct pevent_record *record) +static int test_str(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { const char *val; @@ -1894,48 +1916,57 @@ static int test_str(struct event_format *event, return regexec(&arg->str.reg, val, 0, NULL, 0); default: - /* ?? */ + if (!*err) + *err = PEVENT_ERRNO__ILLEGAL_STRING_CMP; return 0; } } -static int test_op(struct event_format *event, - struct filter_arg *arg, struct pevent_record *record) +static int test_op(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { switch (arg->op.type) { case FILTER_OP_AND: - return test_filter(event, arg->op.left, record) && - test_filter(event, arg->op.right, record); + return test_filter(event, arg->op.left, record, err) && + test_filter(event, arg->op.right, record, err); case FILTER_OP_OR: - return test_filter(event, arg->op.left, record) || - test_filter(event, arg->op.right, record); + return test_filter(event, arg->op.left, record, err) || + test_filter(event, arg->op.right, record, err); case FILTER_OP_NOT: - return !test_filter(event, arg->op.right, record); + return !test_filter(event, arg->op.right, record, err); default: - /* ?? */ + if (!*err) + *err = PEVENT_ERRNO__INVALID_OP_TYPE; return 0; } } -static int test_filter(struct event_format *event, - struct filter_arg *arg, struct pevent_record *record) +static int test_filter(struct event_format *event, struct filter_arg *arg, + struct pevent_record *record, enum pevent_errno *err) { + if (*err) { + /* + * There was an error, no need to process anymore. + */ + return 0; + } + switch (arg->type) { case FILTER_ARG_BOOLEAN: /* easy case */ return arg->boolean.value; case FILTER_ARG_OP: - return test_op(event, arg, record); + return test_op(event, arg, record, err); case FILTER_ARG_NUM: - return test_num(event, arg, record); + return test_num(event, arg, record, err); case FILTER_ARG_STR: - return test_str(event, arg, record); + return test_str(event, arg, record, err); case FILTER_ARG_EXP: case FILTER_ARG_VALUE: @@ -1944,11 +1975,11 @@ static int test_filter(struct event_format *event, * Expressions, fields and values evaluate * to true if they return non zero */ - return !!get_arg_value(event, arg, record); + return !!get_arg_value(event, arg, record, err); default: - die("oops!"); - /* ?? */ + if (!*err) + *err = PEVENT_ERRNO__INVALID_ARG_TYPE; return 0; } } @@ -1961,8 +1992,7 @@ static int test_filter(struct event_format *event, * Returns 1 if filter found for @event_id * otherwise 0; */ -int pevent_event_filtered(struct event_filter *filter, - int event_id) +int pevent_event_filtered(struct event_filter *filter, int event_id) { struct filter_type *filter_type; @@ -1979,31 +2009,36 @@ int pevent_event_filtered(struct event_filter *filter, * @filter: filter struct with filter information * @record: the record to test against the filter * - * Returns: - * 1 - filter found for event and @record matches - * 0 - filter found for event and @record does not match - * -1 - no filter found for @record's event - * -2 - if no filters exist + * Returns: match result or error code (prefixed with PEVENT_ERRNO__) + * FILTER_MATCH - filter found for event and @record matches + * FILTER_MISS - filter found for event and @record does not match + * FILTER_NOT_FOUND - no filter found for @record's event + * NO_FILTER - if no filters exist + * otherwise - error occurred during test */ -int pevent_filter_match(struct event_filter *filter, - struct pevent_record *record) +enum pevent_errno pevent_filter_match(struct event_filter *filter, + struct pevent_record *record) { struct pevent *pevent = filter->pevent; struct filter_type *filter_type; int event_id; + int ret; + enum pevent_errno err = 0; if (!filter->filters) - return FILTER_NONE; + return PEVENT_ERRNO__NO_FILTER; event_id = pevent_data_type(pevent, record); filter_type = find_filter_type(filter, event_id); - if (!filter_type) - return FILTER_NOEXIST; + return PEVENT_ERRNO__FILTER_NOT_FOUND; - return test_filter(filter_type->event, filter_type->filter, record) ? - FILTER_MATCH : FILTER_MISS; + ret = test_filter(filter_type->event, filter_type->filter, record, &err); + if (err) + return err; + + return ret ? PEVENT_ERRNO__FILTER_MATCH : PEVENT_ERRNO__FILTER_MISS; } static char *op_to_str(struct event_filter *filter, struct filter_arg *arg)