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

Hui Zhu
2009-08-31T08:23:14Z

On Sun, Aug 30, 2009 at 22:37, Hui Zhu<teawater@gmail.com> wrote:
> On Sun, Aug 30, 2009 at 22:09, Hui Zhu<teawater@gmail.com> wrote:
>> On Sun, Aug 30, 2009 at 21:56, Mark Kettenis<mark.kettenis@xs4all.nl> wrote:
>>>> From: Hui Zhu <teawater@gmail.com>
>>>> Date: Sun, 30 Aug 2009 21:15:22 +0800
>>>>
>>>> 2009-08-29  Hui Zhu  <teawater@gmail.com>
>>>>
>>>>       * i386-linux-tdep.c (i386_linux_intx80_sysenter_record): Add
>>>>       (unsigned) before tmpu32.
>>>
>>> Ugh!  Casts like that are ugly.
>>>
>>> This made me look at the code again and realize that what you're doing
>>> in that function is wrong.  You should be using
>>> regcache_{raw|cooked}_read_unsigned() instead of regcache_raw_read().
>>> Then the whole issue of printing an uint32_t goes away.  When you do
>>> change the code like that please use a more meaningful variable name
>>> instead of 'tmpu32'.  My suggestion would be 'syscall'.
>>
>> For the regcache_raw_read_unsigned, I am not agres with it.
>>
>> void
>> regcache_raw_read_unsigned (struct regcache *regcache, int regnum,
>>                            ULONGEST *val)
>> {
>>  gdb_byte *buf;
>>  gdb_assert (regcache != NULL);
>>  gdb_assert (regnum >= 0 && regnum < regcache->descr->nr_raw_registers);
>>  buf = alloca (regcache->descr->sizeof_register[regnum]);
>>  regcache_raw_read (regcache, regnum, buf);
>>  (*val) = extract_unsigned_integer
>>             (buf, regcache->descr->sizeof_register[regnum],
>>              gdbarch_byte_order (regcache->descr->gdbarch));
>> }
>>
>> It just add a "extract_unsigned_integer".  For this code, it in
>> i386-linux-tdep.c.  We know that I386_EAX_REGNUM is 32 bits.  So we
>> don't need extract_unsigned_integer to set anything.
>>
>> Thanks,
>> Hui
>
> Oops, I forget that we have the byte order trouble.  Thanks for remind
> me.  I will fix it.
>
> Thanks,
> Hui
>
>>
>>>
>>> Cheers,
>>>
>>> Mark
>>>
>>>> Index: gdb/i386-linux-tdep.c
>>>> ===================================================================
>>>> --- gdb.orig/i386-linux-tdep.c        2009-08-23 21:17:37.000000000 +0800
>>>> +++ gdb/i386-linux-tdep.c     2009-08-30 20:19:53.828125000 +0800
>>>> @@ -374,7 +374,7 @@
>>>>    if (tmpu32 > 499)
>>>>      {
>>>>        printf_unfiltered (_("Process record and replay target doesn't "
>>>> -                           "support syscall number %u\n"), tmpu32);
>>>> +                           "support syscall number %u\n"), (unsigned) tmpu32);
>>>>        return -1;
>>>>      }
>>>>
>>>
>>
>

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;

- 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);
if (ret)
return ret;