From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22627 invoked by alias); 18 Mar 2008 02:47:49 -0000 Received: (qmail 22594 invoked by uid 22791); 18 Mar 2008 02:47:33 -0000 X-Spam-Check-By: sourceware.org Received: from viper.snap.net.nz (HELO viper.snap.net.nz) (202.37.101.8) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 18 Mar 2008 02:47:05 +0000 Received: from kahikatea.snap.net.nz (89.30.255.123.static.snap.net.nz [123.255.30.89]) by viper.snap.net.nz (Postfix) with ESMTP id 37B6C3DAA41; Tue, 18 Mar 2008 15:47:03 +1300 (NZDT) Received: by kahikatea.snap.net.nz (Postfix, from userid 1000) id BFED78FC6D; Tue, 18 Mar 2008 14:46:59 +1200 (NZST) From: Nick Roberts MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-ID: <18399.11554.144087.991511@kahikatea.snap.net.nz> Date: Tue, 18 Mar 2008 02:47:00 -0000 To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: linux native async mode support In-Reply-To: <200803171605.24276.pedro@codesourcery.com> References: <200803140810.22883.pedro@codesourcery.com> <20080314211646.GK31663@caradoc.them.org> <200803171605.24276.pedro@codesourcery.com> X-Mailer: VM 7.19 under Emacs 23.0.60.42 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-03/txt/msg00252.txt.bz2 I've really gone through your patch to try to understand it, not review it but you might as well have my thoughts. > * linux-nat.h (re_check_for_thread_db): Declare. recheck_for_thread_db (assuming it means "check again"). The word "re" on it's own is an abbreviation for "regarding" and in GDB it's generally used for "regular expression". >... > +static void > +clear_waitpid_queue (void) > +{ > + struct waitpid_result *event = waitpid_queue; > + while (event) > + { > + struct waitpid_result *next = event->next; > + xfree (event); > + event = next; > + } > + waitpid_queue = NULL; > +} struct waitpid_result *next, *event = waitpid_queue; while (event) { next = event->next; xfree (event); event = next; I guess the compiler will optimise it anyway but shouldn't the declaration be outside the loop? >... > +/* Disable async mode. */ > + > +static void > +linux_nat_disable_async (void) and > +/* Enable async mode. */ > + > +static void > +linux_nat_enable_async (void) Previously you've added linux_nat_async_events (int enable). So should this be: linux_nat_toggle_async (int enable) for consistency? (or linux_nat_async_events -> linux_nat_async_disable_events linux_nat_async_enable_events) As a rule what does GDB do elsewhere in these situations? >... > + add_setshow_zinteger_cmd ("lin-lwp-async", no_class, > + &debug_linux_nat_async, _("\ > +Set debugging of GNU/Linux async lwp module."), _("\ > +Show debugging of GNU/Linux async lwp module."), _("\ > +Enables printf debugging output."), > + NULL, > + show_debug_linux_nat_async, > + &setdebuglist, &showdebuglist); add_setshow_boolean_cmd ("lin-lwp-async", class_maintenance, I guess you copied from "lin-lwp" but I think this should be boolean too (only a few currently are: timestamp and xml) The other "set debug" commands are also in class_maintenance, although I'm not sure what effect that has here. -- Nick http://www.inet.net.nz/~nickrob