RE: [MI][patch] broken -target-detach

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

> -----Original Message-----
> From: Pedro Alves [mailto:pedro@codesourcery.com] 
> Sent: Friday, November 12, 2010 11:44 AM
> To: Marc Khouzam
> Cc: 'gdb-patches@sourceware.org'; 'Tom Tromey'
> Subject: Re: [MI][patch] broken -target-detach
> 
> On Friday 12 November 2010 16:01:20, Marc Khouzam wrote:
> 
> > > Unfortunately, 7.2 was released accepting the PID form only,
> > > so we may be better off continue accepting it...
> > 
> > If we put this fix in the 7.2 branch, could we get rid of the
> > PID form or is it too late?
> 
> IMO, it's too late for 7.2.  Having 7.2 and 7.2.1 handle this
> command's argument incompatibly would cause pain for frontends
> already passing it an argument, and we can avoid causing that pain.
> So IMO, since it's quite easy to do so, we should continue 
> handling it.
> We can consider mentioning in the manual that the PID form
> is deprecated, and that it may be removed in a later release.
> Then we can consider removing it in the future if it ever causes
> trouble.  (I'm not suggesting you do that.)
> 
> > I knew someone was going to call me on that :-).  Here is the new
> > patch which still accepts PID.  
> 
> Sorry about that.  :-)
> 
> > I have to apologize, I just don't
> > have the time to get a test case for it.  I hope the patch is useful
> > enough as it is.
> 
> It sure is, thanks for persevering.
> 
> > Good for the 7.2 branch too?
> 
> IMO, yes.
> 
> You've just given me a new chance to nit on formatting, though! :-)
> 
> > +      /* First see if we are dealing with a thread-group id */
> 
> Period and double space at end of sentence, like:
> 
>  /* First see if we are dealing with a thread-group id.  */
> 
> 
> > +      if (*(argv[0]) == 'i')
> 
>      if (*argv[0] == 'i')
> 
> Okay with those changes.

Great!
I committed the following to HEAD and 7_2:

2010-11-12  Marc Khouzam  <marc.khouzam@ericsson.com>

        * mi/mi-main.c (mi_cmd_target_detach): Accept new
        thread-group id format.

### Eclipse Workspace Patch 1.0
#P src
Index: gdb/mi/mi-main.c
===================================================================
RCS file: /cvs/src/src/gdb/mi/mi-main.c,v
retrieving revision 1.178.2.2
diff -u -r1.178.2.2 mi-main.c
--- gdb/mi/mi-main.c    1 Sep 2010 19:16:00 -0000       1.178.2.2
+++ gdb/mi/mi-main.c    12 Nov 2010 19:00:48 -0000
@@ -418,19 +418,40 @@
 mi_cmd_target_detach (char *command, char **argv, int argc)
 {
   if (argc != 0 && argc != 1)
-    error ("Usage: -target-detach [thread-group]");
+    error ("Usage: -target-detach [pid | thread-group]");
 
   if (argc == 1)
     {
       struct thread_info *tp;
       char *end = argv[0];
-      int pid = strtol (argv[0], &end, 10);
+      int pid;
 
-      if (*end != '\0')
-       error (_("Cannot parse thread group id '%s'"), argv[0]);
+      /* First see if we are dealing with a thread-group id.  */
+      if (*argv[0] == 'i')
+       {
+         struct inferior *inf;
+         int id = strtoul (argv[0] + 1, &end, 0);
+
+         if (*end != '\0')
+           error (_("Invalid syntax of thread-group id '%s'"), argv[0]);
+
+         inf = find_inferior_id (id);
+         if (!inf)
+           error (_("Non-existent thread-group id '%d'"), id);
+
+         pid = inf->pid;
+       }
+      else
+       {
+         /* We must be dealing with a pid.  */
+         pid = strtol (argv[0], &end, 10);
+
+         if (*end != '\0')
+           error (_("Invalid identifier '%s'"), argv[0]);
+       }
 
       /* Pick any thread in the desired process.  Current
-        target_detach deteches from the parent of inferior_ptid.  */
+        target_detach detaches from the parent of inferior_ptid.  */
       tp = iterate_over_threads (find_thread_of_process, &pid);
       if (!tp)
        error (_("Thread group is empty"));