RE: [MI non-stop 04/11] Implement --thread and --frame.

["Marc Khouzam" <marc dot khouzam at ericsson dot com>]

Hi,

I don't know enough to thoroughly review so I'll give the minor comments
I have.
 
> +  if (parse->frame != -1 && !parse->thread == -1)

You probably didn't mean to have ! before parse->thread

> +    error ("Cannot specify --frame without --thread");
> +  
> +  if (parse->thread != -1)
> +    {
> +      struct thread_info *tp = find_thread_id (parse->thread);
> +      if (!tp)
> + error (_("Invalid thread id: %d"), parse->thread);
> +      
> +      if (non_stop)
> + context_switch_to (tp->ptid);
> +      else
> +   switch_to_thread (tp->ptid);
> +    }
> +  
> +  if (parse->frame != -1)
> +    {
> +      struct frame_info *fid;
> +      int frame = parse->frame;
> +      fid = find_relative_frame (get_current_frame (), &frame);
> +      if (frame == 0)
> +   /* find_relative_frame was successful */
> +   select_frame (fid);
> +      else
> +   error (_("Invalid frame id: %s"), frame_str);
> +    }
> +  
>    if (parse->cmd->argv_func != NULL)
>      {
>        if (target_can_async_p ()
> @@ -1171,6 +1202,7 @@ mi_cmd_execute (struct mi_parse *parse)
>           error_stream (stb);
>         }
>     }
> +
>        current_token = xstrdup (parse->token);
>        cleanup = make_cleanup (free_current_contents, &current_token);
>        parse->cmd->argv_func (parse->command, parse->argv, parse->argc);
> diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
> index a2dc50d..835fb74 100644
> --- a/gdb/mi/mi-parse.c
> +++ b/gdb/mi/mi-parse.c
> @@ -150,6 +150,8 @@ mi_parse (char *cmd)
>    char *chp;
>    struct mi_parse *parse = XMALLOC (struct mi_parse);
>    memset (parse, 0, sizeof (*parse));
> +  parse->thread = -1;
> +  parse->frame = -1;
>  
>    /* Before starting, skip leading white space. */
>    while (isspace (*cmd))
> @@ -199,6 +201,38 @@ mi_parse (char *cmd)
>    while (isspace (*chp))
>      chp++;
>  
> +  /* Parse the --thread and --frame options, if present.  At present,
> +     some important commands, like '-break-*' are implemented by forwarding
> +     to the CLI layer directly.  We want to parse --thread and --frame
> +     here, so as not to leave those option in the string that will be passed
> +     to CLI.  */
> +  for (;;)
> +    {
> +      char *start = chp;
> +      if (strncmp (chp, "--thread", 8) == 0)

Although this works, if you later add a new param like --thread-list,
it will falsly match here.  Maybe you want to strncmp with "--thread "?
(with a space at the end) 
    
I'm not sure what the coding style is for GDB in this case, but I have
found that it is more maintainable not to hard-code the lengths, but to use
something like
   char* threadStr = "--thread ";
   int len = strlen(threadStr);

> +   {
> +     if (parse->thread != -1)
> +       error ("Duplicate '--thread' option");
> +     chp += 8;
> +     parse->thread = strtol (chp, &chp, 10);
> +   }
> +      else if (strncmp (chp, "--frame", 7) == 0)
> +   {
> +     if (parse->frame != -1)
> +       error ("Duplicate '--frame' option");
> +     chp += 7;
> +     parse->frame = strtol (chp, &chp, 10);
> +   }
> +      else
> +   break;

Are --frame and --thread the only options that can be found when we are
running this code? If yes, then its fine; if no, then you shouldn't break
on else but should skip the uninteresting option.

> +
> +      if (*chp != '\0' && !isspace (*chp))
> +   error ("Invalid value for the '%s' option",
> +          start[2] == 't' ? "--thread" : "--frame");
> +      while (isspace (*chp))
> +   chp++;
> +    }
> +
>    /* For new argv commands, attempt to return the parsed argument

[...]