On Monday 08 February 2010 22:20:13 Pedro Alves wrote:

> > -      old_chain = make_cleanup_restore_current_thread ();
> > -      iterate_over_threads (interrupt_thread_callback, &pid);
> > -      do_cleanups (old_chain);
> > +      struct inferior *inf = find_inferior_id (current_context->thread_group);
> > +      iterate_over_threads (interrupt_thread_callback, &(inf->pid));
> 
> Redundant ()'s.

I think the version with () is more readable.

> >    if (parse->thread != -1)
> >      {
> >        struct thread_info *tp = find_thread_id (parse->thread);
> > @@ -1767,6 +1908,8 @@ mi_cmd_execute (struct mi_parse *parse)
> >         error (_("Invalid frame id: %d"), frame);
> >      }
> >  
> > +  current_context = parse;
> > +
> 
> Hmm, aren't the `struct mi_parse' objects leaking
> for every MI command?  I can't see where they're released in
> mi_execute_command ?

Close to the end of that function, I see:

   mi_parse_free (command);

Is it not there for you?

> In the latter strncmp above:
> 
> + if (strncmp (chp, "--all", as) == 0)
> 
> AS is always larger than strlen("--all"), so the
> strncmp's check on AS is useless and confusing here.
> I think you wanted:
> 
>       if (strcmp (chp, "--all") == 0)
>        {
>          parse->all = 1;
>          chp += strlen (chp);
>        }

I've adjusted sizes. I prefer to keep two ifs with the same structure.



> > diff --git a/gdb/testsuite/gdb.mi/mi-nonstop.exp b/gdb/testsuite/gdb.mi/mi-nonstop.exp
> > index 605f48b..397dec5 100644
> > --- a/gdb/testsuite/gdb.mi/mi-nonstop.exp
> > +++ b/gdb/testsuite/gdb.mi/mi-nonstop.exp
> > @@ -151,7 +151,7 @@ if { [is_remote target] } {
> >      unsupported $test
> >  } else {
> >      gdb_expect {
> > -       -re ".*=thread-exited,id=\"2\",group-id=\"\[0-9\]+\"\r\n$" {
> > +       -re ".*=thread-exited,id=\"2\",group-id=\"i\[0-9\]+\"\r\n$" {
> >             pass $test
> >         }
> >         timeout {
> 
> Otherwise, looks good to me.
> 
> Do you know if current frontends had bad assumptions, and will
> behave or misbehave with this change?  E.g., CDI, DSF-GDB, kdevelop?

I don't know of any frontend that makes such assumption. This thread-group
thing is still very new.

I attach revised patch.

Thanks,

-- 
Vladimir Prus
CodeSourcery
vladimir@codesourcery.com
(650) 331-3385 x722