| 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: <20150116111826.GA4104@e104805>
Date: Fri, 16 Jan 2015 11:18:26 +0000
From: Javi Merino <javi.merino@....com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
Namhyung Kim <namhyung@...nel.org>,
Arnaldo Carvalho de Melo <acme@...hat.com>,
Steven Rostedt <srostedt@...hat.com>,
Jiri Olsa <jolsa@...hat.com>
Subject: Re: [RESEND PATCH v2 2/2] tools lib traceevent: Add support for
__print_array()
On Fri, Jan 16, 2015 at 02:35:19AM +0000, Steven Rostedt wrote:
> On Thu, 15 Jan 2015 12:05:52 -0500
> Javi Merino <javi.merino@....com> wrote:
>
> > Trace can now generate traces with variable element size arrays. Add
> > support to parse them.
> >
> > Cc: Namhyung Kim <namhyung@...nel.org>
> > Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> > Cc: Steven Rostedt <srostedt@...hat.com>
> > Cc: Jiri Olsa <jolsa@...hat.com>
> > Signed-off-by: Javi Merino <javi.merino@....com>
> > ---
> > tools/lib/traceevent/event-parse.c | 127
> > +++++++++++++++++++++++++++++++++++++
> > tools/lib/traceevent/event-parse.h | 8 +++ 2 files changed, 135
> > insertions(+)
> >
> > diff --git a/tools/lib/traceevent/event-parse.c
> > b/tools/lib/traceevent/event-parse.c index cf3a44bf1ec3..00dd6213449c
> > 100644 --- a/tools/lib/traceevent/event-parse.c
> > +++ b/tools/lib/traceevent/event-parse.c
> > @@ -757,6 +757,11 @@ static void free_arg(struct print_arg *arg)
> > free_arg(arg->hex.field);
> > free_arg(arg->hex.size);
> > break;
> > + case PRINT_INT_ARRAY:
> > + free_arg(arg->int_array.field);
> > + free_arg(arg->int_array.size);
> > + free_arg(arg->int_array.el_size);
> > + break;
> > case PRINT_TYPE:
> > free(arg->typecast.type);
> > free_arg(arg->typecast.item);
> > @@ -2533,6 +2538,71 @@ process_hex(struct event_format *event, struct
> > print_arg *arg, char **tok) }
> >
> > static enum event_type
> > +process_int_array(struct event_format *event, struct print_arg *arg,
> > char **tok) +{
> > + struct print_arg *field;
> > + enum event_type type;
> > + char *token;
> > +
> > + memset(arg, 0, sizeof(*arg));
> > + arg->type = PRINT_INT_ARRAY;
> > +
> > + field = alloc_arg();
> > + if (!field) {
> > + do_warning_event(event, "%s: not enough memory!",
> > __func__);
> > + goto out;
> > + }
> > +
> > + type = process_arg(event, field, &token);
> > +
> > + if (test_type_token(type, token, EVENT_DELIM, ","))
> > + goto out_free;
> > +
> > + arg->int_array.field = field;
> > +
> > + free_token(token);
> > +
> > + field = alloc_arg();
> > + if (!field) {
> > + do_warning_event(event, "%s: not enough memory!",
> > __func__);
> > + goto out;
> > + }
> > +
> > + type = process_arg(event, field, &token);
> > +
> > + if (test_type_token(type, token, EVENT_DELIM, ","))
> > + goto out_free;
> > +
> > + arg->int_array.size = field;
> > +
> > + free_token(token);
> > +
> > + field = alloc_arg();
> > + if (!field) {
> > + do_warning_event(event, "%s: not enough memory!",
> > __func__);
> > + goto out;
> > + }
>
> Hmm, perhaps we should make a helper function to allocate the field and
> show the warning for the event instead of duplicating the code three
> times.
Ok, I'll also use it for the two similar allocation code done in
process_hex()
> > +
> > + type = process_arg(event, field, &token);
> > +
> > + if (test_type_token(type, token, EVENT_DELIM, ")"))
> > + goto out_free;
> > +
> > + arg->int_array.el_size = field;
> > +
> > + free_token(token);
> > + type = read_token_item(tok);
> > + return type;
> > +
> > + out_free:
> > + free_arg(field);
> > + free_token(token);
> > +out:
> > + *tok = NULL;
> > + return EVENT_ERROR;
> > +}
> > +
> > +static enum event_type
> > process_dynamic_array(struct event_format *event, struct print_arg
> > *arg, char **tok) {
> > struct format_field *field;
> > @@ -2827,6 +2897,10 @@ process_function(struct event_format *event,
> > struct print_arg *arg, free_token(token);
> > return process_hex(event, arg, tok);
> > }
> > + if (strcmp(token, "__print_array") == 0) {
> > + free_token(token);
> > + return process_int_array(event, arg, tok);
> > + }
> > if (strcmp(token, "__get_str") == 0) {
> > free_token(token);
> > return process_str(event, arg, tok);
> > @@ -3355,6 +3429,7 @@ eval_num_arg(void *data, int size, struct
> > event_format *event, struct print_arg break;
> > case PRINT_FLAGS:
> > case PRINT_SYMBOL:
> > + case PRINT_INT_ARRAY:
> > case PRINT_HEX:
> > break;
> > case PRINT_TYPE:
> > @@ -3765,6 +3840,49 @@ static void print_str_arg(struct trace_seq *s,
> > void *data, int size, }
> > break;
> >
> > + case PRINT_INT_ARRAY: {
> > + void *num;
> > + int el_size;
> > +
> > + if (arg->int_array.field->type ==
> > PRINT_DYNAMIC_ARRAY) {
> > + unsigned long offset;
> > +
> > + offset = pevent_read_number(pevent,
> > + data +
> > arg->int_array.field->dynarray.field->offset,
> > +
> > arg->int_array.field->dynarray.field->size);
>
> Grumble, I hate that my mail client is breaking lines up like this.
> I'm using my laptop atm and haven't customized it to not screw up other
> people's email. Sorry for the messy reply here.
>
>
> > + num = data + (offset & 0xffff);
> > + } else {
> > + field = arg->int_array.field->field.field;
> > + if (!field) {
> > + str =
> > arg->int_array.field->field.name;
> > + field = pevent_find_any_field(event,
> > str);
> > + if (!field)
> > + goto out_warning_field;
> > + arg->int_array.field->field.field =
> > field;
> > + }
> > + num = data + field->offset;
> > + }
> > + len = eval_num_arg(data, size, event,
> > arg->int_array.size);
> > + el_size = eval_num_arg(data, size, event,
> > + arg->int_array.el_size);
> > + el_size /= 8;
> > + for (i = 0; i < len; i++) {
> > + if (i)
> > + trace_seq_putc(s, ' ');
> > +
> > + if (el_size == 1)
> > + trace_seq_printf(s, "%u", *(uint8_t
> > *)num);
> > + else if (el_size == 2)
> > + trace_seq_printf(s, "%u", *(uint16_t
> > *)num);
> > + else if (el_size == 4)
> > + trace_seq_printf(s, "%u", *(uint32_t
> > *)num);
> > + else if (el_size == 8)
> > + trace_seq_printf(s, "%lu",
>
> Shouldn't that be "%llu"? This shouldn't warn on i386 compiles either.
>
> > *(uint64_t *)num); +
>
> I wonder if we should have a "else" here to show the same BAD SIZE that
> I replied with in the other patch.
Sure, I'll add it. Cheers,
Javi
--
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