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.


>               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 || ?

-- 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;

Reply via email to