From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9384 invoked by alias); 4 Nov 2008 16:33:27 -0000 Received: (qmail 9291 invoked by uid 22791); 4 Nov 2008 16:33:25 -0000 X-Spam-Check-By: sourceware.org Received: from mx2.redhat.com (HELO mx2.redhat.com) (66.187.237.31) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 04 Nov 2008 16:32:48 +0000 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id mA4GWjio006907; Tue, 4 Nov 2008 11:32:45 -0500 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx2.corp.redhat.com (8.13.1/8.13.1) with ESMTP id mA4GWioD018042; Tue, 4 Nov 2008 11:32:44 -0500 Received: from opsy.redhat.com (vpn-14-82.rdu.redhat.com [10.11.14.82]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id mA4GWhrv000887; Tue, 4 Nov 2008 11:32:44 -0500 Received: by opsy.redhat.com (Postfix, from userid 500) id 30006508089; Tue, 4 Nov 2008 09:32:43 -0700 (MST) To: =?utf-8?Q?S=C3=A9rgio?= Durigan =?utf-8?Q?J=C3=BAnior?= Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 4/4] 'catch syscall' feature -- Build system, documentation and testcase References: <1225773088.24532.56.camel@miki> From: Tom Tromey Reply-To: tromey@redhat.com X-Attribution: Tom Date: Tue, 04 Nov 2008 16:33:00 -0000 In-Reply-To: <1225773088.24532.56.camel@miki> (=?utf-8?Q?=22S=C3=A9rgio?= Durigan =?utf-8?Q?J=C3=BAnior=22's?= message of "Tue\, 04 Nov 2008 02\:31\:28 -0200") Message-ID: User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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/msg00038.txt.bz2 >>>>> "S=C3=A9rgio" =3D=3D S=C3=A9rgio Durigan J=C3=BAnior writes: S=C3=A9rgio> This last part modifies the build system (in order to S=C3=A9rgio> implement the GDB's datadir), adds the documentation and the S=C3=A9rgio> testcase for the new feature. Nice. S=C3=A9rgio> +xml-syscall-copy: S=C3=A9rgio> + list=3D'$(XML_SYSCALLS_FILES)' ; \ S=C3=A9rgio> + for file in $$list ; do \ Two nits here. First, if you build with builddir!=3Dsrcdir, you need to mkdir XML_SYSCALLS_DIR here. Second, I think it would be nice to detect the case where builddir=3D=3Dsrcdir and not copy in that case. S=C3=A9rgio> +xml-syscall-install: S=C3=A9rgio> + $(SHELL) $(srcdir)/../mkinstalldirs \ S=C3=A9rgio> + $(DESTDIR)$(GDB_DATADIR_PATH)/$(XML_SYSCALLS_DIR) ; \ Using DESTDIR is nice attention to detail :-) S=C3=A9rgio> +# Also, the "install" target installs the syscalls' XML files= in the system. S=C3=A9rgio> +install: all install-only xml-syscall-install I think install-only should depend on xml-syscall-install. I believe the intent here is that "make install-only" will install everything without rebuilding. S=C3=A9rgio> +AC_ARG_WITH(gdb-datadir, S=C3=A9rgio> +[ --with-gdb-datadir=3Dpath Look for global separate data = files in this path [DATADIR/gdb]], It is preferable, IMO, to use AS_HELP_STRING here. There are some examples of this in gdb's configure.ac. S=C3=A9rgio> +AC_CONFIG_SUBDIRS(doc testsuite syscalls) =20 You don't want this. There is no syscalls directory, and even if there was, I would argue that it should not have its own configure script. S=C3=A9rgio> +@item syscall S=C3=A9rgio> +@itemx syscall @var{syscall name} S=C3=A9rgio> +@cindex break on call to/return from a system call S=C3=A9rgio> +A call to/return from a @code{syscall} -- similar to the @cod= e{strace} S=C3=A9rgio> +utility. This needs a minor update now that "catch syscall" can take multiple arguments. I didn't read the implementation patch in detail... if it parses numeric syscalls, then that should be mentioned here. Finally, instead of "call to/return from", how about "call to or return from"? This is more grammatical and not much longer :-) Tom