From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6207 invoked by alias); 24 Feb 2007 21:19:20 -0000 Received: (qmail 6198 invoked by uid 22791); 24 Feb 2007 21:19:19 -0000 X-Spam-Check-By: sourceware.org Received: from elrond.portugalmail.pt (HELO elrond.portugalmail.pt) (195.245.179.181) by sourceware.org (qpsmtpd/0.31) with ESMTP; Sat, 24 Feb 2007 21:19:14 +0000 Received: from localhost (localhost [127.0.0.1]) by elrond.portugalmail.pt (Postfix) with ESMTP id 371F837EE3; Sat, 24 Feb 2007 21:19:07 +0000 (WET) Received: from elrond.portugalmail.pt ([127.0.0.1]) by localhost (elrond.portugalmail.pt [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7M+ybhDoXKrj; Sat, 24 Feb 2007 21:19:06 +0000 (WET) Received: from [127.0.0.1] (88.210.67.91.rev.optimus.pt [88.210.67.91]) (Authenticated sender: pedro_alves@portugalmail.pt) by elrond.portugalmail.pt (Postfix) with ESMTP id 390FA38032; Sat, 24 Feb 2007 21:18:55 +0000 (WET) Message-ID: <45E0AB9E.9060107@portugalmail.pt> Date: Sat, 24 Feb 2007 21:19:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.0.9) Gecko/20061207 Thunderbird/1.5.0.9 Mnenhy/0.7.4.0 MIME-Version: 1.0 To: Lerele CC: Eli Zaretskii , gdb-patches@sourceware.org Subject: Re: [Patch] Win32 gdbserver new interrupt support, and attach to process fix. References: <45DF7E27.10102@champenstudios.com> <45E0376D.9010605@champenstudios.com> <45E05872.1040605@champenstudios.com> In-Reply-To: <45E05872.1040605@champenstudios.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-Antivirus: avast! (VPS 000716-3, 23-02-2007), Outbound message X-Antivirus-Status: Clean 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: 2007-02/txt/msg00302.txt.bz2 Lerele wrote: > } > -#endif > + > +/* Expose 'static void input_interrupt (int unused)' function to enable checking for a > + remote interrupt request. */ > Is the 'Expose 'static void input_interrupt (int unused)' function' part really necessary? > +void > +check_remote_input_interrupt_request (void) > +{ > + input_interrupt(0); > +} > > > + > + It should be safe to continue child given this wait status. > + See function get_child_debug_event. Default wait status is spurious, > + and it gets modified if any important debug events get received. > + More specifically, this status gets returned in the wait loop to > + allow socket pooling/resuming, to allow for remote interruption. */ > Full stop, double space. "...get_child_debug_event. Default ..." > TARGET_WAITKIND_SPURIOUS, > }; > > @@ -574,7 +584,7 @@ > > FreeLibrary (kernel32); > > - return res; > + return res? 0:-1; > Space around '?' and ':'. Personally, I liked the res != FALSE ? 0 : -1 version better. > } > > /* Kill all inferiors. */ > @@ -840,7 +850,12 @@ > last_sig = TARGET_SIGNAL_0; > ourstatus->kind = TARGET_WAITKIND_SPURIOUS; > > - if (!(debug_event = WaitForDebugEvent (¤t_event, 1000))) > + /* Check for remote interruption request */ > + check_remote_input_interrupt_request(); > Missing space: "check_remote_input_interrupt_request ()" Full stop double-space: "request. */", although I guess the comment adds no value, since the function name says pretty much the same. > +/* Send a signal to the inferior process, however is appropriate. */ > +static void > +win32_send_signal (int signo) > +{ > + if ( signo == TARGET_SIGNAL_INT ) > + { > + if ( !GenerateConsoleCtrlEvent(CTRL_BREAK_EVENT, current_process_id) ) > Missing space: "GenerateConsoleCtrlEvent (" Also, pedantically speaking, both remote_utils.c:putpkt_binary, and remote_utils.c:input_interrupt (the sole users of _send_signal) send SIGINT, not TARGET_SIGNAL_INT. Luckily on Windows, both are defined as 2. I have a local patch that does: /* Send a signal to the inferior process, however is appropriate. */ - void (*send_signal) (int); + void (*send_signal) (enum target_signal); ... and takes care of the conversion of the target side. I'll post it for review. In the meantime, you may want to change your patch to use SIGINT. Thanks for such a needed feature. Cheers, Pedro Alves