Messages in this thread | | | Date | Fri, 6 Apr 2012 15:21:58 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH 02/15] tools/events: Add files to create libtraceevent.a |
| |
On Fri, Apr 06, 2012 at 12:47:53AM +0200, Frederic Weisbecker wrote: > From: Steven Rostedt <srostedt@redhat.com> > > Copy over the files from trace-cmd to the Linux tools directory > such that applications like perf and latencytrace can use the > more advanced parsing code. > > Because some of the file names of perf conflict with trace-cmd file > names, the trace-cmd files have been renamed as follows: > > parse-events.c ==> event-parse.c > parse-events.h ==> event-parse.h > utils.h ==> event-utils.h > > The files have been updated to handle the changes to the header files > but other than that, they are identical to what was in the trace-cmd > repository. The history of these files, including authorship is > available at the git repo: > > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/trace-cmd.git > > The Makefile was also copied over, but most of it was removed to > focus on the parse-events code first. The parts of the Makefile for > the plugins have also been removed, but will be added back when the > plugin code is copied over as well. But that may be in its own > separate directory. > > Signed-off-by: Steven Rostedt <rostedt@goodmis.org> > Cc: Ingo Molnar <mingo@kernel.org> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Arnaldo Carvalho de Melo <acme@infradead.org> > Cc: Steven Rostedt <rostedt@goodmis.org> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Jiri Olsa <jolsa@redhat.com> > Cc: Arun Sharma <asharma@fb.com> > Cc: Namhyung Kim <namhyung.kim@lge.com> > Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com> > ---
<snip>
Btw, running this through checkpatch generates a load of warnings/errors and some of them actually could be worth fixing...
> diff --git a/tools/lib/traceevent/event-parse.c b/tools/lib/traceevent/event-parse.c > new file mode 100644 > index 0000000..47a3227 > --- /dev/null > +++ b/tools/lib/traceevent/event-parse.c > @@ -0,0 +1,4971 @@ > +/* > + * Copyright (C) 2009, 2010 Red Hat Inc, Steven Rostedt <srostedt@redhat.com> > + * > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; > + * version 2.1 of the License (not later!) > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this program; if not, write to the Free Software > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. > + * > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > + * > + * The parts for function graph printing was taken and modified from the > + * Linux Kernel that were written by > + * - Copyright (C) 2009 Frederic Weisbecker, > + * Frederic Weisbecker gave his permission to relicense the code to > + * the Lesser General Public License. > + */ > +#define _GNU_SOURCE > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > +#include <stdarg.h> > +#include <ctype.h> > +#include <errno.h> > + > +#include "event-parse.h" > + > +static const char *input_buf; > +static unsigned long long input_buf_ptr; > +static unsigned long long input_buf_siz; > + > +static int show_warning = 1; > + > +#define do_warning(fmt, ...) \ > + do { \ > + if (show_warning) \ > + warning(fmt, ##__VA_ARGS__); \ > + } while (0) > +
Yeah, the name "do_warning" is awkward. Can we call it emit_warning or _warning or whatever?
> +static void init_input_buf(const char *buf, unsigned long long size) > +{ > + input_buf = buf; > + input_buf_siz = size; > + input_buf_ptr = 0; > +} > + > +const char *pevent_get_input_buf(void) > +{ > + return input_buf; > +} > + > +unsigned long long pevent_get_input_buf_ptr(void) > +{ > + return input_buf_ptr; > +} > + > +struct event_handler { > + struct event_handler *next; > + int id; > + const char *sys_name; > + const char *event_name; > + pevent_event_handler_func func; > + void *context; > +}; > + > +struct pevent_func_params { > + struct pevent_func_params *next; > + enum pevent_func_arg_type type; > +}; > + > +struct pevent_function_handler { > + struct pevent_function_handler *next; > + enum pevent_func_arg_type ret_type; > + char *name; > + pevent_func_handler func; > + struct pevent_func_params *params; > + int nr_args; > +}; > + > +static unsigned long long > +process_defined_func(struct trace_seq *s, void *data, int size, > + struct event_format *event, struct print_arg *arg); > + > +static void free_func_handle(struct pevent_function_handler *func); > + > +/**
I see you're using kernel-doc style comments. Is this aimed at providing documentation for this library? If so, good idea!
> + * pevent_buffer_init - init buffer for parsing > + * @buf: buffer to parse > + * @size: the size of the buffer > + * > + * For use with pevent_read_token(), this initializes the internal > + * buffer that pevent_read_token() will parse. > + */ > +void pevent_buffer_init(const char *buf, unsigned long long size) > +{ > + init_input_buf(buf, size); > +}
Can't we unify pevent_buffer_init and init_input_buf()? I mean, it is useless to have that kind of indirection.
In general, is "pevent" the prefix for this library's functions which are getting exported to users?
> + > +void breakpoint(void) > +{ > + static int x; > + x++; > +}
This looks like a debugging leftover since it is not used anywhere...?
> + > +struct print_arg *alloc_arg(void) > +{ > + struct print_arg *arg; > + > + arg = malloc_or_die(sizeof(*arg)); > + if (!arg) > + return NULL; > + memset(arg, 0, sizeof(*arg)); > + > + return arg; > +} > + > +struct cmdline { > + char *comm; > + int pid; > +}; > + > +static int cmdline_cmp(const void *a, const void *b) > +{ > + const struct cmdline *ca = a; > + const struct cmdline *cb = b; > + > + if (ca->pid < cb->pid) > + return -1; > + if (ca->pid > cb->pid) > + return 1; > + > + return 0; > +} > + > +struct cmdline_list { > + struct cmdline_list *next; > + char *comm; > + int pid; > +}; > + > +static int cmdline_init(struct pevent *pevent) > +{ > + struct cmdline_list *cmdlist = pevent->cmdlist; > + struct cmdline_list *item; > + struct cmdline *cmdlines; > + int i; > + > + cmdlines = malloc_or_die(sizeof(*cmdlines) * pevent->cmdline_count); > + > + i = 0; > + while (cmdlist) { > + cmdlines[i].pid = cmdlist->pid; > + cmdlines[i].comm = cmdlist->comm; > + i++; > + item = cmdlist; > + cmdlist = cmdlist->next; > + free(item); > + } > + > + qsort(cmdlines, pevent->cmdline_count, sizeof(*cmdlines), cmdline_cmp); > + > + pevent->cmdlines = cmdlines; > + pevent->cmdlist = NULL; > + > + return 0; > +} > + > +static char *find_cmdline(struct pevent *pevent, int pid) > +{ > + const struct cmdline *comm; > + struct cmdline key; > + > + if (!pid) > + return "<idle>"; > + > + if (!pevent->cmdlines) > + cmdline_init(pevent); > + > + key.pid = pid; > + > + comm = bsearch(&key, pevent->cmdlines, pevent->cmdline_count, > + sizeof(*pevent->cmdlines), cmdline_cmp); > + > + if (comm) > + return comm->comm; > + return "<...>"; > +} > + > +/** > + * pevent_pid_is_registered - return if a pid has a cmdline registered > + * @pevent: handle for the pevent > + * @pid: The pid to check if it has a cmdline registered with. > + * > + * Returns 1 if the pid has a cmdline mapped to it > + * 0 otherwise. > + */ > +int pevent_pid_is_registered(struct pevent *pevent, int pid) > +{ > + const struct cmdline *comm; > + struct cmdline key; > + > + if (!pid) > + return 1; > + > + if (!pevent->cmdlines) > + cmdline_init(pevent); > + > + key.pid = pid; > + > + comm = bsearch(&key, pevent->cmdlines, pevent->cmdline_count, > + sizeof(*pevent->cmdlines), cmdline_cmp); > + > + if (comm) > + return 1; > + return 0; > +}
This one looks almost identical to find_cmdline() above, maybe unify?
> + > +/* > + * If the command lines have been converted to an array, then > + * we must add this pid. This is much slower than when cmdlines > + * are added before the array is initialized. > + */ > +static int add_new_comm(struct pevent *pevent, const char *comm, int pid) > +{ > + struct cmdline *cmdlines = pevent->cmdlines; > + const struct cmdline *cmdline; > + struct cmdline key; > + > + if (!pid) > + return 0; > + > + /* avoid duplicates */ > + key.pid = pid; > + > + cmdline = bsearch(&key, pevent->cmdlines, pevent->cmdline_count, > + sizeof(*pevent->cmdlines), cmdline_cmp); > + if (cmdline) { > + errno = EEXIST; > + return -1; > + } > + > + cmdlines = realloc(cmdlines, sizeof(*cmdlines) * (pevent->cmdline_count + 1)); > + if (!cmdlines) { > + errno = ENOMEM; > + return -1; > + } > + > + cmdlines[pevent->cmdline_count].pid = pid; > + cmdlines[pevent->cmdline_count].comm = strdup(comm); > + if (!cmdlines[pevent->cmdline_count].comm) > + die("malloc comm"); > +
extraneous whitespace
> + if (cmdlines[pevent->cmdline_count].comm) > + pevent->cmdline_count++; > + > + qsort(cmdlines, pevent->cmdline_count, sizeof(*cmdlines), cmdline_cmp); > + pevent->cmdlines = cmdlines; > + > + return 0; > +} > + > +/** > + * pevent_register_comm - register a pid / comm mapping > + * @pevent: handle for the pevent > + * @comm: the command line to register > + * @pid: the pid to map the command line to > + * > + * This adds a mapping to search for command line names with > + * a given pid. The comm is duplicated. > + */ > +int pevent_register_comm(struct pevent *pevent, const char *comm, int pid) > +{ > + struct cmdline_list *item; > + > + if (pevent->cmdlines) > + return add_new_comm(pevent, comm, pid); > + > + item = malloc_or_die(sizeof(*item)); > + item->comm = strdup(comm); > + if (!item->comm) > + die("malloc comm"); > + item->pid = pid; > + item->next = pevent->cmdlist; > + > + pevent->cmdlist = item; > + pevent->cmdline_count++; > + > + return 0; > +} > + > +struct func_map { > + unsigned long long addr; > + char *func; > + char *mod; > +}; > + > +struct func_list { > + struct func_list *next; > + unsigned long long addr; > + char *func; > + char *mod; > +}; > + > +static int func_cmp(const void *a, const void *b) > +{ > + const struct func_map *fa = a; > + const struct func_map *fb = b; > + > + if (fa->addr < fb->addr) > + return -1; > + if (fa->addr > fb->addr) > + return 1; > + > + return 0; > +} > + > +/* > + * We are searching for a record in between, not an exact > + * match. > + */ > +static int func_bcmp(const void *a, const void *b) > +{ > + const struct func_map *fa = a; > + const struct func_map *fb = b; > + > + if ((fa->addr == fb->addr) || > +
superfluous newline?
> + (fa->addr > fb->addr && > + fa->addr < (fb+1)->addr)) > + return 0; > + > + if (fa->addr < fb->addr) > + return -1; > + > + return 1; > +} > + > +static int func_map_init(struct pevent *pevent) > +{ > + struct func_list *funclist; > + struct func_list *item; > + struct func_map *func_map; > + int i; > + > + func_map = malloc_or_die(sizeof(*func_map) * (pevent->func_count + 1)); > + funclist = pevent->funclist; > + > + i = 0; > + while (funclist) { > + func_map[i].func = funclist->func; > + func_map[i].addr = funclist->addr; > + func_map[i].mod = funclist->mod; > + i++; > + item = funclist; > + funclist = funclist->next; > + free(item); > + } > + > + qsort(func_map, pevent->func_count, sizeof(*func_map), func_cmp); > + > + /* > + * Add a special record at the end. > + */ > + func_map[pevent->func_count].func = NULL; > + func_map[pevent->func_count].addr = 0; > + func_map[pevent->func_count].mod = NULL; > + > + pevent->func_map = func_map; > + pevent->funclist = NULL; > + > + return 0; > +} > + > +static struct func_map * > +find_func(struct pevent *pevent, unsigned long long addr) > +{ > + struct func_map *func; > + struct func_map key; > + > + if (!pevent->func_map) > + func_map_init(pevent); > + > + key.addr = addr; > + > + func = bsearch(&key, pevent->func_map, pevent->func_count, > + sizeof(*pevent->func_map), func_bcmp); > + > + return func;
Simply do:
return bsearch(&key, ...);
> +} > + > +/** > + * pevent_find_function - find a function by a given address > + * @pevent: handle for the pevent > + * @addr: the address to find the function with > + * > + * Returns a pointer to the function stored that has the given > + * address. Note, the address does not have to be exact, it > + * will select the function that would contain the address. > + */ > +const char *pevent_find_function(struct pevent *pevent, unsigned long long addr) > +{ > + struct func_map *map; > + > + map = find_func(pevent, addr); > + if (!map) > + return NULL; > + > + return map->func; > +} > + > +/** > + * pevent_find_function_address - find a function address by a given address > + * @pevent: handle for the pevent > + * @addr: the address to find the function with > + * > + * Returns the address the function starts at. This can be used in > + * conjunction with pevent_find_function to print both the function > + * name and the function offset. > + */ > +unsigned long long > +pevent_find_function_address(struct pevent *pevent, unsigned long long addr) > +{ > + struct func_map *map; > + > + map = find_func(pevent, addr); > + if (!map) > + return 0; > + > + return map->addr; > +} > + > +/** > + * pevent_register_function - register a function with a given address > + * @pevent: handle for the pevent > + * @function: the function name to register > + * @addr: the address the function starts at > + * @mod: the kernel module the function may be in (NULL for none) > + * > + * This registers a function name with an address and module. > + * The @func passed in is duplicated. > + */ > +int pevent_register_function(struct pevent *pevent, char *func, > + unsigned long long addr, char *mod) > +{ > + struct func_list *item; > + > + item = malloc_or_die(sizeof(*item)); > + > + item->next = pevent->funclist; > + item->func = strdup(func); > + if (mod) > + item->mod = strdup(mod); > + else > + item->mod = NULL; > + item->addr = addr; > + > + pevent->funclist = item; > + > + pevent->func_count++; > + > + return 0; > +} > + > +/** > + * pevent_print_funcs - print out the stored functions > + * @pevent: handle for the pevent > + * > + * This prints out the stored functions. > + */ > +void pevent_print_funcs(struct pevent *pevent) > +{ > + int i; > + > + if (!pevent->func_map) > + func_map_init(pevent); > + > + for (i = 0; i < (int)pevent->func_count; i++) {
simply declare i as "unsigned int" and you don't need the cast here.
> + printf("%016llx %s", > + pevent->func_map[i].addr, > + pevent->func_map[i].func); > + if (pevent->func_map[i].mod) > + printf(" [%s]\n", pevent->func_map[i].mod); > + else > + printf("\n"); > + } > +} > + > +struct printk_map { > + unsigned long long addr; > + char *printk; > +}; > + > +struct printk_list { > + struct printk_list *next; > + unsigned long long addr; > + char *printk; > +}; > + > +static int printk_cmp(const void *a, const void *b) > +{ > + const struct func_map *fa = a; > + const struct func_map *fb = b; > + > + if (fa->addr < fb->addr) > + return -1; > + if (fa->addr > fb->addr) > + return 1; > + > + return 0; > +} > + > +static void printk_map_init(struct pevent *pevent) > +{ > + struct printk_list *printklist; > + struct printk_list *item; > + struct printk_map *printk_map; > + int i; > + > + printk_map = malloc_or_die(sizeof(*printk_map) * (pevent->printk_count + 1)); > + > + printklist = pevent->printklist; > + > + i = 0; > + while (printklist) { > + printk_map[i].printk = printklist->printk; > + printk_map[i].addr = printklist->addr; > + i++; > + item = printklist; > + printklist = printklist->next; > + free(item); > + } > + > + qsort(printk_map, pevent->printk_count, sizeof(*printk_map), printk_cmp); > + > + pevent->printk_map = printk_map; > + pevent->printklist = NULL; > +} > + > +static struct printk_map * > +find_printk(struct pevent *pevent, unsigned long long addr) > +{ > + struct printk_map *printk; > + struct printk_map key; > + > + if (!pevent->printk_map) > + printk_map_init(pevent);
You could pull those existence tests in the _init function above, i.e:
static void printk_map_init(struct pevent *pevent) { if (pevent->printk_map) return;
so that you can keep the init details to the init functions. Ditto for the remaining _init(..) calls.
> + > + key.addr = addr; > + > + printk = bsearch(&key, pevent->printk_map, pevent->printk_count, > + sizeof(*pevent->printk_map), printk_cmp); > + > + return printk; > +} > + > +/** > + * pevent_register_print_string - register a string by its address > + * @pevent: handle for the pevent > + * @fmt: the string format to register > + * @addr: the address the string was located at > + * > + * This registers a string by the address it was stored in the kernel. > + * The @fmt passed in is duplicated. > + */ > +int pevent_register_print_string(struct pevent *pevent, char *fmt, > + unsigned long long addr) > +{ > + struct printk_list *item; > + > + item = malloc_or_die(sizeof(*item)); > + > + item->next = pevent->printklist; > + pevent->printklist = item; > + item->printk = strdup(fmt); > + item->addr = addr; > + > + pevent->printk_count++; > + > + return 0; > +} > + > +/** > + * pevent_print_printk - print out the stored strings > + * @pevent: handle for the pevent > + * > + * This prints the string formats that were stored. > + */ > +void pevent_print_printk(struct pevent *pevent) > +{ > + int i; > + > + if (!pevent->printk_map) > + printk_map_init(pevent);
ditto.
> + > + for (i = 0; i < (int)pevent->printk_count; i++) {
ditto for making i an unsigned int.
> + printf("%016llx %s\n", > + pevent->printk_map[i].addr, > + pevent->printk_map[i].printk); > + } > +} > + > +static struct event_format *alloc_event(void) > +{ > + struct event_format *event; > + > + event = malloc_or_die(sizeof(*event)); > + memset(event, 0, sizeof(*event));
perf (or tools/) probably needs a zalloc_or_die which memset's the ptr to 0 - this'll save a bunch of code.
> + > + return event; > +} > + > +static void add_event(struct pevent *pevent, struct event_format *event) > +{ > + int i; > + > + if (!pevent->events) > + pevent->events = malloc_or_die(sizeof(event)); > + else > + pevent->events = > + realloc(pevent->events, sizeof(event) * > + (pevent->nr_events + 1)); > + if (!pevent->events) > + die("Can not allocate events"); > + > + for (i = 0; i < pevent->nr_events; i++) { > + if (pevent->events[i]->id > event->id) > + break; > + } > + if (i < pevent->nr_events) > + memmove(&pevent->events[i + 1], > + &pevent->events[i], > + sizeof(event) * (pevent->nr_events - i)); > + > + pevent->events[i] = event; > + pevent->nr_events++; > + > + event->pevent = pevent; > +} > + > +static int event_item_type(enum event_type type) > +{ > + switch (type) { > + case EVENT_ITEM ... EVENT_SQUOTE: > + return 1; > + case EVENT_ERROR ... EVENT_DELIM: > + default: > + return 0; > + } > +} > + > +static void free_flag_sym(struct print_flag_sym *fsym) > +{ > + struct print_flag_sym *next; > + > + while (fsym) { > + next = fsym->next; > + free(fsym->value); > + free(fsym->str); > + free(fsym); > + fsym = next; > + } > +} > + > +static void free_arg(struct print_arg *arg) > +{ > + struct print_arg *farg; > + > + if (!arg) > + return; > + > + switch (arg->type) { > + case PRINT_ATOM: > + free(arg->atom.atom); > + break; > + case PRINT_FIELD: > + free(arg->field.name); > + break; > + case PRINT_FLAGS: > + free_arg(arg->flags.field); > + free(arg->flags.delim); > + free_flag_sym(arg->flags.flags); > + break; > + case PRINT_SYMBOL: > + free_arg(arg->symbol.field); > + free_flag_sym(arg->symbol.symbols); > + break; > + case PRINT_TYPE: > + free(arg->typecast.type); > + free_arg(arg->typecast.item); > + break; > + case PRINT_STRING: > + case PRINT_BSTRING: > + free(arg->string.string); > + break; > + case PRINT_DYNAMIC_ARRAY: > + free(arg->dynarray.index); > + break; > + case PRINT_OP: > + free(arg->op.op); > + free_arg(arg->op.left); > + free_arg(arg->op.right); > + break; > + case PRINT_FUNC: > + while (arg->func.args) { > + farg = arg->func.args; > + arg->func.args = farg->next; > + free_arg(farg); > + } > + break; > + > + case PRINT_NULL: > + default: > + break; > + } > + > + free(arg); > +} > + > +static enum event_type get_type(int ch) > +{ > + if (ch == '\n') > + return EVENT_NEWLINE; > + if (isspace(ch)) > + return EVENT_SPACE; > + if (isalnum(ch) || ch == '_') > + return EVENT_ITEM; > + if (ch == '\'') > + return EVENT_SQUOTE; > + if (ch == '"') > + return EVENT_DQUOTE; > + if (!isprint(ch)) > + return EVENT_NONE; > + if (ch == '(' || ch == ')' || ch == ',') > + return EVENT_DELIM; > + > + return EVENT_OP; > +} > + > +static int __read_char(void) > +{ > + if (input_buf_ptr >= input_buf_siz) > + return -1; > + > + return input_buf[input_buf_ptr++]; > +} > + > +static int __peek_char(void) > +{ > + if (input_buf_ptr >= input_buf_siz) > + return -1; > + > + return input_buf[input_buf_ptr]; > +} > + > +/** > + * pevent_peek_char - peek at the next character that will be read > + * > + * Returns the next character read, or -1 if end of buffer. > + */ > +int pevent_peek_char(void) > +{ > + return __peek_char(); > +} > + > +static enum event_type force_token(const char *str, char **tok); > + > +static enum event_type __read_token(char **tok) > +{ > + char buf[BUFSIZ]; > + int ch, last_ch, quote_ch, next_ch; > + int i = 0; > + int tok_size = 0; > + enum event_type type; > + > + *tok = NULL; > + > + > + ch = __read_char(); > + if (ch < 0) > + return EVENT_NONE; > + > + type = get_type(ch); > + if (type == EVENT_NONE) > + return type; > + > + buf[i++] = ch; > + > + switch (type) { > + case EVENT_NEWLINE: > + case EVENT_DELIM: > + *tok = malloc_or_die(2); > + (*tok)[0] = ch; > + (*tok)[1] = 0; > + return type; > + > + case EVENT_OP: > + switch (ch) { > + case '-': > + next_ch = __peek_char(); > + if (next_ch == '>') { > + buf[i++] = __read_char(); > + break; > + } > + /* fall through */ > + case '+': > + case '|': > + case '&': > + case '>': > + case '<': > + last_ch = ch; > + ch = __peek_char(); > + if (ch != last_ch) > + goto test_equal; > + buf[i++] = __read_char(); > + switch (last_ch) { > + case '>': > + case '<': > + goto test_equal; > + default: > + break; > + } > + break; > + case '!': > + case '=': > + goto test_equal; > + default: /* what should we do instead? */ > + break; > + } > + buf[i] = 0; > + *tok = strdup(buf); > + return type; > + > + test_equal: > + ch = __peek_char(); > + if (ch == '=') > + buf[i++] = __read_char(); > + goto out; > + > + case EVENT_DQUOTE: > + case EVENT_SQUOTE: > + /* don't keep quotes */ > + i--; > + quote_ch = ch; > + last_ch = 0; > + concat: > + do { > + if (i == (BUFSIZ - 1)) { > + buf[i] = 0; > + if (*tok) { > + *tok = realloc(*tok, tok_size + BUFSIZ); > + if (!*tok) > + return EVENT_NONE; > + strcat(*tok, buf); > + } else > + *tok = strdup(buf); > + > + if (!*tok) > + return EVENT_NONE; > + tok_size += BUFSIZ; > + i = 0; > + } > + last_ch = ch; > + ch = __read_char(); > + buf[i++] = ch; > + /* the '\' '\' will cancel itself */ > + if (ch == '\\' && last_ch == '\\') > + last_ch = 0; > + } while (ch != quote_ch || last_ch == '\\'); > + /* remove the last quote */ > + i--; > + > + /* > + * For strings (double quotes) check the next token. > + * If it is another string, concatinate the two. > + */ > + if (type == EVENT_DQUOTE) { > + unsigned long long save_input_buf_ptr = input_buf_ptr; > + > + do { > + ch = __read_char(); > + } while (isspace(ch)); > + if (ch == '"') > + goto concat; > + input_buf_ptr = save_input_buf_ptr; > + } > + > + goto out; > + > + case EVENT_ERROR ... EVENT_SPACE: > + case EVENT_ITEM: > + default: > + break; > + } > + > + while (get_type(__peek_char()) == type) { > + if (i == (BUFSIZ - 1)) { > + buf[i] = 0; > + if (*tok) { > + *tok = realloc(*tok, tok_size + BUFSIZ); > + if (!*tok) > + return EVENT_NONE; > + strcat(*tok, buf); > + } else > + *tok = strdup(buf); > + > + if (!*tok) > + return EVENT_NONE; > + tok_size += BUFSIZ; > + i = 0; > + } > + ch = __read_char(); > + buf[i++] = ch; > + } > + > + out: > + buf[i] = 0; > + if (*tok) { > + *tok = realloc(*tok, tok_size + i); > + if (!*tok) > + return EVENT_NONE; > + strcat(*tok, buf); > + } else > + *tok = strdup(buf); > + if (!*tok) > + return EVENT_NONE; > + > + if (type == EVENT_ITEM) { > + /* > + * Older versions of the kernel has a bug that > + * creates invalid symbols and will break the mac80211 > + * parsing. This is a work around to that bug. > + * > + * See Linux kernel commit: > + * 811cb50baf63461ce0bdb234927046131fc7fa8b > + */ > + if (strcmp(*tok, "LOCAL_PR_FMT") == 0) { > + free(*tok); > + *tok = NULL; > + return force_token("\"\%s\" ", tok); > + } else if (strcmp(*tok, "STA_PR_FMT") == 0) { > + free(*tok); > + *tok = NULL; > + return force_token("\" sta:%pM\" ", tok); > + } else if (strcmp(*tok, "VIF_PR_FMT") == 0) { > + free(*tok); > + *tok = NULL; > + return force_token("\" vif:%p(%d)\" ", tok); > + } > + } > + > + return type; > +} > + > +static enum event_type force_token(const char *str, char **tok) > +{ > + const char *save_input_buf; > + unsigned long long save_input_buf_ptr; > + unsigned long long save_input_buf_siz; > + enum event_type type; > +
Superfluous \t
> + /* save off the current input pointers */ > + save_input_buf = input_buf; > + save_input_buf_ptr = input_buf_ptr; > + save_input_buf_siz = input_buf_siz; > + > + init_input_buf(str, strlen(str)); > + > + type = __read_token(tok); > + > + /* reset back to original token */ > + input_buf = save_input_buf; > + input_buf_ptr = save_input_buf_ptr; > + input_buf_siz = save_input_buf_siz; > + > + return type; > +} > + > +static void free_token(char *tok) > +{ > + if (tok) > + free(tok); > +}
According to MALLOC(3), free() can stomach NULL ptrs: "If ptr is NULL, no operation is performed."
> + > +static enum event_type read_token(char **tok) > +{ > + enum event_type type; > + > + for (;;) { > + type = __read_token(tok); > + if (type != EVENT_SPACE) > + return type; > + > + free_token(*tok); > + } > + > + /* not reached */ > + *tok = NULL; > + return EVENT_NONE; > +} > + > +/** > + * pevent_read_token - access to utilites to use the pevent parser > + * @tok: The token to return > + * > + * This will parse tokens from the string given by > + * pevent_init_data(). > + * > + * Returns the token type. > + */ > +enum event_type pevent_read_token(char **tok) > +{ > + return read_token(tok); > +}
Same here: can't we remove all those wrappers and have the library routines be called "pevent_" or whatever and all users of the library simply call them?
> + > +/** > + * pevent_free_token - free a token returned by pevent_read_token > + * @token: the token to free > + */ > +void pevent_free_token(char *token) > +{ > + free_token(token); > +}
Ditto.
> + > +/* no newline */
Please elaborate. Ah, event type cannot be NEWLINE additionally... Well, if that enum event_type were bits, you could unify this function with the read_token() above.
> +static enum event_type read_token_item(char **tok) > +{ > + enum event_type type; > + > + for (;;) { > + type = __read_token(tok); > + if (type != EVENT_SPACE && type != EVENT_NEWLINE) > + return type; > + free_token(*tok); > + *tok = NULL; > + } > + > + /* not reached */ > + *tok = NULL; > + return EVENT_NONE; > +}
Ok, enough for today. I'll review the rest only if you really want me to :-).
-- Regards/Gruss, Boris.
| |