[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1311602587.3526.18.camel@gandalf.stny.rr.com>
Date: Mon, 25 Jul 2011 10:03:07 -0400
From: Steven Rostedt <rostedt@...dmis.org>
To: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
Cc: Michael Rubin <mrubin@...gle.com>,
David Sharp <dhsharp@...gle.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] trace-cmd: Handle invalid opcode parsing gracefully
On Fri, 2011-07-15 at 20:00 -0700, Vaibhav Nagarnaik wrote:
> If an invalid opcode is encountered, trace-cmd exits with an error.
> Instead it can be treated as a soft error where the event's print format
> is not parsed and its binary data is dumped out.
>
> This patch adds a return value to arg_num_eval() function to indicate if
> the parsing was successful. If not, then the error is considered soft
> and the parsing of the offending event fails.
>
> Signed-off-by: Vaibhav Nagarnaik <vnagarnaik@...gle.com>
Thanks Vaibhav, applied!
-- Steve
> ---
> parse-events.c | 125 +++++++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 83 insertions(+), 42 deletions(-)
>
> diff --git a/parse-events.c b/parse-events.c
> index d8f322a..58ffe51 100644
> --- a/parse-events.c
> +++ b/parse-events.c
> @@ -1901,90 +1901,120 @@ eval_type(unsigned long long val, struct print_arg *arg, int pointer)
> return eval_type_str(val, arg->typecast.type, pointer);
> }
>
> -static long long arg_num_eval(struct print_arg *arg)
> +static int arg_num_eval(struct print_arg *arg, long long *val)
> {
> long long left, right;
> - long long val = 0;
> + int ret = 1;
>
> switch (arg->type) {
> case PRINT_ATOM:
> - val = strtoll(arg->atom.atom, NULL, 0);
> + *val = strtoll(arg->atom.atom, NULL, 0);
> break;
> case PRINT_TYPE:
> - val = arg_num_eval(arg->typecast.item);
> - val = eval_type(val, arg, 0);
> + ret = arg_num_eval(arg->typecast.item, val);
> + if (!ret)
> + break;
> + *val = eval_type(*val, arg, 0);
> break;
> case PRINT_OP:
> switch (arg->op.op[0]) {
> case '|':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
> if (arg->op.op[1])
> - val = left || right;
> + *val = left || right;
> else
> - val = left | right;
> + *val = left | right;
> break;
> case '&':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
> if (arg->op.op[1])
> - val = left && right;
> + *val = left && right;
> else
> - val = left & right;
> + *val = left & right;
> break;
> case '<':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
> switch (arg->op.op[1]) {
> case 0:
> - val = left < right;
> + *val = left < right;
> break;
> case '<':
> - val = left << right;
> + *val = left << right;
> break;
> case '=':
> - val = left <= right;
> + *val = left <= right;
> break;
> default:
> - die("unknown op '%s'", arg->op.op);
> + do_warning("unknown op '%s'", arg->op.op);
> + ret = 0;
> }
> break;
> case '>':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
> switch (arg->op.op[1]) {
> case 0:
> - val = left > right;
> + *val = left > right;
> break;
> case '>':
> - val = left >> right;
> + *val = left >> right;
> break;
> case '=':
> - val = left >= right;
> + *val = left >= right;
> break;
> default:
> - die("unknown op '%s'", arg->op.op);
> + do_warning("unknown op '%s'", arg->op.op);
> + ret = 0;
> }
> break;
> case '=':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> -
> - if (arg->op.op[1] != '=')
> - die("unknown op '%s'", arg->op.op);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
>
> - val = left == right;
> + if (arg->op.op[1] != '=') {
> + do_warning("unknown op '%s'", arg->op.op);
> + ret = 0;
> + } else
> + *val = left == right;
> break;
> case '!':
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
>
> switch (arg->op.op[1]) {
> case '=':
> - val = left != right;
> + *val = left != right;
> break;
> default:
> - die("unknown op '%s'", arg->op.op);
> + do_warning("unknown op '%s'", arg->op.op);
> + ret = 0;
> }
> break;
> case '-':
> @@ -1992,12 +2022,17 @@ static long long arg_num_eval(struct print_arg *arg)
> if (arg->op.left->type == PRINT_NULL)
> left = 0;
> else
> - left = arg_num_eval(arg->op.left);
> - right = arg_num_eval(arg->op.right);
> - val = left - right;
> + ret = arg_num_eval(arg->op.left, &left);
> + if (!ret)
> + break;
> + ret = arg_num_eval(arg->op.right, &right);
> + if (!ret)
> + break;
> + *val = left - right;
> break;
> default:
> - die("unknown op '%s'", arg->op.op);
> + do_warning("unknown op '%s'", arg->op.op);
> + ret = 0;
> }
> break;
>
> @@ -2006,10 +2041,11 @@ static long long arg_num_eval(struct print_arg *arg)
> case PRINT_STRING:
> case PRINT_BSTRING:
> default:
> - die("invalid eval type %d", arg->type);
> + do_warning("invalid eval type %d", arg->type);
> + ret = 0;
>
> }
> - return val;
> + return ret;
> }
>
> static char *arg_eval (struct print_arg *arg)
> @@ -2023,7 +2059,8 @@ static char *arg_eval (struct print_arg *arg)
> case PRINT_TYPE:
> return arg_eval(arg->typecast.item);
> case PRINT_OP:
> - val = arg_num_eval(arg);
> + if (!arg_num_eval(arg, &val))
> + break;
> sprintf(buf, "%lld", val);
> return buf;
>
> @@ -2065,6 +2102,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
> memset(field, 0, sizeof(field));
>
> value = arg_eval(arg);
> + if (value == NULL)
> + goto out_free;
> field->value = strdup(value);
>
> free_arg(arg);
> @@ -2076,6 +2115,8 @@ process_fields(struct event_format *event, struct print_flag_sym **list, char **
> goto out_free;
>
> value = arg_eval(arg);
> + if (value == NULL)
> + goto out_free;
> field->str = strdup(value);
> free_arg(arg);
> arg = NULL;
--
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