On Wed, 27 Feb 2019 21:42:45 -0500 Steven Rostedt <[email protected]> wrote:
> On Wed, 27 Feb 2019 23:44:42 +0900 > Masami Hiramatsu <[email protected]> wrote: > > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c > > index a7012de37a00..0efef172db17 100644 > > --- a/kernel/trace/trace_probe.c > > +++ b/kernel/trace/trace_probe.c > > @@ -239,6 +239,7 @@ parse_probe_arg(char *arg, const struct fetch_type > > *type, > > { > > struct fetch_insn *code = *pcode; > > unsigned long param; > > + int deref = FETCH_OP_DEREF; > > long offset = 0; > > char *tmp; > > int ret = 0; > > @@ -301,8 +302,17 @@ parse_probe_arg(char *arg, const struct fetch_type > > *type, > > break; > > > > case '+': /* deref memory */ > > - arg++; /* Skip '+', because kstrtol() rejects it. */ > > case '-': > > + if (arg[0] == '+') { > > + arg++; /* Skip '+', because kstrtol() rejects it. */ > > + if (arg[0] == 'u') { > > + deref = FETCH_OP_UDEREF; > > + arg++; > > + } > > + } else if (arg[1] == 'u') { /* Start with "-u" */ > > + deref = FETCH_OP_UDEREF; > > + *(++arg) = '-'; > > + } > > What about: > > if (arg[1] == 'u') { > deref = FETCH_OP_UDEREF; > arg[1] = arg[0]; > arg++; > } > if (arg[0] == '+') > arg++; /* Skip '+', because kstrtol() rejects it. */ > > A bit less messy. Ah, thanks! I'll take it. > > > > tmp = strchr(arg, '('); > > if (!tmp) > > return -EINVAL; > > @@ -328,7 +338,7 @@ parse_probe_arg(char *arg, const struct fetch_type > > *type, > > return -E2BIG; > > *pcode = code; > > > > - code->op = FETCH_OP_DEREF; > > + code->op = deref; > > code->offset = offset; > > } > > break; > > @@ -444,13 +454,14 @@ static int traceprobe_parse_probe_arg_body(char *arg, > > ssize_t *size, > > /* Store operation */ > > if (!strcmp(parg->type->name, "string") || > > !strcmp(parg->type->name, "ustring")) { > > - if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_IMM && > > - code->op != FETCH_OP_COMM) { > > + if (code->op != FETCH_OP_DEREF && code->op != FETCH_OP_UDEREF > > + && code->op != FETCH_OP_IMM && code->op != FETCH_OP_COMM) { > > pr_info("string only accepts memory or address.\n"); > > ret = -EINVAL; > > goto fail; > > } > > - if (code->op != FETCH_OP_DEREF || parg->count) { > > + if ((code->op == FETCH_OP_IMM && code->op == FETCH_OP_COMM) > > + || parg->count) { > > How would "code->op == FETCH_OP_IMM && code->op == FETCH_OP_COMM" ever be > true? > > Did you mean || ? Oops, yes. It is a simple mistake... Thank you! > > -- Steve > > > /* > > * IMM and COMM is pointing actual address, those must > > * be kept, and if parg->count != 0, this is an array > > @@ -463,7 +474,8 @@ static int traceprobe_parse_probe_arg_body(char *arg, > > ssize_t *size, > > } > > } > > /* If op == DEREF, replace it with STRING */ > > - if (!strcmp(parg->type->name, "ustring")) > > + if (!strcmp(parg->type->name, "ustring") || > > + code->op == FETCH_OP_UDEREF) > > code->op = FETCH_OP_ST_USTRING; > > else > > code->op = FETCH_OP_ST_STRING; -- Masami Hiramatsu <[email protected]>
