Messages in this thread | | | Date | Sun, 23 Dec 2018 12:23:52 +0900 | From | Masami Hiramatsu <> | Subject | Re: [for-next][PATCH 5/5] tracing: Use the return of str_has_prefix() to remove open coded numbers |
| |
On Sat, 22 Dec 2018 11:20:12 -0500 Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org> > > There are several locations that compare constants to the beginning of > string variables to determine what commands should be done, then the > constant length is used to index into the string. This is error prone as the > hard coded numbers have to match the size of the constants. Instead, use the > len returned from str_has_prefix() and remove the open coded string length > sizes.
Looks good to me.
Acked-by: Masami Hiramatsu <mhiramat@kernel.org>
for trace_probe part.
Thanks!
> > Cc: Joe Perches <joe@perches.com> > Cc: Masami Hiramatsu <mhiramat@kernel.org> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > kernel/trace/trace.c | 8 +++++--- > kernel/trace/trace_probe.c | 17 +++++++++-------- > kernel/trace/trace_stack.c | 6 ++++-- > 3 files changed, 18 insertions(+), 13 deletions(-) > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c > index eac2824a18ab..18b86c3974e1 100644 > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -4408,13 +4408,15 @@ static int trace_set_options(struct trace_array *tr, char *option) > int neg = 0; > int ret; > size_t orig_len = strlen(option); > + int len; > > cmp = strstrip(option); > > - if (str_has_prefix(cmp, "no")) { > + len = str_has_prefix(cmp, "no"); > + if (len) > neg = 1; > - cmp += 2; > - } > + > + cmp += len; > > mutex_lock(&trace_types_lock); > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > index 541375737403..9962cb5da8ac 100644 > --- a/kernel/trace/trace_probe.c > +++ b/kernel/trace/trace_probe.c > @@ -186,19 +186,20 @@ int traceprobe_parse_event_name(const char **pevent, const char **pgroup, > static int parse_probe_vars(char *arg, const struct fetch_type *t, > struct fetch_insn *code, unsigned int flags) > { > - int ret = 0; > unsigned long param; > + int ret = 0; > + int len; > > if (strcmp(arg, "retval") == 0) { > if (flags & TPARG_FL_RETURN) > code->op = FETCH_OP_RETVAL; > else > ret = -EINVAL; > - } else if (str_has_prefix(arg, "stack")) { > - if (arg[5] == '\0') { > + } else if ((len = str_has_prefix(arg, "stack"))) { > + if (arg[len] == '\0') { > code->op = FETCH_OP_STACKP; > - } else if (isdigit(arg[5])) { > - ret = kstrtoul(arg + 5, 10, ¶m); > + } else if (isdigit(arg[len])) { > + ret = kstrtoul(arg + len, 10, ¶m); > if (ret || ((flags & TPARG_FL_KERNEL) && > param > PARAM_MAX_STACK)) > ret = -EINVAL; > @@ -213,10 +214,10 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t, > #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API > } else if (((flags & TPARG_FL_MASK) == > (TPARG_FL_KERNEL | TPARG_FL_FENTRY)) && > - str_has_prefix(arg, "arg")) { > - if (!isdigit(arg[3])) > + (len = str_has_prefix(arg, "arg"))) { > + if (!isdigit(arg[len])) > return -EINVAL; > - ret = kstrtoul(arg + 3, 10, ¶m); > + ret = kstrtoul(arg + len, 10, ¶m); > if (ret || !param || param > PARAM_MAX_STACK) > return -EINVAL; > code->op = FETCH_OP_ARG; > diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c > index 3641f28c343f..eec648a0d673 100644 > --- a/kernel/trace/trace_stack.c > +++ b/kernel/trace/trace_stack.c > @@ -448,8 +448,10 @@ static char stack_trace_filter_buf[COMMAND_LINE_SIZE+1] __initdata; > > static __init int enable_stacktrace(char *str) > { > - if (str_has_prefix(str, "_filter=")) > - strncpy(stack_trace_filter_buf, str+8, COMMAND_LINE_SIZE); > + int len; > + > + if ((len = str_has_prefix(str, "_filter="))) > + strncpy(stack_trace_filter_buf, str + len, COMMAND_LINE_SIZE); > > stack_tracer_enabled = 1; > last_stack_tracer_enabled = 1; > -- > 2.19.2 > >
-- Masami Hiramatsu <mhiramat@kernel.org>
| |