From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 57202 invoked by alias); 15 Sep 2017 13:39:01 -0000 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 Received: (qmail 56286 invoked by uid 89); 15 Sep 2017 13:39:00 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.9 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RP_MATCHES_RCVD,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 15 Sep 2017 13:38:58 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id v8FDcpAC005369 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 15 Sep 2017 09:38:56 -0400 Received: by simark.ca (Postfix, from userid 112) id E9BD91ECEE; Fri, 15 Sep 2017 09:38:51 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id DB42B1E5B9; Fri, 15 Sep 2017 09:38:50 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Fri, 15 Sep 2017 13:39:00 -0000 From: Simon Marchi To: Simon Marchi Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 2/2] gdbserver: Move detach code to its own function In-Reply-To: <1505482514-10206-2-git-send-email-simon.marchi@ericsson.com> References: <1505482514-10206-1-git-send-email-simon.marchi@ericsson.com> <1505482514-10206-2-git-send-email-simon.marchi@ericsson.com> Message-ID: <1bdaa1a6e4be7db9b3cc03244b4fdba4@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.0 X-Poly-FromMTA: (simark.ca [158.69.221.121]) at Fri, 15 Sep 2017 13:38:52 +0000 X-IsSubscribed: yes X-SW-Source: 2017-09/txt/msg00409.txt.bz2 On 2017-09-15 15:35, Simon Marchi wrote: > The code required to handle the 'D' packet is non trivial, so move it > out to its own function. > > The moved out code is identical, except for the call to strtol and some > breaks that became returns. > > Tested manually, and by running gdb.base/*detach*.exp with > native-gdbserver and native-extended-gdbserver. > > gdb/gdbserver/ChangeLog: > > * server.c (handle_detach): New function. > (process_serial_event): Move code out, call handle_detach. > --- > gdb/gdbserver/server.c | 179 > +++++++++++++++++++++++++------------------------ > 1 file changed, 93 insertions(+), 86 deletions(-) > > diff --git a/gdb/gdbserver/server.c b/gdb/gdbserver/server.c > index 8e0bf5b..59d648d 100644 > --- a/gdb/gdbserver/server.c > +++ b/gdb/gdbserver/server.c > @@ -1157,6 +1157,98 @@ handle_search_memory (char *own_buf, int > packet_len) > free (pattern); > } > > +static void > +handle_detach (char *own_buf) > +{ > + require_running_or_return (own_buf); > + > + int pid; > + > + if (multi_process) > + { > + /* skip 'D;' */ > + pid = strtol (&own_buf[2], NULL, 16); > + } > + else > + pid = ptid_get_pid (current_ptid); > + > + if ((tracing && disconnected_tracing) || any_persistent_commands ()) > + { > + struct process_info *process = find_process_pid (pid); > + > + if (process == NULL) > + { > + write_enn (own_buf); > + return; > + } > + > + if (tracing && disconnected_tracing) > + fprintf (stderr, > + "Disconnected tracing in effect, " > + "leaving gdbserver attached to the process\n"); > + > + if (any_persistent_commands ()) > + fprintf (stderr, > + "Persistent commands are present, " > + "leaving gdbserver attached to the process\n"); > + > + /* Make sure we're in non-stop/async mode, so we we can both > + wait for an async socket accept, and handle async target > + events simultaneously. There's also no point either in > + having the target stop all threads, when we're going to > + pass signals down without informing GDB. */ > + if (!non_stop) > + { > + if (debug_threads) > + debug_printf ("Forcing non-stop mode\n"); > + > + non_stop = 1; > + start_non_stop (1); > + } > + > + process->gdb_detached = 1; > + > + /* Detaching implicitly resumes all threads. */ > + target_continue_no_signal (minus_one_ptid); > + > + write_ok (own_buf); > + return; > + } > + > + fprintf (stderr, "Detaching from process %d\n", pid); > + stop_tracing (); > + if (detach_inferior (pid) != 0) > + write_enn (own_buf); > + else > + { > + discard_queued_stop_replies (pid_to_ptid (pid)); > + write_ok (own_buf); > + > + if (extended_protocol || target_running ()) > + { > + /* There is still at least one inferior remaining or > + we are in extended mode, so don't terminate gdbserver, > + and instead treat this like a normal program exit. */ > + last_status.kind = TARGET_WAITKIND_EXITED; > + last_status.value.integer = 0; > + last_ptid = pid_to_ptid (pid); > + > + current_thread = NULL; > + } > + else > + { > + putpkt (own_buf); > + remote_close (); > + > + /* If we are attached, then we can exit. Otherwise, we > + need to hang around doing nothing, until the child is > + gone. */ > + join_inferior (pid); > + exit (0); > + } > + } > +} > + > /* Parse options to --debug-format= and "monitor set debug-format". > ARG is the text after "--debug-format=" or "monitor set > debug-format". > IS_MONITOR is non-zero if we're invoked via "monitor set > debug-format". > @@ -4009,7 +4101,6 @@ process_serial_event (void) > unsigned int len; > int res; > CORE_ADDR mem_addr; > - int pid; > unsigned char sig; > int packet_len; > int new_packet_len = -1; > @@ -4037,91 +4128,7 @@ process_serial_event (void) > handle_general_set (own_buf); > break; > case 'D': > - require_running_or_break (own_buf); > - > - if (multi_process) > - { > - i++; /* skip ';' */ > - pid = strtol (&own_buf[i], NULL, 16); > - } > - else > - pid = ptid_get_pid (current_ptid); > - > - if ((tracing && disconnected_tracing) || any_persistent_commands > ()) > - { > - struct process_info *process = find_process_pid (pid); > - > - if (process == NULL) > - { > - write_enn (own_buf); > - break; > - } > - > - if (tracing && disconnected_tracing) > - fprintf (stderr, > - "Disconnected tracing in effect, " > - "leaving gdbserver attached to the process\n"); > - > - if (any_persistent_commands ()) > - fprintf (stderr, > - "Persistent commands are present, " > - "leaving gdbserver attached to the process\n"); > - > - /* Make sure we're in non-stop/async mode, so we we can both > - wait for an async socket accept, and handle async target > - events simultaneously. There's also no point either in > - having the target stop all threads, when we're going to > - pass signals down without informing GDB. */ > - if (!non_stop) > - { > - if (debug_threads) > - debug_printf ("Forcing non-stop mode\n"); > - > - non_stop = 1; > - start_non_stop (1); > - } > - > - process->gdb_detached = 1; > - > - /* Detaching implicitly resumes all threads. */ > - target_continue_no_signal (minus_one_ptid); > - > - write_ok (own_buf); > - break; /* from switch/case */ > - } > - > - fprintf (stderr, "Detaching from process %d\n", pid); > - stop_tracing (); > - if (detach_inferior (pid) != 0) > - write_enn (own_buf); > - else > - { > - discard_queued_stop_replies (pid_to_ptid (pid)); > - write_ok (own_buf); > - > - if (extended_protocol || target_running ()) > - { > - /* There is still at least one inferior remaining or > - we are in extended mode, so don't terminate gdbserver, > - and instead treat this like a normal program exit. */ > - last_status.kind = TARGET_WAITKIND_EXITED; > - last_status.value.integer = 0; > - last_ptid = pid_to_ptid (pid); > - > - current_thread = NULL; > - } > - else > - { > - putpkt (own_buf); > - remote_close (); > - > - /* If we are attached, then we can exit. Otherwise, we > - need to hang around doing nothing, until the child is > - gone. */ > - join_inferior (pid); > - exit (0); > - } > - } > + handle_detach (own_buf); > break; > case '!': > extended_protocol = 1; Self-comment: the variable i in process_serial_event becomes useless, I'll remove it before pushing.