From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30674 invoked by alias); 4 Nov 2008 21:13:19 -0000 Received: (qmail 30568 invoked by uid 22791); 4 Nov 2008 21:13:18 -0000 X-Spam-Check-By: sourceware.org Received: from mtaout1.012.net.il (HELO mtaout1.012.net.il) (84.95.2.1) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 04 Nov 2008 21:12:35 +0000 Received: from conversion-daemon.i-mtaout1.012.net.il by i-mtaout1.012.net.il (HyperSendmail v2007.08) id <0K9T00200UWJXX00@i-mtaout1.012.net.il> for gdb-patches@sourceware.org; Tue, 04 Nov 2008 23:14:14 +0200 (IST) Received: from HOME-C4E4A596F7 ([77.126.241.172]) by i-mtaout1.012.net.il (HyperSendmail v2007.08) with ESMTPA id <0K9T006GZUZQCGK0@i-mtaout1.012.net.il>; Tue, 04 Nov 2008 23:14:14 +0200 (IST) Date: Tue, 04 Nov 2008 21:13:00 -0000 From: Eli Zaretskii Subject: Re: [PATCH 1/4] 'catch syscall' feature -- Architecture-independent part In-reply-to: <1225773079.24532.52.camel@miki> X-012-Sender: halo1@inter.net.il To: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= Cc: gdb-patches@sourceware.org Reply-to: Eli Zaretskii Message-id: References: <1225773079.24532.52.camel@miki> 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/msg00050.txt.bz2 > From: =?ISO-8859-1?Q?S=E9rgio?= Durigan =?ISO-8859-1?Q?J=FAnior?= > Date: Tue, 04 Nov 2008 02:31:19 -0200 Thanks, but I cannot say I like the non-portable assumptions of this design. For example: > +void > +clear_syscall_catchpoints_info (void) > +{ > + struct breakpoint *b; > + > + ALL_BREAKPOINTS (b) > + if (syscall_catchpoint_p (b)) > + b->syscall_number = UNKNOWN_SYSCALL; > +} ^^^^^^^^^^^^^^^^^ Who said that a syscall is necessarily defined by some number? More generally, let's say I'd like to implement support for this on Windows -- how would I need to go about it? > + /* We are in a entry breakpoint. */ > + entry_breakpoint, > + This comment is too obscure, IMO. Please make it more explicit; I think you want to say that it's an entry to a syscall, right? > + const char *syscall_name = > + gdbarch_syscall_name_from_number (current_gdbarch, b->syscall_number); Does this mean the notion that a syscall is a number have crept into gdbarch, which is supposed to be the most architecture-independent part of GDB? > + /* Checking if the user provided a syscall name or a number. */ > + if (isdigit (cur_name[0])) Is the assumption that no name will ever begin with a digit universally valid? > + error (_("The number '%d' does not represent a valid syscall."), ^^^^^^^^^^^^^^^ "a known syscall", I'd say. > + if (syscall_number == UNKNOWN_SYSCALL) > + error (_("Invalid syscall name '%s'."), cur_name); ^^^^^^^ "Unknown" is more accurate. > static void > @@ -7441,6 +7792,7 @@ delete_command (char *arg, int from_tty) > b->type != bp_shlib_event && > b->type != bp_thread_event && > b->type != bp_overlay_event && > + b->type != bp_entry_breakpoint && The previous lines used TABs and spaces for indentation, while your line uses only spaces. > + add_catch_command ("syscall", _("\ > +Catch calls to syscalls.\n\ "call to syscalls" is awkward. I suggest to drop the "calls to" part. > +With an argument, catch only calls of that syscall."), Ditto, suggest to drop the "calls of" part. > + add_setshow_filename_cmd ("gdb_datadir", class_maintenance, > + &gdb_datadir, _("Set GDB's datadir path."), > + _("Show GDB's datadir path."), > + _("\ > +When set, GDB uses the specified path to search for data files."), > + NULL, NULL, > + &maintenance_set_cmdlist, > + &maintenance_show_cmdlist); This should be described in the manual.