From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 14762 invoked by alias); 25 Feb 2007 01:57:44 -0000 Received: (qmail 14443 invoked by uid 22791); 25 Feb 2007 01:57:41 -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; Sun, 25 Feb 2007 01:57:36 +0000 Received: from localhost (localhost [127.0.0.1]) by elrond.portugalmail.pt (Postfix) with ESMTP id C4C4037DB4; Sun, 25 Feb 2007 01:57:29 +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 vSbQJJEEClCW; Sun, 25 Feb 2007 01:57:29 +0000 (WET) Received: from [127.0.0.1] (62.169.107.209.rev.optimus.pt [62.169.107.209]) (Authenticated sender: pedro_alves@portugalmail.pt) by elrond.portugalmail.pt (Postfix) with ESMTP id B513437D03; Sun, 25 Feb 2007 01:57:25 +0000 (WET) Message-ID: <45E0ECB2.3060906@portugalmail.pt> Date: Sun, 25 Feb 2007 01:57: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> <45E0AB9E.9060107@portugalmail.pt> <45E0CA5A.1020005@champenstudios.com> In-Reply-To: <45E0CA5A.1020005@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/msg00307.txt.bz2 Lerele escreveu: > Pedro Alves wrote: > >> >> 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? >> > You mean the comment? Yep. Sorry I wasn't clear. >> Full stop, double space. "...get_child_debug_event. Default ..." >> > Please clarify. > The GNU coding convention states that you use a full stop followed by two spaces to end a sentence. >>> - return res; >>> + return res? 0:-1; >>> >> >> Space around '?' and ':'. Personally, I liked the res != FALSE ? 0 : >> -1 version better. >> > > Honestly, I don't really care too much. > You guys decide. > I don't care much either, but I think the spaces around '?' and ':' are mandatory. + return res ? 0 : -1; >> >>> +/* 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) ) >>> >> >> 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. >> > > You mean line gdb/signals/signal.c 'target_signal_from_host' and > 'do_target_signal_to_host' functions? > I'm not really sure if we should use SIGINT in win32-i386-low.c, given > that it should only know about target signals. > The only send_signal calls I could find, are in remote_utils like so: (*the_target->send_signal) (SIGINT); So, *pedantically*, it should be: +static void +win32_send_signal (int signo) +{ + if (signo == SIGINT) And I agree that it doesn't look right. With my patch installed, it turns to: +static void +win32_send_signal (enum target_signal signo) +{ + if (signo == TARGET_SIGNAL_INT) Anyway, don't let this bother you. If it gets OKed as is, I'll take care of it later. Cheers, Pedro Alves