Re: [RFA] cleanup of syscall consts in process record

Joel Brobecker
2009-09-08T18:04:21Z

> 2009-09-08 Michael Snyder <msnyder@vmware.com>
>
> * 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.

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.

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.

--
Joel