| From dd58504c50969dbb928b1a66ab6bdb2f8e40b6cb Mon Sep 17 00:00:00 2001 |
| From: Tom Zanussi <zanussi@kernel.org> |
| Date: Fri, 28 Jun 2019 12:40:20 -0500 |
| Subject: [PATCH] tracing: Simplify assignment parsing for hist triggers |
| |
| commit b527b638fd63ba791dc90a0a6e9a3035b10df52b upstream. |
| |
| In the process of adding better error messages for sorting, I realized |
| that strsep was being used incorrectly and some of the error paths I |
| was expecting to be hit weren't and just fell through to the common |
| invalid key error case. |
| |
| It also became obvious that for keyword assignments, it wasn't |
| necessary to save the full assignment and reparse it later, and having |
| a common empty-assignment check would also make more sense in terms of |
| error processing. |
| |
| Change the code to fix these problems and simplify it for new error |
| message changes in a subsequent patch. |
| |
| Link: http://lkml.kernel.org/r/1c3ef0b6655deaf345f6faee2584a0298ac2d743.1561743018.git.zanussi@kernel.org |
| |
| Fixes: e62347d24534 ("tracing: Add hist trigger support for user-defined sorting ('sort=' param)") |
| Fixes: 7ef224d1d0e3 ("tracing: Add 'hist' event trigger command") |
| Fixes: a4072fe85ba3 ("tracing: Add a clock attribute for hist triggers") |
| Reported-by: Masami Hiramatsu <mhiramat@kernel.org> |
| Reviewed-by: Masami Hiramatsu <mhiramat@kernel.org> |
| Signed-off-by: Tom Zanussi <zanussi@kernel.org> |
| Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c |
| index bca7998a8069..d1dc4384b0ad 100644 |
| --- a/kernel/trace/trace_events_hist.c |
| +++ b/kernel/trace/trace_events_hist.c |
| @@ -2009,12 +2009,6 @@ static int parse_map_size(char *str) |
| unsigned long size, map_bits; |
| int ret; |
| |
| - strsep(&str, "="); |
| - if (!str) { |
| - ret = -EINVAL; |
| - goto out; |
| - } |
| - |
| ret = kstrtoul(str, 0, &size); |
| if (ret) |
| goto out; |
| @@ -2074,25 +2068,25 @@ static int parse_action(char *str, struct hist_trigger_attrs *attrs) |
| static int parse_assignment(struct trace_array *tr, |
| char *str, struct hist_trigger_attrs *attrs) |
| { |
| - int ret = 0; |
| + int len, ret = 0; |
| |
| - if ((str_has_prefix(str, "key=")) || |
| - (str_has_prefix(str, "keys="))) { |
| - attrs->keys_str = kstrdup(str, GFP_KERNEL); |
| + if ((len = str_has_prefix(str, "key=")) || |
| + (len = str_has_prefix(str, "keys="))) { |
| + attrs->keys_str = kstrdup(str + len, GFP_KERNEL); |
| if (!attrs->keys_str) { |
| ret = -ENOMEM; |
| goto out; |
| } |
| - } else if ((str_has_prefix(str, "val=")) || |
| - (str_has_prefix(str, "vals=")) || |
| - (str_has_prefix(str, "values="))) { |
| - attrs->vals_str = kstrdup(str, GFP_KERNEL); |
| + } else if ((len = str_has_prefix(str, "val=")) || |
| + (len = str_has_prefix(str, "vals=")) || |
| + (len = str_has_prefix(str, "values="))) { |
| + attrs->vals_str = kstrdup(str + len, GFP_KERNEL); |
| if (!attrs->vals_str) { |
| ret = -ENOMEM; |
| goto out; |
| } |
| - } else if (str_has_prefix(str, "sort=")) { |
| - attrs->sort_key_str = kstrdup(str, GFP_KERNEL); |
| + } else if ((len = str_has_prefix(str, "sort="))) { |
| + attrs->sort_key_str = kstrdup(str + len, GFP_KERNEL); |
| if (!attrs->sort_key_str) { |
| ret = -ENOMEM; |
| goto out; |
| @@ -2103,12 +2097,8 @@ static int parse_assignment(struct trace_array *tr, |
| ret = -ENOMEM; |
| goto out; |
| } |
| - } else if (str_has_prefix(str, "clock=")) { |
| - strsep(&str, "="); |
| - if (!str) { |
| - ret = -EINVAL; |
| - goto out; |
| - } |
| + } else if ((len = str_has_prefix(str, "clock="))) { |
| + str += len; |
| |
| str = strstrip(str); |
| attrs->clock = kstrdup(str, GFP_KERNEL); |
| @@ -2116,8 +2106,8 @@ static int parse_assignment(struct trace_array *tr, |
| ret = -ENOMEM; |
| goto out; |
| } |
| - } else if (str_has_prefix(str, "size=")) { |
| - int map_bits = parse_map_size(str); |
| + } else if ((len = str_has_prefix(str, "size="))) { |
| + int map_bits = parse_map_size(str + len); |
| |
| if (map_bits < 0) { |
| ret = map_bits; |
| @@ -2157,8 +2147,14 @@ parse_hist_trigger_attrs(struct trace_array *tr, char *trigger_str) |
| |
| while (trigger_str) { |
| char *str = strsep(&trigger_str, ":"); |
| + char *rhs; |
| |
| - if (strchr(str, '=')) { |
| + rhs = strchr(str, '='); |
| + if (rhs) { |
| + if (!strlen(++rhs)) { |
| + ret = -EINVAL; |
| + goto free; |
| + } |
| ret = parse_assignment(tr, str, attrs); |
| if (ret) |
| goto free; |
| @@ -4527,10 +4523,6 @@ static int create_val_fields(struct hist_trigger_data *hist_data, |
| if (!fields_str) |
| goto out; |
| |
| - strsep(&fields_str, "="); |
| - if (!fields_str) |
| - goto out; |
| - |
| for (i = 0, j = 1; i < TRACING_MAP_VALS_MAX && |
| j < TRACING_MAP_VALS_MAX; i++) { |
| field_str = strsep(&fields_str, ","); |
| @@ -4625,10 +4617,6 @@ static int create_key_fields(struct hist_trigger_data *hist_data, |
| if (!fields_str) |
| goto out; |
| |
| - strsep(&fields_str, "="); |
| - if (!fields_str) |
| - goto out; |
| - |
| for (i = n_vals; i < n_vals + TRACING_MAP_KEYS_MAX; i++) { |
| field_str = strsep(&fields_str, ","); |
| if (!field_str) |
| @@ -4786,12 +4774,6 @@ static int create_sort_keys(struct hist_trigger_data *hist_data) |
| if (!fields_str) |
| goto out; |
| |
| - strsep(&fields_str, "="); |
| - if (!fields_str) { |
| - ret = -EINVAL; |
| - goto out; |
| - } |
| - |
| for (i = 0; i < TRACING_MAP_SORT_KEYS_MAX; i++) { |
| struct hist_field *hist_field; |
| char *field_str, *field_name; |
| @@ -4800,9 +4782,11 @@ static int create_sort_keys(struct hist_trigger_data *hist_data) |
| sort_key = &hist_data->sort_keys[i]; |
| |
| field_str = strsep(&fields_str, ","); |
| - if (!field_str) { |
| - if (i == 0) |
| - ret = -EINVAL; |
| + if (!field_str) |
| + break; |
| + |
| + if (!*field_str) { |
| + ret = -EINVAL; |
| break; |
| } |
| |
| @@ -4812,7 +4796,7 @@ static int create_sort_keys(struct hist_trigger_data *hist_data) |
| } |
| |
| field_name = strsep(&field_str, "."); |
| - if (!field_name) { |
| + if (!field_name || !*field_name) { |
| ret = -EINVAL; |
| break; |
| } |
| -- |
| 2.7.4 |
| |