| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20120406132158.GG30425@liondog.tnic>
Date: Fri, 6 Apr 2012 15:21:58 +0200
From: Borislav Petkov <bp@...en8.de>
To: Frederic Weisbecker <fweisbec@...il.com>
Cc: LKML <linux-kernel@...r.kernel.org>,
Steven Rostedt <srostedt@...hat.com>,
Steven Rostedt <rostedt@...dmis.org>,
Ingo Molnar <mingo@...nel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Peter Zijlstra <peterz@...radead.org>,
Arnaldo Carvalho de Melo <acme@...radead.org>,
Jiri Olsa <jolsa@...hat.com>, Arun Sharma <asharma@...com>,
Namhyung Kim <namhyung.kim@....com>
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@...hat.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@...dmis.org>
> Cc: Ingo Molnar <mingo@...nel.org>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Arnaldo Carvalho de Melo <acme@...radead.org>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> Cc: Borislav Petkov <bp@...en8.de>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Arun Sharma <asharma@...com>
> Cc: Namhyung Kim <namhyung.kim@....com>
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.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@...hat.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists