Re: [PATCH] Fix cygwin build error with i386-linux-tdep.c

Michael Snyder
2009-09-05T20:34:06Z

Hui Zhu wrote:

> Hi guys,
>
> I make a new patch that change regcache_raw_read to regcache_raw_read_unsigned.
> Please help me review it.
>
> Thanks,
> Hui
>
> 2009-08-31 Hui Zhu <teawater@gmail.com>
>
> * i386-linux-tdep.c (i386_linux_intx80_sysenter_record): Change
> regcache_raw_read to regcache_raw_read_unsigned.
>
> ---
> i386-linux-tdep.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> --- a/i386-linux-tdep.c
> +++ b/i386-linux-tdep.c
> @@ -367,18 +367,18 @@ static int
> i386_linux_intx80_sysenter_record (struct regcache *regcache)
> {
> int ret;
> - uint32_t tmpu32;
> + ULONGEST num;

I like Mark's suggestion of calling it "syscall".
Calling it "num" is not much better than calling it "tmp". ;-)

Now, talking about signed vs. unsigned vs. casts...

You declared it a ULONGEST because that's what
regcache_raw_read_unsigned expects, right? -- fair enough.
See below.

> - regcache_raw_read (regcache, I386_EAX_REGNUM, (gdb_byte *) &tmpu32);
> + regcache_raw_read_unsigned (regcache, I386_EAX_REGNUM, &num);
>
> - if (tmpu32 > 499)
> + if (num > 499)
> {
> printf_unfiltered (_("Process record and replay target doesn't "
> - "support syscall number %u\n"), tmpu32);
> + "support syscall number %d\n"), (int) num);
> return -1;
> }
>
> - ret = record_linux_system_call (tmpu32, regcache,
> + ret = record_linux_system_call ((int) num, regcache,
> &i386_linux_record_tdep);

But here you cast it to int because that's what
record_linux_system_call expects, right?

And since we know at this point that the value is between
0 and 499, the cast can't really do any harm, so it seems
fair.

I could suggest casting it to (unsigned int), but it wouldn't
really make any difference, would it? Mark -- Jiang -- would
that make you guys more comfortable?

Or would it be more correct to change the prototype of
linux-record-system-call so that it expects an unsigned int?

> if (ret)
> return ret;
>