From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 32080 invoked by alias); 8 Sep 2009 19:31:20 -0000 Received: (qmail 32068 invoked by uid 22791); 8 Sep 2009 19:31:17 -0000 X-SWARE-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from sibelius.xs4all.nl (HELO sibelius.xs4all.nl) (83.163.83.176) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 08 Sep 2009 19:31:09 +0000 Received: from brahms.sibelius.xs4all.nl (kettenis@localhost [127.0.0.1]) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3) with ESMTP id n88JUoqI028358; Tue, 8 Sep 2009 21:30:50 +0200 (CEST) Received: (from kettenis@localhost) by brahms.sibelius.xs4all.nl (8.14.3/8.14.3/Submit) id n88JUnWn026850; Tue, 8 Sep 2009 21:30:49 +0200 (CEST) Date: Tue, 08 Sep 2009 19:31:00 -0000 Message-Id: <200909081930.n88JUnWn026850@brahms.sibelius.xs4all.nl> From: Mark Kettenis To: msnyder@vmware.com CC: brobecker@adacore.com, gdb-patches@sourceware.org, teawater@gmail.com In-reply-to: <4AA6AB33.20304@vmware.com> (message from Michael Snyder on Tue, 08 Sep 2009 12:06:27 -0700) Subject: Re: [RFA] cleanup of syscall consts in process record References: <4AA2D95B.4090904@vmware.com> <20090908164934.GP30677@adacore.com> <4AA696F6.6040503@vmware.com> <4AA69878.9060602@vmware.com> <20090908180402.GR30677@adacore.com> <4AA6AB33.20304@vmware.com> 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: 2009-09/txt/msg00220.txt.bz2 > Date: Tue, 08 Sep 2009 12:06:27 -0700 > From: Michael Snyder > > Joel Brobecker wrote: > >> 2009-09-08 Michael Snyder > >> > >> * amd64-linux-tdep.h (enum amd64_syscall): New enum consts, > >> to replace literal consts used in amd64-linux-tdep.c > >> * linux-record.h (enum gdb_syscall): New enum consts, to replace > >> literal consts used in amd64-linux-tdep.c and linux-record.c. > >> * amd64-linux-tdep.c (amd64_canonicalize_syscall): New function, > >> translate from native amd64 Linux syscall id to internal gdb id. > >> (amd64_linux_syscall_record): Switch statement abstracted out > >> and replaced with a call to amd64_canonicalize_syscall. > >> * linux-record.c (record_linux_system_call): Replace literal > >> consts with enum consts. > >> * i386-linux-tdep.c (i386_canonicalize_syscall): New function, > >> trivially translate from native i386 Linux syscalls to gdb syscalls. > >> (i386_linux_intx80_sysenter_record): > > > > Nice! I really like this version much better. The approach you took > > with i386 made me wonder whether we really need the amd64_syscall enum > > at all - we could have used a plain int as the argument to > > amd64_canonicalize_syscall, and use plain numbers there, rather than > > having an enum that's only used there. I don't mind, though, so > > don't worry about it unless you agree as well. > > Nah, the idea was to get rid of magic numbers, > plus it makes the code more readable. Self-documenting. > > > > Note that this should also fix the issue that Hui reported about > > building on cygwin with --enable-64-bit-bfd. So I'll remove Hui's > > patch from my list. > > Yes, I broke down and included Hui's patch in this one. > > > Just one comment: > > > >> +static enum gdb_syscall > >> +i386_canonicalize_syscall (int syscall) > >> +{ > >> + enum { i386_syscall_max = 499 }; > >> + > >> + if (syscall <= i386_syscall_max) > >> + return syscall; > > > > I thought that we should incorporate Mark's suggestion of checking > > syscall against negative values. But I now realize that if syscall > > is negative, we'll return a value that's equivalent to returning -1. > > And the check against negative values in i386_linux_intx80_sysenter_record > > should then catch it. > > Correct. > > So this is approved, then? Fine with me.