On Fri, 26 Feb 2016 10:01:13 -0600
Tom Zanussi <[email protected]> wrote:
> +static void hist_trigger_elt_copy(struct tracing_map_elt *to,
> + struct tracing_map_elt *from)
> +{
> + char *comm_from = from->private_data;
> + char *comm_to = to->private_data;
> +
> + if (comm_from)
> + memcpy(comm_to, comm_from, TASK_COMM_LEN + 1);
> +}
> +
> +static void hist_trigger_elt_init(struct tracing_map_elt *elt)
> +{
> + char *comm = elt->private_data;
> +
> + if (comm)
> + save_comm(comm, current);
> +}
> +
> +static const struct tracing_map_ops hist_trigger_ops = {
> + .elt_alloc = hist_trigger_elt_alloc,
> + .elt_copy = hist_trigger_elt_copy,
> + .elt_free = hist_trigger_elt_free,
> + .elt_init = hist_trigger_elt_init,
These are only used for saving or displaying comm. Wouldn't adding that
in the name be better. Otherwise it looks like they are more generic. I
find that dangerous, especially since they just assume that the
private_data is a string.
What about hist_trigger_elt_comm_*
?
-- Steve
> +};
> +
> static void destroy_hist_field(struct hist_field *hist_field)
> {
> kfree(hist_field);
> @@ -399,6 +467,9 @@ static int create_key_field(struct hist_trigger_data
> *hist_data,
> flags |= HIST_FIELD_FL_SYM;
> else if (strcmp(field_str, "sym-offset") == 0)
> flags |= HIST_FIELD_FL_SYM_OFFSET;
> + else if ((strcmp(field_str, "execname") == 0) &&
> + (strcmp(field_name, "common_pid") == 0))
> + flags |= HIST_FIELD_FL_EXECNAME;
> else {
> ret = -EINVAL;
> goto out;
