Messages in this thread | | | Date | Wed, 2 Mar 2016 09:33:20 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v15 02/23] tracing: Add 'hist' event trigger command |
| |
On Fri, 26 Feb 2016 10:01:05 -0600 Tom Zanussi <tom.zanussi@linux.intel.com> wrote:
> +static int create_hitcount_val(struct hist_trigger_data *hist_data) > +{ > + hist_data->fields[HITCOUNT_IDX] = > + create_hist_field(NULL, HIST_FIELD_FL_HITCOUNT); > + if (!hist_data->fields[HITCOUNT_IDX]) > + return -ENOMEM; > + > + if (WARN_ON(++hist_data->n_vals > TRACING_MAP_VALS_MAX))
Never put side effects in a WARN_ON() macro. Some configs define WARN_ON() as a nop. Which would break this code. Always do the following:
hist_data->n_vals++;
if (WARN_ON(hist_data->n_vals > TRACING_MAP_VALS_MAX)) > + return -EINVAL;
> + > + return 0; > +} > + > +static int create_val_fields(struct hist_trigger_data *hist_data, > + struct trace_event_file *file) > +{ > + int ret; > + > + ret = create_hitcount_val(hist_data); > + > + return ret; > +} > + > +static int create_key_field(struct hist_trigger_data *hist_data, > + unsigned int key_idx, > + struct trace_event_file *file, > + char *field_str) > +{ > + struct ftrace_event_field *field = NULL; > + unsigned long flags = 0; > + unsigned int key_size; > + int ret = 0; > + > + if (WARN_ON(key_idx >= TRACING_MAP_FIELDS_MAX)) > + return -EINVAL; > + > + flags |= HIST_FIELD_FL_KEY; > + > + field = trace_find_event_field(file->event_call, field_str); > + if (!field) { > + ret = -EINVAL; > + goto out; > + } > + > + key_size = field->size; > + > + hist_data->fields[key_idx] = create_hist_field(field, flags); > + if (!hist_data->fields[key_idx]) { > + ret = -ENOMEM; > + goto out; > + } > + > + key_size = ALIGN(key_size, sizeof(u64)); > + hist_data->fields[key_idx]->size = key_size; > + hist_data->key_size = key_size; > + if (hist_data->key_size > HIST_KEY_SIZE_MAX) { > + ret = -EINVAL; > + goto out; > + } > + > + if (WARN_ON(++hist_data->n_keys > TRACING_MAP_KEYS_MAX))
Same here.
> + return -EINVAL; > + > + ret = key_size; > + out: > + return ret; > +} > +
-- Steve
| |