From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3795 invoked by alias); 12 Nov 2008 23:20:38 -0000 Received: (qmail 3655 invoked by uid 22791); 12 Nov 2008 23:20:37 -0000 X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (65.74.133.4) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 12 Nov 2008 23:19:53 +0000 Received: (qmail 23901 invoked from network); 12 Nov 2008 23:19:51 -0000 Received: from unknown (HELO orlando.local) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 12 Nov 2008 23:19:51 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [RFA] Implement 'detach pid'. Date: Thu, 13 Nov 2008 12:52:00 -0000 User-Agent: KMail/1.9.10 Cc: Vladimir Prus References: <200811122339.02463.vladimir@codesourcery.com> In-Reply-To: <200811122339.02463.vladimir@codesourcery.com> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline Message-Id: <200811122319.57637.pedro@codesourcery.com> X-IsSubscribed: yes Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2008-11/txt/msg00259.txt.bz2 On Wednesday 12 November 2008 20:39:02, Vladimir Prus wrote: > > This patch makes CLI 'detach' and MI '-target-detach' accept the PID > of the process to detach from. > I see several issues with this patch: - The target is not the right layer to do this. Before you reach here, you've already done things to the current inferior. E.g., target.c:target_detach will call remove_breakpoints before reaching remote_detach_1. This means that, if you have e.g., selected inferior 1, and do detach 3, you'll remove breakpoints from inferior 1, and detach process 3. Breakpoints are an example. Other example is that before you reach the process stratum, you can pass by the thread_stratum, which again would do anything to the wrong detachee, since it's usually the process_statum that does the final real detach. That's bad. You should do the needed parsing at a higher level, switch to a thread of the process you want and then call the target method. In the long run, ideally inferior_ptid shouldn't be so pervasive, but that's in a really far long run yet. This also means that the syntax becomes uniform across targets. - Other targets are already using the args to mean something different, although at the cli level that broke (I'm guessing) with the introduction of the checkpoint/multifork support, which added subcommands to "detach". Several targets (e.g. inf-ptrace.c, procfs.c, inf-ttrace, nto-procfs.c) use it to detach with a signal. - "detach 42000" will make it through even in remote target (not extended remote). That's the pid used when we don't know about any (from magic_null_ptid). inferior ids have internal ids != pids (shown with info inferiors", just like threads have the internal thread id, and a ptid. I wonder if at the cli level (don't know about MI), we shouldn't use those instead of the real PID (or have switches to specify what the number represents. So, hippotetically "detach 1" would detach from "inferior 1", which could be pid 4234. I'm not so sure the cli needs this extension (as opposed to switching to the inferior and using plain "detach"). > Is the infcmd.c change OK? > > - Volodya > > From 8c0569ee17d85cf591426a0b9900d8cad0e02389 Mon Sep 17 00:00:00 2001 > > * infcmd.c (_initialize_infcmd): Make 'detach' accept > parameter. > * mi/mi-cmds.c (mi_cmds): Make '-target-detach' pass parameter > to 'detach'. > * remote.c (remote_detach_1): Interpret the passed > parameter as a pid to detach from. > --- > gdb/infcmd.c | 2 +- > gdb/mi/mi-cmds.c | 2 +- > gdb/remote.c | 13 +++++++++++-- > 3 files changed, 13 insertions(+), 4 deletions(-) > > diff --git a/gdb/infcmd.c b/gdb/infcmd.c > index b3af31f..76b48ae 100644 > --- a/gdb/infcmd.c > +++ b/gdb/infcmd.c > @@ -2556,7 +2556,7 @@ to specify the program, and to load its symbol table.")); > Detach a process or file previously attached.\n\ > If a process, it is no longer traced, and it continues its execution. If\n\ > you were debugging a file, the file is closed and gdb no longer accesses it."), > - &detachlist, "detach ", 0, &cmdlist); > + &detachlist, "detach ", 1, &cmdlist); > > add_com ("disconnect", class_run, disconnect_command, _("\ > Disconnect from a target.\n\ > diff --git a/gdb/mi/mi-cmds.c b/gdb/mi/mi-cmds.c > index d38de35..708dcf8 100644 > --- a/gdb/mi/mi-cmds.c > +++ b/gdb/mi/mi-cmds.c > @@ -121,7 +121,7 @@ struct mi_cmd mi_cmds[] = > { "symbol-type", { NULL, 0 }, NULL }, > { "target-attach", { "attach", 1 }, NULL }, > { "target-compare-sections", { NULL, 0 }, NULL }, > - { "target-detach", { "detach", 0 }, 0 }, > + { "target-detach", { "detach", 1 }, 0 }, > { "target-disconnect", { "disconnect", 0 }, 0 }, > { "target-download", { "load", 1 }, NULL}, > { "target-exec-status", { NULL, 0 }, NULL }, > diff --git a/gdb/remote.c b/gdb/remote.c > index 5cb36b8..b1adfdb 100644 > --- a/gdb/remote.c > +++ b/gdb/remote.c > @@ -3261,11 +3261,20 @@ remote_open_1 (char *name, int from_tty, struct target_ops *target, int extended > static void > remote_detach_1 (char *args, int from_tty, int extended) > { > - int pid = ptid_get_pid (inferior_ptid); > + int pid; > struct remote_state *rs = get_remote_state (); > > if (args) > - error (_("Argument given to \"detach\" when remotely debugging.")); > + { > + char *end = args; > + pid = strtol (args, &end, 10); > + if (*end != '\0') > + error (_("Cannot parse process id '%s'"), args); > + if (!in_inferior_list (pid)) > + error (_("Invalid process id %d"), pid); > + } > + else > + pid = ptid_get_pid (inferior_ptid); > > if (!target_has_execution) > error (_("No process to detach from.")); -- Pedro Alves