[PATCH v7 fix 5/8] Implement new filtering language parsing
Eugene Syromiatnikov
esyr at redhat.com
Mon Aug 14 01:59:24 UTC 2017
On Sat, Aug 12, 2017 at 03:23:24PM +0700, Nikolay Marchuk wrote:
> * basic_filters.c (parse_set, parse_syscall_set): Use set inversion only in
> qualify mode.
> * filter.h (parse_filter_action, parse_qualify_action,
> parse_filter_expression): Add new declarations.
> * filter_action.c (parse_filter_action): Add new parsing function.
> (filtering_parse_finish): Use filtering_parse instead of
> parse_qualify_filter.
> * filter_expression.c (parse_filter_expression): Implement parsing of filter
> expression.
> (add_variable_token, add_operator_token, parse_operator, push_operator,
> is_higher_priority, is_allowed_in_name): Add helper functions.
> (is_space_ascii): Add new definition.
> * filter_parse.c: New file.
> * filter_qualify.c (parse_read, parse_write, qualify_signals, parse_trace,
> parse_abbrev, parse_verbose, parse_raw, parse_inject_common, parse_fault,
> parse_inject): Use new parsing API.
> * strace.c (init): Use filtering_parse instead of parse_qualify_filter.
> * Makefile.am (strace_SOURCES): Add filter_parse.c.
> * defs.h (filtering_parse): Add new definition.
> ---
> Makefile.am | 1 +
> basic_filters.c | 34 +++---
> defs.h | 2 +-
> filter.h | 7 ++
> filter_action.c | 14 ++-
> filter_expression.c | 220 +++++++++++++++++++++++++++++++++++
> filter_parse.c | 223 ++++++++++++++++++++++++++++++++++++
> filter_qualify.c | 99 +++++++++-------
> strace.c | 6 +-
> tests/filtering_syscall-syntax.test | 17 +--
> 10 files changed, 546 insertions(+), 77 deletions(-)
> create mode 100644 filter_parse.c
>
> diff --git a/Makefile.am b/Makefile.am
> index 84df39a8..ae10dc5f 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -136,6 +136,7 @@ strace_SOURCES = \
> file_ioctl.c \
> filter_action.c \
> filter_expression.c \
> + filter_parse.c \
> filter_qualify.c \
> filter.c \
> filter.h \
> diff --git a/basic_filters.c b/basic_filters.c
> index ada7b1a5..f12c9046 100644
> --- a/basic_filters.c
> +++ b/basic_filters.c
> @@ -254,15 +254,17 @@ parse_syscall_set(const char *const str, struct number_set *const set)
> unsigned int p;
> const char *s = str;
>
> - /*
> - * Each leading ! character means inversion
> - * of the remaining specification.
> - */
> - while (*s == '!') {
> - for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> - set[p].not = !set[p].not;
> + if (parsing_qualify_mode) {
It's better to pass such parameters via arguments and not via global
variable.
> + /*
> + * Each leading ! character means inversion
> + * of the remaining specification.
> + */
> + while (*s == '!') {
> + for (p = 0; p < SUPPORTED_PERSONALITIES; ++p) {
> + set[p].not = !set[p].not;
> + }
> + ++s;
> }
> - ++s;
> }
>
> if (strcmp(s, "none") == 0) {
> @@ -345,13 +347,15 @@ parse_set(const char *const str, struct number_set *const set,
> {
> const char *s = str;
>
> - /*
> - * Each leading ! character means inversion
> - * of the remaining specification.
> - */
> - while (*s == '!') {
> - set->not = !set->not;
> - ++s;
> + if (parsing_qualify_mode) {
Ditto here.
> + /*
> + * Each leading ! character means inversion
> + * of the remaining specification.
> + */
> + while (*s == '!') {
> + set->not = !set->not;
> + ++s;
> + }
> }
>
> if (strcmp(s, "none") == 0) {
> diff --git a/defs.h b/defs.h
> index 69cd765e..13893f64 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -670,7 +670,7 @@ extern struct number_set signal_set;
> extern bool is_number_in_set(unsigned int number, const struct number_set *);
> extern void filtering_parsing_finish(void);
> extern void filter_syscall(struct tcb *);
> -extern void parse_qualify_filter(const char *);
> +extern void filtering_parse(const char *);
>
> #define DECL_IOCTL(name) \
> extern int \
> diff --git a/filter.h b/filter.h
> index 7509b1e5..3861f8b8 100644
> --- a/filter.h
> +++ b/filter.h
> @@ -35,6 +35,8 @@ struct filter_action;
>
> struct bool_expression;
>
> +extern bool parsing_qualify_mode;
> +
> typedef int (*string_to_uint_func)(const char *);
> void parse_set(const char *const, struct number_set *const,
> string_to_uint_func, const char *const);
> @@ -56,6 +58,7 @@ void set_filters_qualify_mode(struct filter **, unsigned int *nfilters);
> /* filter action api */
> struct filter *create_filter(struct filter_action *, const char *name);
> struct filter_action *find_or_add_action(const char *);
> +void parse_filter_action(const char *, const char *, const char *);
> void *get_filter_action_priv_data(struct filter_action *);
> void set_filter_action_priv_data(struct filter_action *, void *);
> void set_qualify_mode(struct filter_action *);
> @@ -64,5 +67,9 @@ void set_qualify_mode(struct filter_action *);
> struct bool_expression *create_expression();
> bool run_expression(struct bool_expression *, bool *, unsigned int);
> void set_expression_qualify_mode(struct bool_expression *);
> +void parse_filter_expression(struct bool_expression *, const char *,
> + struct filter_action *, unsigned int);
> +
> +void parse_qualify_action(const char *, const char *, const char *);
>
> #endif /* !STRACE_FILTER_H */
> diff --git a/filter_action.c b/filter_action.c
> index be4af4d3..776ffded 100644
> --- a/filter_action.c
> +++ b/filter_action.c
> @@ -127,7 +127,7 @@ filtering_parsing_finish(void)
>
> /* Init trace action if pathtracing is enabled */
> if (tracing_paths && (default_flags & QUAL_TRACE))
> - parse_qualify_filter("trace=all");
> + filtering_parse("trace=all");
>
> /* Sort actions by priority */
> if (nfilter_actions == 0)
> @@ -193,6 +193,18 @@ find_or_add_action(const char *name)
> return add_action(type);
> }
>
> +void
> +parse_filter_action(const char *action_name, const char *expr, const char *args)
> +{
> + struct filter_action *action = find_or_add_action(action_name);
> +
> + parse_filter_expression(action->expr, expr, action, action->nfilters);
> + if (args && action->type->parse_args == &parse_null)
> + error_msg("%s action takes no arguments, ignored argument '%s'",
> + action->type->name, args);
> + action->_priv_data = action->type->parse_args(args);
> +}
> +
> static void
> run_filter_action(struct tcb *tcp, struct filter_action *action)
> {
> diff --git a/filter_expression.c b/filter_expression.c
> index 05f8903e..c991bb2b 100644
> --- a/filter_expression.c
> +++ b/filter_expression.c
> @@ -27,6 +27,16 @@
>
> #include "defs.h"
> #include <stdarg.h>
> +#include "filter.h"
> +
> +extern bool is_space_ascii(char);
> +
> +static bool
> +is_allowed_in_name(char c)
> +{
> + return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z')
> + || (c >= '0' && c <= '9') || (c == '_');
> +}
>
> struct expression_token {
> enum token_type {
> @@ -42,6 +52,8 @@ struct expression_token {
> } operator_id;
> } data;
> };
> +/* Pseudo-operator for parsing */
> +#define OP_PARENTHESIS 3
>
> struct bool_expression {
> unsigned int ntokens;
> @@ -68,6 +80,25 @@ reallocate_expression(struct bool_expression *const expr,
> expr->ntokens = new_ntokens;
> }
>
> +static void
> +add_variable_token(struct bool_expression *expr, unsigned int id)
> +{
> + struct expression_token token;
> + token.type = TOK_VARIABLE;
> + token.data.variable_id = id;
> + reallocate_expression(expr, expr->ntokens + 1);
> + expr->tokens[expr->ntokens - 1] = token;
> +}
> +
> +static void
> +add_operator_token(struct bool_expression *expr, int op) {
> + struct expression_token token;
> + token.type = TOK_OPERATOR;
> + token.data.operator_id = op;
> + reallocate_expression(expr, expr->ntokens + 1);
> + expr->tokens[expr->ntokens - 1] = token;
> +}
> +
> void
> set_expression_qualify_mode(struct bool_expression *expr)
> {
> @@ -233,3 +264,192 @@ run_expression(struct bool_expression *expr, bool *variables,
> variables, variables_num);
> return stack[0];
> }
> +
> +/*
> + * Parse operator and add operator length to str and pos.
> + * Return -1 if no operator found.
> + */
> +static int
> +parse_operator(char **str, unsigned int *pos)
> +{
> +#define _OP(s, op) { s, sizeof(s) - 1, op }
> + struct {
> + const char *str;
> + int len;
> + enum operator_type op;
> + } ops[] = {
> + _OP("!", OP_NOT),
> + _OP("not", OP_NOT),
> + _OP("&&", OP_AND),
> + _OP("and", OP_AND),
> + _OP("||", OP_OR),
> + _OP("or", OP_OR),
> + };
> +#undef _OP
> + char *p = *str;
> + unsigned int i;
> +
> + for (i = 0; i < ARRAY_SIZE(ops); i++) {
> + if (!strncmp(p, ops[i].str, ops[i].len) &&
> + (!is_allowed_in_name(ops[i].str[0]) ||
> + !is_allowed_in_name(p[ops[i].len]))) {
> + *str += ops[i].len - 1;
> + *pos += ops[i].len - 1;
> + return ops[i].op;
> + }
> + }
> + return -1;
> +}
> +
> +static char *
> +unescape_argument(char **str)
> +{
> + char *p;
> + char *p_new;
> + bool escaped = false;
> + unsigned int size = 1;
> + char *new_str = xcalloc(strlen(*str) + 1, 1);
> +
> + for (p = *str, p_new = new_str; *p; ++p) {
> + if (!escaped) {
> + if (*p == '\\') {
> + escaped = true;
> + continue;
> + } else if (is_space_ascii(*p) || *p == ')' || *p == '|'
> + || *p == '&') {
> + break;
> + }
> + }
> + escaped = false;
> + *(p_new++) = *p;
> + size++;
> + }
> + *str = p - 1;
> + return xreallocarray(new_str, size, 1);;
Excess semicolon.
> +}
> +
> +static void
> +push_operator(int *stack, unsigned int *stack_size, int op)
> +{
> + if (*stack_size == MAX_STACK_SIZE)
> + error_msg_and_die("stack overflow");
> + stack[*stack_size] = op;
> + (*stack_size)++;
> +}
> +
> +static bool
> +is_higher_priority(int op_a, int op_b)
> +{
> + bool op_priority[] = {
> + [OP_NOT] = 2,
> + [OP_AND] = 1,
> + [OP_OR] = 0
Curiously, previous version of the patch had comma here.
> + };
> + return op_priority[op_a] > op_priority[op_b];
> +}
> +
> +void
> +parse_filter_expression(struct bool_expression *expr, const char *str,
> + struct filter_action *action, unsigned int start_id)
> +{
> + enum {
> + WAIT_FILTER,
> + FILTER_NAME,
> + FILTER_ARG,
> + WAIT_OPERATOR,
> + } state = WAIT_FILTER;
> + unsigned int variable_id = start_id;
> + /* Current stack stack_size */
> + unsigned int st_size = 0;
> + int stack[MAX_STACK_SIZE];
> + char *buf = xstrdup(str);
> + struct filter *cur_filter = NULL;
> + char *filter_name = NULL;
> + char *filter_arg = NULL;
> + int op;
> + char *p;
> + unsigned int pos = 0;
> +
> + for (p = buf; *p; ++p, ++pos) {
> + switch (state) {
> + case WAIT_FILTER:
> + if (*p == '(') {
> + push_operator(stack, &st_size, OP_PARENTHESIS);
> + } else if ((op = parse_operator(&p, &pos)) >= 0) {
> + if (op == OP_NOT) {
> + push_operator(stack, &st_size, op);
> + } else {
> + error_msg_and_die("invalid operator "
> + "at '%s':%u",
> + str, pos);
> + }
> + } else if (!is_space_ascii(*p)) {
> + filter_name = p;
> + state = FILTER_NAME;
> + }
> + break;
> +
> + case FILTER_NAME:
> + if (is_space_ascii(*p)) {
> + *p = '\0';
> + cur_filter = create_filter(action, filter_name);
> + filter_arg = NULL;
> + state = FILTER_ARG;
> + }
> + break;
> +
> + case FILTER_ARG:
> + if (!filter_arg && is_space_ascii(*p))
> + break;
> + filter_arg = unescape_argument(&p);
> + parse_filter(cur_filter, filter_arg);
> + free(filter_arg);
> + add_variable_token(expr, variable_id++);
> + state = WAIT_OPERATOR;
> + break;
> +
> + case WAIT_OPERATOR:
> + if (is_space_ascii(*p))
> + break;
> + if (*p == ')') {
> + while ((st_size > 0) &&
> + (stack[st_size - 1] != OP_PARENTHESIS)) {
> + op = stack[--st_size];
> + add_operator_token(expr, op);
> + }
> + if (st_size == 0)
> + error_msg_and_die("unexpected ')' at "
> + "'%s':%u", str, pos);
> + --st_size;
> + break;
> + }
> + op = parse_operator(&p, &pos);
> + if (op < 0 || op == OP_NOT)
> + error_msg_and_die("invalid operator at '%s':%u",
> + str, pos);
> +
> + /* Pop operators with higher priority. */
> + while ((st_size > 0) &&
> + (stack[st_size - 1] != OP_PARENTHESIS) &&
> + is_higher_priority(stack[st_size - 1], op))
> + add_operator_token(expr, stack[--st_size]);
> +
> + push_operator(stack, &st_size, op);
> + state = WAIT_FILTER;
> + break;
> + }
> + }
> +
> + free(buf);
> + if (state != WAIT_OPERATOR)
> + error_msg_and_die("unfinished filter expression '%s'", str);
> +
> + while (st_size > 0) {
> + if (stack[--st_size] == OP_PARENTHESIS)
> + error_msg_and_die("missing ')' in expression '%s'",
> + str);
> + add_operator_token(expr, stack[st_size]);
> + }
> + if (start_id > 0)
> + add_operator_token(expr, OP_OR);
> +}
> diff --git a/filter_parse.c b/filter_parse.c
> new file mode 100644
> index 00000000..dd69806f
> --- /dev/null
> +++ b/filter_parse.c
> @@ -0,0 +1,223 @@
> +/*
> + * Copyright (c) 2017 Nikolay Marchuk <marchuk.nikolay.a at gmail.com>
> + * All rights reserved.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + * notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + * notice, this list of conditions and the following disclaimer in the
> + * documentation and/or other materials provided with the distribution.
> + * 3. The name of the author may not be used to endorse or promote products
> + * derived from this software without specific prior written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR
> + * IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
> + * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED.
> + * IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT,
> + * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> + * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
> + * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include "defs.h"
> +#include "filter.h"
> +
> +bool
> +is_space_ascii(char c)
> +{
> + return (c == ' ') || (c == '\t') || (c == '\n') || (c == '\r') ||
> + (c == '\v') || (c == '\f');
> +}
> +
> +/*
> + * Split expression into action name, filter expression or qualify set
> + * and action arguments.
> + */
> +void
> +filtering_parse(const char *str)
> +{
> + enum parsing_states {
> + F_EMPTY,
> + F_BEGIN,
> + F_QUAL_SET,
> + F_FILT_EXPR,
> + F_QUAL_ARGS,
> + F_FILT_ARGS,
> + F_END,
> + } state = F_EMPTY;
> + const char *begin = NULL;
> + const char *action_name = NULL;
> + const char *main_part = NULL;
> + const char *args = NULL;
> + bool escaped = false;
> + int parentheses_count = 0;
> + /* Used to store position of last terminating parenthesis. */
> + char *expression_end = NULL;
> + /* Used to provide diagnostics. */
> + unsigned int pos = 0;
> + char *buf = xstrdup(str);
> + char *p;
> +
> + for (p = buf; *p; ++p, ++pos) {
> + switch (state) {
> + case F_EMPTY:
> + switch (*p) {
> + /* trace(), action name omitted */
> + case '(':
> + parentheses_count++;
> + action_name = "trace";
> + main_part = p + 1;
Btw, the fact that this change hadn't affected tests is remarkable.
> + state = F_FILT_EXPR;
> + break;
> + case '=':
> + error_msg_and_die("invalid filter action ''");
> + default:
> + if (!is_space_ascii(*p)) {
> + begin = p;
> + state = F_BEGIN;
> + }
> + }
> + if (state != F_BEGIN)
> + break;
> + /* else fall through to check for qualify set */
> +
> + case F_BEGIN:
> + switch (*p) {
> + /* action(...) */
> + case '(':
> + parentheses_count++;
> + /* expression without action with leading ! */
> + if (*begin == '!')
> + break;
> + action_name = begin;
> + *p = '\0';
> + main_part = p + 1;
> + state = F_FILT_EXPR;
> + break;
> + /* action=... */
> + case '=':
> + action_name = begin;
> + *p = '\0';
> + main_part = p + 1;
> + state = F_QUAL_SET;
> + break;
> + /* qualify set without action. */
> + case ',':
> + case '?':
> + case '/':
> + case '%':
> + case '-':
> + action_name = "trace";
> + main_part = begin;
> + state = F_QUAL_SET;
> + break;
> + default:
> + /* new expression without action. */
> + if (is_space_ascii(*p)) {
> + action_name = "trace";
> + main_part = begin;
> + state = F_FILT_EXPR;
> + }
> + }
> + break;
> +
> + case F_QUAL_SET:
> + if (*p == ':') {
> + *p = '\0';
> + args = p + 1;
> + state = F_QUAL_ARGS;
> + }
> + break;
> +
> + case F_FILT_EXPR:
> + if (!escaped) {
> + switch (*p) {
> + case ';':
> + if (parentheses_count != 1)
> + error_msg_and_die("invalid "
> + "arguments "
> + "position "
> + "'%s':%u",
> + str, pos);
> + *p = '\0';
> + args = p + 1;
> + state = F_FILT_ARGS;
> + break;
> + case '(':
> + parentheses_count++;
> + break;
> + case ')':
> + if (parentheses_count <= 0)
> + error_msg_and_die("unexpected "
> + "')' at "
> + "'%s':%u",
> + str, pos);
> + parentheses_count--;
> + expression_end = p;
> + break;
> + case '\\':
> + escaped = true;
> + break;
> + }
> + } else
> + escaped = false;
> + break;
> +
> + case F_QUAL_ARGS:
> + break;
> + case F_FILT_ARGS:
> + if (!escaped) {
> + switch (*p) {
> + case ')':
> + if (parentheses_count <= 0)
> + error_msg_and_die("unexpected "
> + "')' at "
> + "'%s':%u",
> + str, pos);
> + parentheses_count--;
> + expression_end = p;
> + state = F_END;
> + break;
> + case '\\':
> + escaped = true;
> + break;
> + }
> + } else
> + escaped = false;
> + break;
> + case F_END:
> + if (!is_space_ascii(*p))
> + error_msg_and_die("illegal character '%c' in "
> + "'%s':%u", *p, str, pos);
> + }
> + }
> +
> + switch (state) {
> + case F_EMPTY:
> + error_msg_and_die("invalid filter expression '%s'", str);
> + case F_BEGIN:
> + action_name = "trace";
> + main_part = begin;
> + /* fall through */
> + case F_QUAL_SET:
> + case F_QUAL_ARGS:
> + parse_qualify_action(action_name, main_part, args);
> + break;
> + case F_FILT_EXPR:
> + case F_FILT_ARGS:
> + case F_END:
> + if (parentheses_count != 0)
> + error_msg_and_die("missing ')' in '%s'", str);
> + if (expression_end)
> + *expression_end = '\0';
> + parse_filter_action(action_name, main_part, args);
> + break;
> + }
> + free(buf);
> +}
> diff --git a/filter_qualify.c b/filter_qualify.c
> index 3765f656..51f1a2ba 100644
> --- a/filter_qualify.c
> +++ b/filter_qualify.c
> @@ -181,27 +181,31 @@ parse_inject_common_args(char *str, struct inject_opts *const opts,
> }
>
> static void
> -parse_read(const char *const str)
> +parse_read(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("read");
> struct filter *filter = create_filter(action, "fd");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("read action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -parse_write(const char *const str)
> +parse_write(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("write");
> struct filter *filter = create_filter(action, "fd");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("write action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -qualify_signals(const char *const str)
> +qualify_signals(const char *const main_part, const char *const args)
> {
> /* Clear the set. */
> if (signal_set.nslots)
> @@ -209,89 +213,97 @@ qualify_signals(const char *const str)
> sizeof(*signal_set.vec) * signal_set.nslots);
> signal_set.not = false;
>
> - parse_set(str, &signal_set, sigstr_to_uint, "signal");
> + parse_set(main_part, &signal_set, sigstr_to_uint, "signal");
> + if (args)
> + error_msg("signal action takes no arguments: '%s'", args);
> }
>
> static void
> -parse_trace(const char *const str)
> +parse_trace(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("trace");
> struct filter *filter = create_filter(action, "syscall");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("trace action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -parse_abbrev(const char *const str)
> +parse_abbrev(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("abbrev");
> struct filter *filter = create_filter(action, "syscall");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("abbrev action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -parse_verbose(const char *const str)
> +parse_verbose(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("verbose");
> struct filter *filter = create_filter(action, "syscall");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("verbose action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -parse_raw(const char *const str)
> +parse_raw(const char *const main_part, const char *const args)
> {
> struct filter_action *action = find_or_add_action("raw");
> struct filter *filter = create_filter(action, "syscall");
>
> - parse_filter(filter, str);
> + parse_filter(filter, main_part);
> + if (args)
> + error_msg("raw action takes no arguments: '%s'", args);
> set_qualify_mode(action);
> }
>
> static void
> -parse_inject_common_qualify(const char *const str, const bool fault_tokens_only,
> - const char *const description)
> +parse_inject_common_qualify(const char *const main_part, const char *const args,
> + const bool fault_tokens_only, const char *const description)
> {
> struct inject_opts *opts = xmalloc(sizeof(struct inject_opts));
> - char *buf = xstrdup(str);
> + char *buf = args ? xstrdup(args) : NULL;
> struct filter_action *action;
> struct filter *filter;
> - char *args = strchr(buf, ':');
> -
> - if (args)
> - *(args++) = '\0';
>
> action = find_or_add_action(fault_tokens_only ? "fault" : "inject");
> filter = create_filter(action, "syscall");
> - parse_filter(filter, buf);
> + parse_filter(filter, main_part);
> set_qualify_mode(action);
> - parse_inject_common_args(args, opts, ":", fault_tokens_only);
> - if (!opts->init)
> + parse_inject_common_args(buf, opts, ":", fault_tokens_only);
> + if (!opts->init) {
Unexpected addition of curly braces.
> error_msg_and_die("invalid %s '%s'", description,
> args ? args : "");
> - free(buf);
> + }
> + if (buf)
> + free(buf);
> set_filter_action_priv_data(action, opts);
> }
>
> static void
> -parse_fault(const char *const str)
> +parse_fault(const char *const main_part, const char *const args)
> {
> - parse_inject_common_qualify(str, true, "fault argument");
> + parse_inject_common_qualify(main_part, args, true, "fault argument");
> }
>
> static void
> -parse_inject(const char *const str)
> +parse_inject(const char *const main_part, const char *const args)
> {
> - parse_inject_common_qualify(str, false, "inject argument");
> + parse_inject_common_qualify(main_part, args, false, "inject argument");
> }
>
> static const struct qual_options {
> const char *name;
> - void (*qualify)(const char *);
> + void (*qualify)(const char *, const char *);
> } qual_options[] = {
> { "trace", parse_trace },
> { "t", parse_trace },
> @@ -314,23 +326,26 @@ static const struct qual_options {
> { "inject", parse_inject },
> };
>
> +bool parsing_qualify_mode = false;
> +
> void
> -parse_qualify_filter(const char *str)
> +parse_qualify_action(const char *action_name, const char *main_part,
> + const char *args)
> {
> - const struct qual_options *opt = qual_options;
> + const struct qual_options *opt = NULL;
> unsigned int i;
>
> for (i = 0; i < ARRAY_SIZE(qual_options); ++i) {
> - const char *name = qual_options[i].name;
> - const size_t len = strlen(name);
> - const char *val = str_strip_prefix_len(str, name, len);
> -
> - if (val == str || *val != '=')
> - continue;
> - str = val + 1;
> - opt = &qual_options[i];
> - break;
> + if (!strcmp(action_name, qual_options[i].name)) {
> + opt = &qual_options[i];
> + break;
> + }
> }
>
> - opt->qualify(str);
> + if (!opt)
> + error_msg_and_die("invalid filter action '%s'", action_name);
> + /* Use qualify mode of set parsing. */
> + parsing_qualify_mode = true;
> + opt->qualify(main_part, args);
> + parsing_qualify_mode = false;
> }
> diff --git a/strace.c b/strace.c
> index 6162d3b3..79c5397e 100644
> --- a/strace.c
> +++ b/strace.c
> @@ -1580,7 +1580,7 @@ init(int argc, char *argv[])
> #if DEFAULT_QUAL_FLAGS != (QUAL_TRACE | QUAL_ABBREV | QUAL_VERBOSE)
> # error Bug in DEFAULT_QUAL_FLAGS
> #endif
> - parse_qualify_filter("signal=all");
> + filtering_parse("signal=all");
> while ((c = getopt(argc, argv,
> "+b:cCdfFhiqrtTvVwxyz"
> #ifdef USE_LIBUNWIND
> @@ -1647,7 +1647,7 @@ init(int argc, char *argv[])
> show_fd_path++;
> break;
> case 'v':
> - parse_qualify_filter("abbrev=none");
> + filtering_parse("abbrev=none");
> break;
> case 'V':
> print_version();
> @@ -1662,7 +1662,7 @@ init(int argc, char *argv[])
> error_opt_arg(c, optarg);
> break;
> case 'e':
> - parse_qualify_filter(optarg);
> + filtering_parse(optarg);
> break;
> case 'o':
> outfname = optarg;
> diff --git a/tests/filtering_syscall-syntax.test b/tests/filtering_syscall-syntax.test
> index 2a4125ae..3f8db02b 100755
> --- a/tests/filtering_syscall-syntax.test
> +++ b/tests/filtering_syscall-syntax.test
> @@ -54,25 +54,11 @@ for arg in '' , ,, ,,, \! \!, \
> check_syscall "$arg" "$arg"
> done
>
> -# Temporary workaround
> for arg in : :: ::: \
> ; do
> - check_e "invalid system call '$arg'" -e trace="$arg"
> - check_e "invalid system call '$arg'" -e abbrev="$arg"
> - check_e "invalid system call '$arg'" -e verbose="$arg"
> - check_e "invalid system call '$arg'" -e raw="$arg"
> - check_e "invalid system call ''" -e inject="$arg"
> - check_e "invalid system call ''" -e fault="$arg"
> + check_syscall "" "$arg"
> done
>
> -# Temporary workaround for '!:' check
> -check_e "invalid system call ':'" -e trace=\!:
> -check_e "invalid system call ':'" -e abbrev=\!:
> -check_e "invalid system call ':'" -e verbose=\!:
> -check_e "invalid system call ':'" -e raw=\!:
> -check_e "invalid system call '!'" -e inject=\!:
> -check_e "invalid system call '!'" -e fault=\!:
> -
> # invalid syscall with inversion
> for arg in -1 -2 -3 -4 -5 \
> invalid_syscall_name \
> @@ -99,6 +85,7 @@ for arg in file,nonsense \
> done
>
> # special cases
> +check_syscall "!" \!:
> check_syscall "!" 1,\!
> check_syscall "!open" 1,\!open
> check_syscall "none" 1,none
> --
> 2.11.0
More information about the Strace-devel
mailing list