[Prec/RFA] fix build error of prec in cygwin

[Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 2957 bytes --]

Hi guys,

This patch is to fix build error of prec in cgywin.
Pierre had help me tested it both in 32bit and 64bit CORE_ADDR length.

Please help me review it.

2009-05-11  Hui Zhu  <teawater@gmail.com>

	Add sys_fcntl argument interfaces to linux_record_tdep.

	* linux-record.h (linux_record_tdep): Add fcntl_F_GETLK,
	fcntl_F_GETLK64, fcntl_F_SETLK64 and fcntl_F_SETLKW64 to be
	interfaces.
	* i386-linux-tdep.c (I386_LINUX_RECORD_FCNTL_F_GETLK,
	I386_LINUX_RECORD_FCNTL_F_GETLK64,
	I386_LINUX_RECORD_FCNTL_F_SETLK64,
	I386_LINUX_RECORD_FCNTL_F_SETLKW64): New macros. The values
	of I386 Linux sys_fcntl arguments.
	(i386_linux_init_abi): Set macros values to linux_record_tdep.
	* linux-record.c (record_linux_system_call): Change to use
	the interface in linux_record_tdep in sys_fcntl and
	sys_fcntl64.

	* linux-record.c (record_linux_system_call): Fix the process
	record build error about type in CYGWIN.


Thanks,
Hui


On Fri, May 8, 2009 at 20:11, Pierre Muller <muller@ics.u-strasbg.fr> wrote:
> I tested this both in 32bit and 64bit CORE_ADDR length.
>
> I can confirm that cygwin compilation is fixed in both
> cases with that patch and hope it will be approved
> rapidly.
>
>  Thanks, Hui.
>
>
> Pierre Muller
> Pascal language support maintainer for GDB
>
>
>
>
>> -----Message d'origine-----
>> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
>> owner@sourceware.org] De la part de Hui Zhu
>> Envoyé : Friday, May 08, 2009 7:12 AM
>> À : Pierre Muller
>> Cc : gdb-patches@sourceware.org; Michael Snyder; Pedro Alves
>> Objet : Re: Process record and replay checked in to main trunk
>>
>> Hi Pierre,
>>
>> I make a patch to fix this bug, but I don't have cygwin.
>>
>> Could you please help me test this patch in cygwin?
>>
>> Thanks,
>> Hui
>>
>> On Fri, May 8, 2009 at 07:06, Pedro Alves <pedro@codesourcery.com>
>> wrote:
>> > On Thursday 07 May 2009 23:43:13, Michael Snyder wrote:
>> >> Guessing it isn't implemented for 64 bit.
>> >
>> > I don't think that's the problem here.  Cygwin is 32-bit, and has
>> > 32-bit ints.
>> >
>> >> ../../purecvs/gdb/linux-record.c:397: warning: unsigned int format,
>> >> uint32_t arg
>> >
>> >> ../../purecvs/gdb/linux-record.c:629: warning: int format, uint32_t
>> >> arg (arg
>> >
>> > Better use casts, or use plongest/pulongest/hex_string where
>> appropriate?
>> >
>> >> Perhaps we should have a --disable-process-record config option?
>> >> If only for a back-up plan?
>> >
>> > I think that would be more work than fixing this properly.  :-)
>> >
>> >> ../../purecvs/gdb/linux-record.c:1636: error: `F_GETLK64' undeclared
>> >> (first use in this function)
>> >
>> > There are a bunch of RECORD_* macros defined on top of linux-record.c
>> > to replace contants like these.  E.g., RECORD_Q_GETFMT.  Looks like
>> > this case was just missed.
>> >
>> > --
>> > Pedro Alves
>> >
>
>

[-- Attachment #2: fix-prec-cygwin-build-error.txt --]
[-- Type: text/plain, Size: 4434 bytes --]

---
 i386-linux-tdep.c |   14 ++++++++++++++
 linux-record.c    |   28 +++++++++++++---------------
 linux-record.h    |    7 +++++++
 3 files changed, 34 insertions(+), 15 deletions(-)

--- a/i386-linux-tdep.c
+++ b/i386-linux-tdep.c
@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
 #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP		0x545F
 #define I386_LINUX_RECORD_IOCTL_FIOQSIZE		0x5460
 
+/* The values of the second argument of system call "sys_fcntl"
+   and "sys_fcntl64".  The values of these macros were obtained from
+   Linux Kernel source.  */
+#define I386_LINUX_RECORD_FCNTL_F_GETLK			5
+#define I386_LINUX_RECORD_FCNTL_F_GETLK64		12
+#define I386_LINUX_RECORD_FCNTL_F_SETLK64		13
+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64		14
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -781,6 +789,12 @@ i386_linux_init_abi (struct gdbarch_info
     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
 
+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
+
   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
--- a/linux-record.c
+++ b/linux-record.c
@@ -394,7 +394,7 @@ record_linux_system_call (int num, struc
 	{
 	  printf_unfiltered (_("Process record and replay target doesn't "
 			       "support ioctl request 0x%08x.\n"),
-			     tmpu32);
+			     (int)tmpu32);
 	  return 1;
 	}
       break;
@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
       /* XXX */
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
     sys_fcntl:
-      if (tmpu32 == F_GETLK)
+      if (tmpu32 == tdep->fcntl_F_GETLK)
 	{
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
@@ -626,7 +626,7 @@ record_linux_system_call (int num, struc
 		    "It will free the memory addr = 0x%s len = %d.  "
 		    "It will make record target get error.  "
 		    "Do you want to stop the program?"),
-		  paddr_nz (tmpu32), len);
+		  paddr_nz (tmpu32), (int)len);
 	target_terminal_inferior ();
 	if (q)
 	  return 1;
@@ -935,7 +935,7 @@ record_linux_system_call (int num, struc
 	default:
 	  printf_unfiltered (_("Process record and replay target "
 			       "doesn't support socketcall call 0x%08x\n"),
-			     tmpu32);
+			     (int)tmpu32);
 	  return -1;
 	  break;
 	}
@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
       /* sys_fcntl64 */
     case 221:
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
-      switch (tmpu32)
-	{
-	case F_GETLK64:
+      if (tmpu32 == tdep->fcntl_F_GETLK64)
+        {
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
 	  if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
 	    return -1;
-	  break;
-	case F_SETLK64:
-	case F_SETLKW64:
-	  break;
-	default:
+	}
+      else if (tmpu32 != tdep->fcntl_F_SETLK64
+	       && tmpu32 != tdep->fcntl_F_SETLKW64)
+        {
 	  goto sys_fcntl;
-	  break;
 	}
       break;
 
@@ -1786,7 +1783,8 @@ record_linux_system_call (int num, struc
 		fprintf_unfiltered (gdb_stdlog,
 				    "Process record: error reading memory "
 				    "at addr = 0x%s len = %d.\n",
-				    paddr_nz (tmpu32), nr * tdep->size_int);
+				    paddr_nz (tmpu32),
+				    (int)(nr * tdep->size_int));
 	      return -1;
 	    }
 	  for (i = 0; i < nr; i++)
@@ -2196,7 +2194,7 @@ record_linux_system_call (int num, struc
     default:
       printf_unfiltered (_("Process record and replay target doesn't "
 			   "support syscall number 0x%08x\n"),
-			 tmpu32);
+			 (int)tmpu32);
       return -1;
       break;
     }
--- a/linux-record.h
+++ b/linux-record.h
@@ -158,6 +158,13 @@ struct linux_record_tdep
   int ioctl_TIOCSHAYESESP;
   int ioctl_FIOQSIZE;
 
+  /* The values of the second argument of system call "sys_fcntl"
+     and "sys_fcntl64".  */
+  int fcntl_F_GETLK;
+  int fcntl_F_GETLK64;
+  int fcntl_F_SETLK64;
+  int fcntl_F_SETLKW64;
+
   /* The number of the registers that are used as the arguments of
      a system call.  */
   int arg1;

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Mon, May 11, 2009 at 01:31:18AM +0800, Hui Zhu wrote:
>--- a/i386-linux-tdep.c
>+++ b/i386-linux-tdep.c
>@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
> #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP		0x545F
> #define I386_LINUX_RECORD_IOCTL_FIOQSIZE		0x5460
> 
>+/* The values of the second argument of system call "sys_fcntl"
>+   and "sys_fcntl64".  The values of these macros were obtained from
>+   Linux Kernel source.  */
>+#define I386_LINUX_RECORD_FCNTL_F_GETLK			5
>+#define I386_LINUX_RECORD_FCNTL_F_GETLK64		12
>+#define I386_LINUX_RECORD_FCNTL_F_SETLK64		13
>+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64		14
>+
> static void
> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
> {
>@@ -781,6 +789,12 @@ i386_linux_init_abi (struct gdbarch_info
>     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
>   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
> 
>+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
>+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
>+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
>+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
>+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
>+
>   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
>   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
>   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
>--- a/linux-record.c
>+++ b/linux-record.c
>@@ -394,7 +394,7 @@ record_linux_system_call (int num, struc
> 	{
> 	  printf_unfiltered (_("Process record and replay target doesn't "
> 			       "support ioctl request 0x%08x.\n"),
>-			     tmpu32);
>+			     (int)tmpu32);

How did a 0x%08x make it into the source after the "2009/04/17 15:44:28"
change which changed all %p's to host_address_to_string?  Shouldn't
this just be a call to host_address_to_string?

> 	  return 1;
> 	}
>       break;
>@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
>       /* XXX */
>       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>     sys_fcntl:
>-      if (tmpu32 == F_GETLK)
>+      if (tmpu32 == tdep->fcntl_F_GETLK)
> 	{
> 	  regcache_raw_read (regcache, tdep->arg3,
> 			     (gdb_byte *) & tmpu32);
>@@ -626,7 +626,7 @@ record_linux_system_call (int num, struc
> 		    "It will free the memory addr = 0x%s len = %d.  "
> 		    "It will make record target get error.  "
> 		    "Do you want to stop the program?"),
>-		  paddr_nz (tmpu32), len);
>+		  paddr_nz (tmpu32), (int)len);

If len is a uint32_t isn't casting it to an int the wrong thing to do?
Looking at the code, len is first defined as uint32_t, then a pointer to
it is cast as (gdb_byte *) (and it doesn't look like the space following
the '&' doesn't follow GNU coding standards).  So it is never actually
used as a uint32_t.  That doesn't seem right.

> 	target_terminal_inferior ();
> 	if (q)
> 	  return 1;
>@@ -935,7 +935,7 @@ record_linux_system_call (int num, struc
> 	default:
> 	  printf_unfiltered (_("Process record and replay target "
> 			       "doesn't support socketcall call 0x%08x\n"),
>-			     tmpu32);
>+			     (int)tmpu32);

Same observation.  This cast seems wrong.

> 	  return -1;
> 	  break;
> 	}
>@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
>       /* sys_fcntl64 */
>     case 221:
>       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>-      switch (tmpu32)
>-	{
>-	case F_GETLK64:
>+      if (tmpu32 == tdep->fcntl_F_GETLK64)
>+        {
> 	  regcache_raw_read (regcache, tdep->arg3,
> 			     (gdb_byte *) & tmpu32);
> 	  if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
> 	    return -1;
>-	  break;
>-	case F_SETLK64:
>-	case F_SETLKW64:
>-	  break;
>-	default:
>+	}
>+      else if (tmpu32 != tdep->fcntl_F_SETLK64
>+	       && tmpu32 != tdep->fcntl_F_SETLKW64)
>+        {
> 	  goto sys_fcntl;
>-	  break;
> 	}
>       break;
> 
>@@ -1786,7 +1783,8 @@ record_linux_system_call (int num, struc
> 		fprintf_unfiltered (gdb_stdlog,
> 				    "Process record: error reading memory "
> 				    "at addr = 0x%s len = %d.\n",
>-				    paddr_nz (tmpu32), nr * tdep->size_int);
>+				    paddr_nz (tmpu32),
>+				    (int)(nr * tdep->size_int));

...and so on...
> 	      return -1;
> 	    }
> 	  for (i = 0; i < nr; i++)
>@@ -2196,7 +2194,7 @@ record_linux_system_call (int num, struc
>     default:
>       printf_unfiltered (_("Process record and replay target doesn't "
> 			   "support syscall number 0x%08x\n"),
>-			 tmpu32);
>+			 (int)tmpu32);

...and so on...

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

On Mon, May 11, 2009 at 01:48, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Mon, May 11, 2009 at 01:31:18AM +0800, Hui Zhu wrote:
>>--- a/i386-linux-tdep.c
>>+++ b/i386-linux-tdep.c
>>@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
>> #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP         0x545F
>> #define I386_LINUX_RECORD_IOCTL_FIOQSIZE              0x5460
>>
>>+/* The values of the second argument of system call "sys_fcntl"
>>+   and "sys_fcntl64".  The values of these macros were obtained from
>>+   Linux Kernel source.  */
>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK                       5
>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK64             12
>>+#define I386_LINUX_RECORD_FCNTL_F_SETLK64             13
>>+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64            14
>>+
>> static void
>> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>> {
>>@@ -781,6 +789,12 @@ i386_linux_init_abi (struct gdbarch_info
>>     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
>>   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
>>
>>+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
>>+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
>>+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
>>+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
>>+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
>>+
>>   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
>>   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
>>   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
>>--- a/linux-record.c
>>+++ b/linux-record.c
>>@@ -394,7 +394,7 @@ record_linux_system_call (int num, struc
>>       {
>>         printf_unfiltered (_("Process record and replay target doesn't "
>>                              "support ioctl request 0x%08x.\n"),
>>-                           tmpu32);
>>+                           (int)tmpu32);
>
> How did a 0x%08x make it into the source after the "2009/04/17 15:44:28"
> change which changed all %p's to host_address_to_string?  Shouldn't
> this just be a call to host_address_to_string?
>
I am not sure %p or host_address_to_string is right.  It is not a
address.  Just a simple value.

>>         return 1;
>>       }
>>       break;
>>@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
>>       /* XXX */
>>       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>>     sys_fcntl:
>>-      if (tmpu32 == F_GETLK)
>>+      if (tmpu32 == tdep->fcntl_F_GETLK)
>>       {
>>         regcache_raw_read (regcache, tdep->arg3,
>>                            (gdb_byte *) & tmpu32);
>>@@ -626,7 +626,7 @@ record_linux_system_call (int num, struc
>>                   "It will free the memory addr = 0x%s len = %d.  "
>>                   "It will make record target get error.  "
>>                   "Do you want to stop the program?"),
>>-                paddr_nz (tmpu32), len);
>>+                paddr_nz (tmpu32), (int)len);
>
> If len is a uint32_t isn't casting it to an int the wrong thing to do?
> Looking at the code, len is first defined as uint32_t, then a pointer to
> it is cast as (gdb_byte *) (and it doesn't look like the space following
> the '&' doesn't follow GNU coding standards).  So it is never actually
> used as a uint32_t.  That doesn't seem right.
>

Use uint32_t because it's a 32 bits register's value.

>>       target_terminal_inferior ();
>>       if (q)
>>         return 1;
>>@@ -935,7 +935,7 @@ record_linux_system_call (int num, struc
>>       default:
>>         printf_unfiltered (_("Process record and replay target "
>>                              "doesn't support socketcall call 0x%08x\n"),
>>-                           tmpu32);
>>+                           (int)tmpu32);
>
> Same observation.  This cast seems wrong.
>
>>         return -1;
>>         break;
>>       }
>>@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
>>       /* sys_fcntl64 */
>>     case 221:
>>       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>>-      switch (tmpu32)
>>-      {
>>-      case F_GETLK64:
>>+      if (tmpu32 == tdep->fcntl_F_GETLK64)
>>+        {
>>         regcache_raw_read (regcache, tdep->arg3,
>>                            (gdb_byte *) & tmpu32);
>>         if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
>>           return -1;
>>-        break;
>>-      case F_SETLK64:
>>-      case F_SETLKW64:
>>-        break;
>>-      default:
>>+      }
>>+      else if (tmpu32 != tdep->fcntl_F_SETLK64
>>+             && tmpu32 != tdep->fcntl_F_SETLKW64)
>>+        {
>>         goto sys_fcntl;
>>-        break;
>>       }
>>       break;
>>
>>@@ -1786,7 +1783,8 @@ record_linux_system_call (int num, struc
>>               fprintf_unfiltered (gdb_stdlog,
>>                                   "Process record: error reading memory "
>>                                   "at addr = 0x%s len = %d.\n",
>>-                                  paddr_nz (tmpu32), nr * tdep->size_int);
>>+                                  paddr_nz (tmpu32),
>>+                                  (int)(nr * tdep->size_int));
>
> ...and so on...
>>             return -1;
>>           }
>>         for (i = 0; i < nr; i++)
>>@@ -2196,7 +2194,7 @@ record_linux_system_call (int num, struc
>>     default:
>>       printf_unfiltered (_("Process record and replay target doesn't "
>>                          "support syscall number 0x%08x\n"),
>>-                       tmpu32);
>>+                       (int)tmpu32);
>
> ...and so on...
>

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Mon, May 11, 2009 at 02:07:29AM +0800, Hui Zhu wrote:
>On Mon, May 11, 2009 at 01:48, Christopher Faylor
><cgf-use-the-mailinglist-please@sourceware.org> wrote:
>> On Mon, May 11, 2009 at 01:31:18AM +0800, Hui Zhu wrote:
>>>--- a/i386-linux-tdep.c
>>>+++ b/i386-linux-tdep.c
>>>@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
>>> #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP ? ? ? ? 0x545F
>>> #define I386_LINUX_RECORD_IOCTL_FIOQSIZE ? ? ? ? ? ? ?0x5460
>>>
>>>+/* The values of the second argument of system call "sys_fcntl"
>>>+ ? and "sys_fcntl64". ?The values of these macros were obtained from
>>>+ ? Linux Kernel source. ?*/
>>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK ? ? ? ? ? ? ? ? ? ? ? 5
>>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK64 ? ? ? ? ? ? 12
>>>+#define I386_LINUX_RECORD_FCNTL_F_SETLK64 ? ? ? ? ? ? 13
>>>+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64 ? ? ? ? ? ?14
>>>+
>>> static void
>>> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>> {
>>>@@ -781,6 +789,12 @@ i386_linux_init_abi (struct gdbarch_info
>>> ? ? I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
>>> ? i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
>>>
>>>+ ?i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
>>>+ ?i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
>>>+ ?i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
>>>+ ?i386_linux_record_tdep.fcntl_F_SETLKW64 =
>>>+ ? ?I386_LINUX_RECORD_FCNTL_F_SETLKW64;
>>>+
>>> ? i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
>>> ? i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
>>> ? i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
>>>--- a/linux-record.c
>>>+++ b/linux-record.c
>>>@@ -394,7 +394,7 @@ record_linux_system_call (int num, struc
>>> ? ? ? {
>>> ? ? ? ? printf_unfiltered (_("Process record and replay target doesn't "
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"support ioctl request 0x%08x.\n"),
>>>- ? ? ? ? ? ? ? ? ? ? ? ? ? tmpu32);
>>>+ ? ? ? ? ? ? ? ? ? ? ? ? ? (int)tmpu32);
>>
>> How did a 0x%08x make it into the source after the "2009/04/17 15:44:28"
>> change which changed all %p's to host_address_to_string? ?Shouldn't
>> this just be a call to host_address_to_string?
>>
>I am not sure %p or host_address_to_string is right.  It is not a
>address.  Just a simple value.

Perusing the gdb source code, I see a fair amount of precedent for using
host_address_to_string just to display 0x...

>>> ? ? ? ? return 1;
>>> ? ? ? }
>>> ? ? ? break;
>>>@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
>>> ? ? ? /* XXX */
>>> ? ? ? regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>>> ? ? sys_fcntl:
>>>- ? ? ?if (tmpu32 == F_GETLK)
>>>+ ? ? ?if (tmpu32 == tdep->fcntl_F_GETLK)
>>> ? ? ? {
>>> ? ? ? ? regcache_raw_read (regcache, tdep->arg3,
>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?(gdb_byte *) & tmpu32);
>>>@@ -626,7 +626,7 @@ record_linux_system_call (int num, struc
>>> ? ? ? ? ? ? ? ? ? "It will free the memory addr = 0x%s len = %d. ?"
>>> ? ? ? ? ? ? ? ? ? "It will make record target get error. ?"
>>> ? ? ? ? ? ? ? ? ? "Do you want to stop the program?"),
>>>- ? ? ? ? ? ? ? ?paddr_nz (tmpu32), len);
>>>+ ? ? ? ? ? ? ? ?paddr_nz (tmpu32), (int)len);
>>
>> If len is a uint32_t isn't casting it to an int the wrong thing to do?
>> Looking at the code, len is first defined as uint32_t, then a pointer to
>> it is cast as (gdb_byte *) (and it doesn't look like the space following
>> the '&' doesn't follow GNU coding standards). ?So it is never actually
>> used as a uint32_t. ?That doesn't seem right.
>>
>
>Use uint32_t because it's a 32 bits register's value.

So wouldn't that indicate that your format specifier is wrong and should
be "%u" rather than "%d"?  Coercion is something that should be avoided
unless it is absolutely necessary.

Also, grepping gdb source code, it seems like other places which use
this call use a real gdb_byte to catch it.  Either that or they call
regcache_raw_read_unsigned.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 4761 bytes --]

On Mon, May 11, 2009 at 07:51, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Mon, May 11, 2009 at 02:07:29AM +0800, Hui Zhu wrote:
>>On Mon, May 11, 2009 at 01:48, Christopher Faylor
>><cgf-use-the-mailinglist-please@sourceware.org> wrote:
>>> On Mon, May 11, 2009 at 01:31:18AM +0800, Hui Zhu wrote:
>>>>--- a/i386-linux-tdep.c
>>>>+++ b/i386-linux-tdep.c
>>>>@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
>>>> #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP ? ? ? ? 0x545F
>>>> #define I386_LINUX_RECORD_IOCTL_FIOQSIZE ? ? ? ? ? ? ?0x5460
>>>>
>>>>+/* The values of the second argument of system call "sys_fcntl"
>>>>+ ? and "sys_fcntl64". ?The values of these macros were obtained from
>>>>+ ? Linux Kernel source. ?*/
>>>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK ? ? ? ? ? ? ? ? ? ? ? 5
>>>>+#define I386_LINUX_RECORD_FCNTL_F_GETLK64 ? ? ? ? ? ? 12
>>>>+#define I386_LINUX_RECORD_FCNTL_F_SETLK64 ? ? ? ? ? ? 13
>>>>+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64 ? ? ? ? ? ?14
>>>>+
>>>> static void
>>>> i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
>>>> {
>>>>@@ -781,6 +789,12 @@ i386_linux_init_abi (struct gdbarch_info
>>>> ? ? I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
>>>> ? i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
>>>>
>>>>+ ?i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
>>>>+ ?i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
>>>>+ ?i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
>>>>+ ?i386_linux_record_tdep.fcntl_F_SETLKW64 =
>>>>+ ? ?I386_LINUX_RECORD_FCNTL_F_SETLKW64;
>>>>+
>>>> ? i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
>>>> ? i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
>>>> ? i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
>>>>--- a/linux-record.c
>>>>+++ b/linux-record.c
>>>>@@ -394,7 +394,7 @@ record_linux_system_call (int num, struc
>>>> ? ? ? {
>>>> ? ? ? ? printf_unfiltered (_("Process record and replay target doesn't "
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"support ioctl request 0x%08x.\n"),
>>>>- ? ? ? ? ? ? ? ? ? ? ? ? ? tmpu32);
>>>>+ ? ? ? ? ? ? ? ? ? ? ? ? ? (int)tmpu32);
>>>
>>> How did a 0x%08x make it into the source after the "2009/04/17 15:44:28"
>>> change which changed all %p's to host_address_to_string? ?Shouldn't
>>> this just be a call to host_address_to_string?
>>>
>>I am not sure %p or host_address_to_string is right.  It is not a
>>address.  Just a simple value.
>
> Perusing the gdb source code, I see a fair amount of precedent for using
> host_address_to_string just to display 0x...

const char *
host_address_to_string (const void *addr)
{
  char *str = get_cell ();

  xsnprintf (str, CELLSIZE, "0x%s", phex_nz ((uintptr_t) addr, sizeof (addr)));
  return str;
}
And code in linux-record.c is for arch target.  So current address
size is not same with 32 bits sometime.
So I use phex_nz directly.

>
>>>> ? ? ? ? return 1;
>>>> ? ? ? }
>>>> ? ? ? break;
>>>>@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
>>>> ? ? ? /* XXX */
>>>> ? ? ? regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
>>>> ? ? sys_fcntl:
>>>>- ? ? ?if (tmpu32 == F_GETLK)
>>>>+ ? ? ?if (tmpu32 == tdep->fcntl_F_GETLK)
>>>> ? ? ? {
>>>> ? ? ? ? regcache_raw_read (regcache, tdep->arg3,
>>>> ? ? ? ? ? ? ? ? ? ? ? ? ? ?(gdb_byte *) & tmpu32);
>>>>@@ -626,7 +626,7 @@ record_linux_system_call (int num, struc
>>>> ? ? ? ? ? ? ? ? ? "It will free the memory addr = 0x%s len = %d. ?"
>>>> ? ? ? ? ? ? ? ? ? "It will make record target get error. ?"
>>>> ? ? ? ? ? ? ? ? ? "Do you want to stop the program?"),
>>>>- ? ? ? ? ? ? ? ?paddr_nz (tmpu32), len);
>>>>+ ? ? ? ? ? ? ? ?paddr_nz (tmpu32), (int)len);
>>>
>>> If len is a uint32_t isn't casting it to an int the wrong thing to do?
>>> Looking at the code, len is first defined as uint32_t, then a pointer to
>>> it is cast as (gdb_byte *) (and it doesn't look like the space following
>>> the '&' doesn't follow GNU coding standards). ?So it is never actually
>>> used as a uint32_t. ?That doesn't seem right.
>>>
>>
>>Use uint32_t because it's a 32 bits register's value.
>
> So wouldn't that indicate that your format specifier is wrong and should
> be "%u" rather than "%d"?  Coercion is something that should be avoided
> unless it is absolutely necessary.
>

I changed it.

> Also, grepping gdb source code, it seems like other places which use
> this call use a real gdb_byte to catch it.  Either that or they call
> regcache_raw_read_unsigned.

It will changed to gdb_byte in linux-record.c  64 bits support that I
am working on.

Thanks,
Hui

[-- Attachment #2: fix-prec-cygwin-build-error.txt --]
[-- Type: text/plain, Size: 9058 bytes --]

---
 i386-linux-tdep.c |   14 +++++++++++
 linux-record.c    |   64 ++++++++++++++++++++++++++----------------------------
 linux-record.h    |    7 +++++
 3 files changed, 52 insertions(+), 33 deletions(-)

--- a/i386-linux-tdep.c
+++ b/i386-linux-tdep.c
@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
 #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP		0x545F
 #define I386_LINUX_RECORD_IOCTL_FIOQSIZE		0x5460
 
+/* The values of the second argument of system call "sys_fcntl"
+   and "sys_fcntl64".  The values of these macros were obtained from
+   Linux Kernel source.  */
+#define I386_LINUX_RECORD_FCNTL_F_GETLK			5
+#define I386_LINUX_RECORD_FCNTL_F_GETLK64		12
+#define I386_LINUX_RECORD_FCNTL_F_SETLK64		13
+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64		14
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -783,6 +791,12 @@ i386_linux_init_abi (struct gdbarch_info
     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
 
+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
+
   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
--- a/linux-record.c
+++ b/linux-record.c
@@ -393,8 +393,8 @@ record_linux_system_call (int num, struc
       else
 	{
 	  printf_unfiltered (_("Process record and replay target doesn't "
-			       "support ioctl request 0x%08x.\n"),
-			     tmpu32);
+			       "support ioctl request 0x%s.\n"),
+			     phex_nz (tmpu32, 4));
 	  return 1;
 	}
       break;
@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
       /* XXX */
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
     sys_fcntl:
-      if (tmpu32 == F_GETLK)
+      if (tmpu32 == tdep->fcntl_F_GETLK)
 	{
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
@@ -546,7 +546,7 @@ record_linux_system_call (int num, struc
 		if (record_debug)
 		  fprintf_unfiltered (gdb_stdlog,
 				      "Process record: error reading memory "
-				      "at addr = 0x%s len = %d.\n",
+				      "at addr = 0x%s len = %u.\n",
 				      paddr_nz (tmpu32),
 				      (int) sizeof (sel));
 		return -1;
@@ -623,10 +623,10 @@ record_linux_system_call (int num, struc
 	target_terminal_ours ();
 	q =
 	  yquery (_("The next instruction is syscall munmap.  "
-		    "It will free the memory addr = 0x%s len = %d.  "
+		    "It will free the memory addr = 0x%s len = %u.  "
 		    "It will make record target get error.  "
 		    "Do you want to stop the program?"),
-		  paddr_nz (tmpu32), len);
+		  paddr_nz (tmpu32), (int)len);
 	target_terminal_inferior ();
 	if (q)
 	  return 1;
@@ -686,7 +686,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 					  (int) sizeof (a));
 		    return -1;
@@ -711,7 +711,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 				          (int) sizeof (a));
 		    return -1;
@@ -736,7 +736,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 					  (int) sizeof (a));
 		    return -1;
@@ -750,7 +750,7 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %u.\n",
 					      paddr_nz (a[2]),
 					      (int) sizeof (a[2]));
 			return -1;
@@ -773,7 +773,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 					  (int) sizeof (a));
 		    return -1;
@@ -787,7 +787,7 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %u.\n",
 					      paddr_nz (a[2]),
 					      (int) sizeof (a[2]));
 			return -1;
@@ -819,7 +819,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 					  (int) sizeof (a));
 		    return -1;
@@ -833,7 +833,7 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %u.\n",
 					      paddr_nz (a[4]),
 					      (int) sizeof (av));
 			return -1;
@@ -876,7 +876,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (tmpu32),
 					  (int) sizeof (a));
 		    return -1;
@@ -892,7 +892,7 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %u.\n",
 					      paddr_nz (a[1]),
 					      (int) sizeof (rec));
 			return -1;
@@ -916,7 +916,7 @@ record_linux_system_call (int num, struc
 						      "Process record: error "
 						      "reading memory at "
 						      "addr = 0x%s "
-						      "len = %d.\n",
+						      "len = %u.\n",
 						      paddr_nz (rec.
 								msg_iov),
 						      (int) sizeof (iov));
@@ -934,8 +934,8 @@ record_linux_system_call (int num, struc
 	  break;
 	default:
 	  printf_unfiltered (_("Process record and replay target "
-			       "doesn't support socketcall call 0x%08x\n"),
-			     tmpu32);
+			       "doesn't support socketcall call 0x%s\n"),
+			     phex_nz (tmpu32, 4));
 	  return -1;
 	  break;
 	}
@@ -1233,7 +1233,7 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %u.\n",
 					  paddr_nz (vec),
 					  (int) sizeof (struct record_iovec));
 		    return -1;
@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
       /* sys_fcntl64 */
     case 221:
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
-      switch (tmpu32)
-	{
-	case F_GETLK64:
+      if (tmpu32 == tdep->fcntl_F_GETLK64)
+        {
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
 	  if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
 	    return -1;
-	  break;
-	case F_SETLK64:
-	case F_SETLKW64:
-	  break;
-	default:
+	}
+      else if (tmpu32 != tdep->fcntl_F_SETLK64
+	       && tmpu32 != tdep->fcntl_F_SETLKW64)
+        {
 	  goto sys_fcntl;
-	  break;
 	}
       break;
 
@@ -1785,8 +1782,9 @@ record_linux_system_call (int num, struc
 	      if (record_debug)
 		fprintf_unfiltered (gdb_stdlog,
 				    "Process record: error reading memory "
-				    "at addr = 0x%s len = %d.\n",
-				    paddr_nz (tmpu32), nr * tdep->size_int);
+				    "at addr = 0x%s len = %u.\n",
+				    paddr_nz (tmpu32),
+				    (int)(nr * tdep->size_int));
 	      return -1;
 	    }
 	  for (i = 0; i < nr; i++)
@@ -2195,8 +2193,8 @@ record_linux_system_call (int num, struc
 
     default:
       printf_unfiltered (_("Process record and replay target doesn't "
-			   "support syscall number 0x%08x\n"),
-			 tmpu32);
+			   "support syscall number %u\n"),
+			 (int)tmpu32);
       return -1;
       break;
     }
--- a/linux-record.h
+++ b/linux-record.h
@@ -158,6 +158,13 @@ struct linux_record_tdep
   int ioctl_TIOCSHAYESESP;
   int ioctl_FIOQSIZE;
 
+  /* The values of the second argument of system call "sys_fcntl"
+     and "sys_fcntl64".  */
+  int fcntl_F_GETLK;
+  int fcntl_F_GETLK64;
+  int fcntl_F_SETLK64;
+  int fcntl_F_SETLKW64;
+
   /* The number of the registers that are used as the arguments of
      a system call.  */
   int arg1;

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Mon, May 18, 2009 at 04:43:36PM +0800, Hui Zhu wrote:
>On Mon, May 11, 2009 at 07:51, Christopher Faylor wrote:
>> So wouldn't that indicate that your format specifier is wrong and should
>> be "%u" rather than "%d"? ?Coercion is something that should be avoided
>> unless it is absolutely necessary.
>>
>
>I changed it.

It looks like this diff is against your previous version rather than CVS
so it is confusing.  But, it still seems like you're still coercing
using (int)len but now you're using %u.  So, it looks like you've just
swapped the problem.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

On Mon, May 18, 2009 at 23:35, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Mon, May 18, 2009 at 04:43:36PM +0800, Hui Zhu wrote:
>>On Mon, May 11, 2009 at 07:51, Christopher Faylor wrote:
>>> So wouldn't that indicate that your format specifier is wrong and should
>>> be "%u" rather than "%d"? ?Coercion is something that should be avoided
>>> unless it is absolutely necessary.
>>>
>>
>>I changed it.
>
> It looks like this diff is against your previous version rather than CVS
> so it is confusing.  But, it still seems like you're still coercing
> using (int)len but now you're using %u.  So, it looks like you've just
> swapped the problem.

No, it based on cvs-head. Are you really try?

With out (int) it will get:
../../src/gdb/linux-record.c: In function `record_linux_system_call':
../../src/gdb/linux-record.c:1787: warning: unsigned int format, long
int arg (arg 4)
make: *** [linux-record.o] Error 1

In Cygwin.

Thanks,
Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

Ping

On Tue, May 19, 2009 at 10:34, Hui Zhu <teawater@gmail.com> wrote:
>
> On Mon, May 18, 2009 at 23:35, Christopher Faylor
> <cgf-use-the-mailinglist-please@sourceware.org> wrote:
> > On Mon, May 18, 2009 at 04:43:36PM +0800, Hui Zhu wrote:
> >>On Mon, May 11, 2009 at 07:51, Christopher Faylor wrote:
> >>> So wouldn't that indicate that your format specifier is wrong and should
> >>> be "%u" rather than "%d"? ?Coercion is something that should be avoided
> >>> unless it is absolutely necessary.
> >>>
> >>
> >>I changed it.
> >
> > It looks like this diff is against your previous version rather than CVS
> > so it is confusing.  But, it still seems like you're still coercing
> > using (int)len but now you're using %u.  So, it looks like you've just
> > swapped the problem.
>
> No, it based on cvs-head. Are you really try?
>
> With out (int) it will get:
> ../../src/gdb/linux-record.c: In function `record_linux_system_call':
> ../../src/gdb/linux-record.c:1787: warning: unsigned int format, long
> int arg (arg 4)
> make: *** [linux-record.o] Error 1
>
> In Cygwin.
>
> Thanks,
> Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Thu, May 21, 2009 at 10:38:58AM +0800, Hui Zhu wrote:
>Ping

You are aware that it took you about seven days to respond to one of my
messages right?

I was confused by the fact that you didn't seem to be using CVS to
generate your diffs and, on closer inspection, I see that there are
multiple places where coercion is used inappropriately when the format
specifier should just have matched the parameter.

Some of your changes are now using "%u" with "(int) sizeof" (which is
wrong) and some of the existing code is just using "(int) sizeof" with
"%d" which is also wrong.

So, in other words, the file was already not quite "right" and I don't
think your changes are getting it much closer to "right".

I was not implying that your changes didn't fix compiler errors.  I was
saying that I didn't think they were the correct way to fix the errors.
However, since I am apparently the only person who cares about this
stuff, and it doesn't seem like my point is getting across, I think I'll
just shut up now.  Other, global maintainers have said that they were ok
with your changes so I'm not going to object even if they don't make
sense to me.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Pedro Alves]

On Thursday 21 May 2009 16:26:21, Christopher Faylor wrote:
> I was not implying that your changes didn't fix compiler errors.  I was
> saying that I didn't think they were the correct way to fix the errors.
> However, since I am apparently the only person who cares about this
> stuff, and it doesn't seem like my point is getting across, I think I'll
> just shut up now.  Other, global maintainers have said that they were ok
> with your changes so I'm not going to object even if they don't make
> sense to me.

If you're thinking of me, I don't remember saying this was OK.  I'd like
to have this fixed properly.

-- 
Pedro Alves

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

Hi Christopher,

On Thu, May 21, 2009 at 23:26, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Thu, May 21, 2009 at 10:38:58AM +0800, Hui Zhu wrote:
>>Ping
>
> You are aware that it took you about seven days to respond to one of my
> messages right?
>

Sorry for that. To get a cygwin environment is very hard thing for a
linux guy.  And I found that cygwin cannot work with wine.

> I was confused by the fact that you didn't seem to be using CVS to
> generate your diffs and, on closer inspection, I see that there are
> multiple places where coercion is used inappropriately when the format
> specifier should just have matched the parameter.

Sorry for my poor english.  I got patch with quilt.

>
> Some of your changes are now using "%u" with "(int) sizeof" (which is
> wrong) and some of the existing code is just using "(int) sizeof" with
> "%d" which is also wrong.
>
> So, in other words, the file was already not quite "right" and I don't
> think your changes are getting it much closer to "right".
>

Thanks for you help me review my patch.  Could you give me some help with it?
Actually, try something with cygwin still a very hard thing for me.  I
just can use it in a notebook of other guy.  And I had return it.
Maybe you can make a patch for this bug.  That will be very great for me.

> I was not implying that your changes didn't fix compiler errors.  I was
> saying that I didn't think they were the correct way to fix the errors.
> However, since I am apparently the only person who cares about this
> stuff, and it doesn't seem like my point is getting across, I think I'll
> just shut up now.  Other, global maintainers have said that they were ok
> with your changes so I'm not going to object even if they don't make
> sense to me.

Please do not misunderstand us.  I just get a mail from Pierre who
report this bug.  And this mail just say this patch test with cygwin
is OK.  Because I let Pierre help me test the patch with cygwin.  That
is all.


Thanks,
Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Thu, May 21, 2009 at 04:34:55PM +0100, Pedro Alves wrote:
>On Thursday 21 May 2009 16:26:21, Christopher Faylor wrote:
>>I was not implying that your changes didn't fix compiler errors.  ?I
>>was saying that I didn't think they were the correct way to fix the
>>errors.  However, since I am apparently the only person who cares about
>>this stuff, and it doesn't seem like my point is getting across, I
>>think I'll just shut up now.  ?Other, global maintainers have said that
>>they were ok with your changes so I'm not going to object even if they
>>don't make sense to me.
>
>If you're thinking of me, I don't remember saying this was OK.  I'd
>like to have this fixed properly.

Sorry, I didn't really have anyone in mind.  I thought a few people had
indicated they were ok with the patch.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>Thanks for you help me review my patch.  Could you give me some help
>with it?  Actually, try something with cygwin still a very hard thing
>for me.  I just can use it in a notebook of other guy.  And I had
>return it.  Maybe you can make a patch for this bug.  That will be very
>great for me.

I don't see why this has anything to do with Cygwin.  We're talking about
general principles here.  If you know C, which you obviously do, then
I don't really understand why this needs a lot of explanation.  Look at the
places where you're using coercion with a format string and just use the
right format specifier instead.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

On Fri, May 22, 2009 at 01:38, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>>Thanks for you help me review my patch.  Could you give me some help
>>with it?  Actually, try something with cygwin still a very hard thing
>>for me.  I just can use it in a notebook of other guy.  And I had
>>return it.  Maybe you can make a patch for this bug.  That will be very
>>great for me.
>
> I don't see why this has anything to do with Cygwin.  We're talking about
> general principles here.  If you know C, which you obviously do, then
> I don't really understand why this needs a lot of explanation.  Look at the
> places where you're using coercion with a format string and just use the
> right format specifier instead.

Because the patch must be work OK with cygwin.  Even if for the
format.  I think you must have a lot of good idea with it.
We need your help.  That is very great for me.  Thanks a lot.

Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Fri, May 22, 2009 at 07:14:48AM +0800, Hui Zhu wrote:
>On Fri, May 22, 2009 at 01:38, Christopher Faylor  wrote:
>> On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>>>Thanks for you help me review my patch. ?Could you give me some help
>>>with it? ?Actually, try something with cygwin still a very hard thing
>>>for me. ?I just can use it in a notebook of other guy. ?And I had
>>>return it. ?Maybe you can make a patch for this bug. ?That will be very
>>>great for me.
>>
>>I don't see why this has anything to do with Cygwin.  ?We're talking
>>about general principles here.  If you know C, which you obviously do,
>>then I don't really understand why this needs a lot of explanation.
>>Look at the places where you're using coercion with a format string and
>>just use the right format specifier instead.
>
>Because the patch must be work OK with cygwin.  Even if for the format.
>I think you must have a lot of good idea with it.  We need your help.
>That is very great for me.  Thanks a lot.

No need to thank me yet.  I'll be happy to review your change when you
make the simple changes that I requested.  You can hold off thanking me
until then.

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

On Fri, May 22, 2009 at 08:46, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Fri, May 22, 2009 at 07:14:48AM +0800, Hui Zhu wrote:
>>On Fri, May 22, 2009 at 01:38, Christopher Faylor  wrote:
>>> On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>>>>Thanks for you help me review my patch. ?Could you give me some help
>>>>with it? ?Actually, try something with cygwin still a very hard thing
>>>>for me. ?I just can use it in a notebook of other guy. ?And I had
>>>>return it. ?Maybe you can make a patch for this bug. ?That will be very
>>>>great for me.
>>>
>>>I don't see why this has anything to do with Cygwin.  ?We're talking
>>>about general principles here.  If you know C, which you obviously do,
>>>then I don't really understand why this needs a lot of explanation.
>>>Look at the places where you're using coercion with a format string and
>>>just use the right format specifier instead.
>>
>>Because the patch must be work OK with cygwin.  Even if for the format.
>>I think you must have a lot of good idea with it.  We need your help.
>>That is very great for me.  Thanks a lot.
>
> No need to thank me yet.  I'll be happy to review your change when you
> make the simple changes that I requested.  You can hold off thanking me
> until then.

About %u and sizeof, if without (int). It's OK with cygwin.  But it
cannot work OK with amd64.
It will get:
../../src/gdb/linux-record.c: In function ‘record_linux_system_call’:
../../src/gdb/linux-record.c:551: warning: format ‘%u’ expects type
‘unsigned int’, but argument 4 has type ‘long unsigned int’
Could you give me some help with it?

Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Fri, May 22, 2009 at 10:12:01AM +0800, Hui Zhu wrote:
>On Fri, May 22, 2009 at 08:46, Christopher Faylor
><cgf-use-the-mailinglist-please@sourceware.org> wrote:
>> On Fri, May 22, 2009 at 07:14:48AM +0800, Hui Zhu wrote:
>>>On Fri, May 22, 2009 at 01:38, Christopher Faylor ?wrote:
>>>> On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>>>>>Thanks for you help me review my patch. ?Could you give me some help
>>>>>with it? ?Actually, try something with cygwin still a very hard thing
>>>>>for me. ?I just can use it in a notebook of other guy. ?And I had
>>>>>return it. ?Maybe you can make a patch for this bug. ?That will be very
>>>>>great for me.
>>>>
>>>>I don't see why this has anything to do with Cygwin. ??We're talking
>>>>about general principles here. ?If you know C, which you obviously do,
>>>>then I don't really understand why this needs a lot of explanation.
>>>>Look at the places where you're using coercion with a format string and
>>>>just use the right format specifier instead.
>>>
>>>Because the patch must be work OK with cygwin. ?Even if for the format.
>>>I think you must have a lot of good idea with it. ?We need your help.
>>>That is very great for me. ?Thanks a lot.
>>
>> No need to thank me yet. ?I'll be happy to review your change when you
>> make the simple changes that I requested. ?You can hold off thanking me
>> until then.
>
>About %u and sizeof, if without (int). It's OK with cygwin.  But it
>cannot work OK with amd64.
>It will get:
>../../src/gdb/linux-record.c: In function ?record_linux_system_call?:
>../../src/gdb/linux-record.c:551: warning: format ?%u? expects type
>?unsigned int?, but argument 4 has type ?long unsigned int?
>Could you give me some help with it?

"%zu"

cgf

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 1969 bytes --]

On Fri, May 22, 2009 at 12:13, Christopher Faylor
<cgf-use-the-mailinglist-please@sourceware.org> wrote:
> On Fri, May 22, 2009 at 10:12:01AM +0800, Hui Zhu wrote:
>>On Fri, May 22, 2009 at 08:46, Christopher Faylor
>><cgf-use-the-mailinglist-please@sourceware.org> wrote:
>>> On Fri, May 22, 2009 at 07:14:48AM +0800, Hui Zhu wrote:
>>>>On Fri, May 22, 2009 at 01:38, Christopher Faylor ?wrote:
>>>>> On Fri, May 22, 2009 at 12:43:07AM +0800, Hui Zhu wrote:
>>>>>>Thanks for you help me review my patch. ?Could you give me some help
>>>>>>with it? ?Actually, try something with cygwin still a very hard thing
>>>>>>for me. ?I just can use it in a notebook of other guy. ?And I had
>>>>>>return it. ?Maybe you can make a patch for this bug. ?That will be very
>>>>>>great for me.
>>>>>
>>>>>I don't see why this has anything to do with Cygwin. ??We're talking
>>>>>about general principles here. ?If you know C, which you obviously do,
>>>>>then I don't really understand why this needs a lot of explanation.
>>>>>Look at the places where you're using coercion with a format string and
>>>>>just use the right format specifier instead.
>>>>
>>>>Because the patch must be work OK with cygwin. ?Even if for the format.
>>>>I think you must have a lot of good idea with it. ?We need your help.
>>>>That is very great for me. ?Thanks a lot.
>>>
>>> No need to thank me yet. ?I'll be happy to review your change when you
>>> make the simple changes that I requested. ?You can hold off thanking me
>>> until then.
>>
>>About %u and sizeof, if without (int). It's OK with cygwin.  But it
>>cannot work OK with amd64.
>>It will get:
>>../../src/gdb/linux-record.c: In function ?record_linux_system_call?:
>>../../src/gdb/linux-record.c:551: warning: format ?%u? expects type
>>?unsigned int?, but argument 4 has type ?long unsigned int?
>>Could you give me some help with it?
>
> "%zu"
>
Please help me review this patch.

Thanks,
Hui

[-- Attachment #2: fix-prec-cygwin-build-error.txt --]
[-- Type: text/plain, Size: 10637 bytes --]

Index: i386-linux-tdep.c
===================================================================
RCS file: /cvs/src/src/gdb/i386-linux-tdep.c,v
retrieving revision 1.62
diff -p -u -r1.62 i386-linux-tdep.c
--- i386-linux-tdep.c	17 May 2009 12:26:44 -0000	1.62
+++ i386-linux-tdep.c	22 May 2009 05:51:41 -0000
@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
 #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP		0x545F
 #define I386_LINUX_RECORD_IOCTL_FIOQSIZE		0x5460
 
+/* The values of the second argument of system call "sys_fcntl"
+   and "sys_fcntl64".  The values of these macros were obtained from
+   Linux Kernel source.  */
+#define I386_LINUX_RECORD_FCNTL_F_GETLK			5
+#define I386_LINUX_RECORD_FCNTL_F_GETLK64		12
+#define I386_LINUX_RECORD_FCNTL_F_SETLK64		13
+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64		14
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -783,6 +791,12 @@ i386_linux_init_abi (struct gdbarch_info
     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
 
+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
+
   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
Index: linux-record.c
===================================================================
RCS file: /cvs/src/src/gdb/linux-record.c,v
retrieving revision 1.2
diff -p -u -r1.2 linux-record.c
--- linux-record.c	30 Apr 2009 03:04:28 -0000	1.2
+++ linux-record.c	22 May 2009 05:51:42 -0000
@@ -393,8 +393,8 @@ record_linux_system_call (int num, struc
       else
 	{
 	  printf_unfiltered (_("Process record and replay target doesn't "
-			       "support ioctl request 0x%08x.\n"),
-			     tmpu32);
+			       "support ioctl request 0x%s.\n"),
+			     phex_nz (tmpu32, 4));
 	  return 1;
 	}
       break;
@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
       /* XXX */
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
     sys_fcntl:
-      if (tmpu32 == F_GETLK)
+      if (tmpu32 == tdep->fcntl_F_GETLK)
 	{
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
@@ -546,9 +546,9 @@ record_linux_system_call (int num, struc
 		if (record_debug)
 		  fprintf_unfiltered (gdb_stdlog,
 				      "Process record: error reading memory "
-				      "at addr = 0x%s len = %d.\n",
+				      "at addr = 0x%s len = %zu.\n",
 				      paddr_nz (tmpu32),
-				      (int) sizeof (sel));
+				      sizeof (sel));
 		return -1;
 	      }
 	    if (record_arch_list_add_mem (sel.inp, tdep->size_fd_set))
@@ -623,10 +623,10 @@ record_linux_system_call (int num, struc
 	target_terminal_ours ();
 	q =
 	  yquery (_("The next instruction is syscall munmap.  "
-		    "It will free the memory addr = 0x%s len = %d.  "
+		    "It will free the memory addr = 0x%s len = %u.  "
 		    "It will make record target get error.  "
 		    "Do you want to stop the program?"),
-		  paddr_nz (tmpu32), len);
+		  paddr_nz (tmpu32), (int)len);
 	target_terminal_inferior ();
 	if (q)
 	  return 1;
@@ -686,9 +686,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[1], tdep->size_sockaddr))
@@ -711,9 +711,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-				          (int) sizeof (a));
+				          sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[3], tdep->size_int))
@@ -736,9 +736,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  sizeof (a));
 		    return -1;
 		  }
 		if (a[2])
@@ -750,9 +750,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %zu.\n",
 					      paddr_nz (a[2]),
-					      (int) sizeof (a[2]));
+					      sizeof (a[2]));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[1], a[2]))
@@ -773,9 +773,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  sizeof (a));
 		    return -1;
 		  }
 		if (a[2])
@@ -787,9 +787,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %zu.\n",
 					      paddr_nz (a[2]),
-					      (int) sizeof (a[2]));
+					      sizeof (a[2]));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[1], a[2]))
@@ -819,9 +819,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  sizeof (a));
 		    return -1;
 		  }
 		if (a[4])
@@ -833,9 +833,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %zu.\n",
 					      paddr_nz (a[4]),
-					      (int) sizeof (av));
+					      sizeof (av));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[3], av))
@@ -876,9 +876,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[1], tdep->size_msghdr))
@@ -892,9 +892,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %zu.\n",
 					      paddr_nz (a[1]),
-					      (int) sizeof (rec));
+					      sizeof (rec));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem
@@ -916,10 +916,10 @@ record_linux_system_call (int num, struc
 						      "Process record: error "
 						      "reading memory at "
 						      "addr = 0x%s "
-						      "len = %d.\n",
+						      "len = %zu.\n",
 						      paddr_nz (rec.
 								msg_iov),
-						      (int) sizeof (iov));
+						      sizeof (iov));
 				return -1;
 			      }
 			    if (record_arch_list_add_mem
@@ -934,8 +934,8 @@ record_linux_system_call (int num, struc
 	  break;
 	default:
 	  printf_unfiltered (_("Process record and replay target "
-			       "doesn't support socketcall call 0x%08x\n"),
-			     tmpu32);
+			       "doesn't support socketcall call 0x%s\n"),
+			     phex_nz (tmpu32, 4));
 	  return -1;
 	  break;
 	}
@@ -1233,9 +1233,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %zu.\n",
 					  paddr_nz (vec),
-					  (int) sizeof (struct record_iovec));
+					  sizeof (struct record_iovec));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (iov.iov_base, iov.iov_len))
@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
       /* sys_fcntl64 */
     case 221:
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
-      switch (tmpu32)
-	{
-	case F_GETLK64:
+      if (tmpu32 == tdep->fcntl_F_GETLK64)
+        {
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
 	  if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
 	    return -1;
-	  break;
-	case F_SETLK64:
-	case F_SETLKW64:
-	  break;
-	default:
+	}
+      else if (tmpu32 != tdep->fcntl_F_SETLK64
+	       && tmpu32 != tdep->fcntl_F_SETLKW64)
+        {
 	  goto sys_fcntl;
-	  break;
 	}
       break;
 
@@ -1785,8 +1782,9 @@ record_linux_system_call (int num, struc
 	      if (record_debug)
 		fprintf_unfiltered (gdb_stdlog,
 				    "Process record: error reading memory "
-				    "at addr = 0x%s len = %d.\n",
-				    paddr_nz (tmpu32), nr * tdep->size_int);
+				    "at addr = 0x%s len = %u.\n",
+				    paddr_nz (tmpu32),
+				    (int)(nr * tdep->size_int));
 	      return -1;
 	    }
 	  for (i = 0; i < nr; i++)
@@ -2195,8 +2193,8 @@ record_linux_system_call (int num, struc
 
     default:
       printf_unfiltered (_("Process record and replay target doesn't "
-			   "support syscall number 0x%08x\n"),
-			 tmpu32);
+			   "support syscall number %u\n"),
+			 (int)tmpu32);
       return -1;
       break;
     }
Index: linux-record.h
===================================================================
RCS file: /cvs/src/src/gdb/linux-record.h,v
retrieving revision 1.2
diff -p -u -r1.2 linux-record.h
--- linux-record.h	30 Apr 2009 03:04:28 -0000	1.2
+++ linux-record.h	22 May 2009 05:51:42 -0000
@@ -158,6 +158,13 @@ struct linux_record_tdep
   int ioctl_TIOCSHAYESESP;
   int ioctl_FIOQSIZE;
 
+  /* The values of the second argument of system call "sys_fcntl"
+     and "sys_fcntl64".  */
+  int fcntl_F_GETLK;
+  int fcntl_F_GETLK64;
+  int fcntl_F_SETLK64;
+  int fcntl_F_SETLKW64;
+
   /* The number of the registers that are used as the arguments of
      a system call.  */
   int arg1;

Re: [Prec/RFA] fix build error of prec in cygwin

[Joel Brobecker]

> "%zu"

Are we allowed to use %z? (I thought that %z is C99, but I don't have
a C90 manual to double-check)

-- 
Joel

Re: [Prec/RFA] fix build error of prec in cygwin

[Eli Zaretskii]

> Date: Thu, 21 May 2009 23:05:45 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > "%zu"
> 
> Are we allowed to use %z?

No, I don't think so.

> I thought that %z is C99

It is.

I think the best we can do with C90 is use %lu and cast to
`unsigned long'.

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

I tried with gcc.

cat 1.c
#include <sys/types.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <stdint.h>

int
main(int argc,char *argv[],char *envp[])
{
	printf ("%uz", sizeof (int));

	return (0);
}

gcc -std=c89 -Wall -pedantic 1.c

No warning.

Thanks,
Hui

On Fri, May 22, 2009 at 14:55, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 21 May 2009 23:05:45 -0700
>> From: Joel Brobecker <brobecker@adacore.com>
>>
>> > "%zu"
>>
>> Are we allowed to use %z?
>
> No, I don't think so.
>
>> I thought that %z is C99
>
> It is.
>
> I think the best we can do with C90 is use %lu and cast to
> `unsigned long'.
>

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

[-- Attachment #1: Type: text/plain, Size: 448 bytes --]

Hi Eli,

On Fri, May 22, 2009 at 14:55, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Thu, 21 May 2009 23:05:45 -0700
>> From: Joel Brobecker <brobecker@adacore.com>
>>
>> > "%zu"
>>
>> Are we allowed to use %z?
>
> No, I don't think so.
>
>> I thought that %z is C99
>
> It is.
>
> I think the best we can do with C90 is use %lu and cast to
> `unsigned long'.
>
I make a new patch use %lu and unsigned long.  Please help me review it.

Thanks,
hui

[-- Attachment #2: fix-prec-cygwin-build-error.txt --]
[-- Type: text/plain, Size: 10292 bytes --]

---
 i386-linux-tdep.c |   14 ++++++++
 linux-record.c    |   90 ++++++++++++++++++++++++++----------------------------
 linux-record.h    |    7 ++++
 3 files changed, 65 insertions(+), 46 deletions(-)

--- a/i386-linux-tdep.c
+++ b/i386-linux-tdep.c
@@ -586,6 +586,14 @@ static int i386_linux_sc_reg_offset[] =
 #define I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP		0x545F
 #define I386_LINUX_RECORD_IOCTL_FIOQSIZE		0x5460
 
+/* The values of the second argument of system call "sys_fcntl"
+   and "sys_fcntl64".  The values of these macros were obtained from
+   Linux Kernel source.  */
+#define I386_LINUX_RECORD_FCNTL_F_GETLK			5
+#define I386_LINUX_RECORD_FCNTL_F_GETLK64		12
+#define I386_LINUX_RECORD_FCNTL_F_SETLK64		13
+#define I386_LINUX_RECORD_FCNTL_F_SETLKW64		14
+
 static void
 i386_linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
 {
@@ -783,6 +791,12 @@ i386_linux_init_abi (struct gdbarch_info
     I386_LINUX_RECORD_IOCTL_TIOCSHAYESESP;
   i386_linux_record_tdep.ioctl_FIOQSIZE = I386_LINUX_RECORD_IOCTL_FIOQSIZE;
 
+  i386_linux_record_tdep.fcntl_F_GETLK = I386_LINUX_RECORD_FCNTL_F_GETLK;
+  i386_linux_record_tdep.fcntl_F_GETLK64 = I386_LINUX_RECORD_FCNTL_F_GETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLK64 = I386_LINUX_RECORD_FCNTL_F_SETLK64;
+  i386_linux_record_tdep.fcntl_F_SETLKW64 =
+    I386_LINUX_RECORD_FCNTL_F_SETLKW64;
+
   i386_linux_record_tdep.arg1 = I386_EBX_REGNUM;
   i386_linux_record_tdep.arg2 = I386_ECX_REGNUM;
   i386_linux_record_tdep.arg3 = I386_EDX_REGNUM;
--- a/linux-record.c
+++ b/linux-record.c
@@ -393,8 +393,8 @@ record_linux_system_call (int num, struc
       else
 	{
 	  printf_unfiltered (_("Process record and replay target doesn't "
-			       "support ioctl request 0x%08x.\n"),
-			     tmpu32);
+			       "support ioctl request 0x%s.\n"),
+			     phex_nz (tmpu32, 4));
 	  return 1;
 	}
       break;
@@ -404,7 +404,7 @@ record_linux_system_call (int num, struc
       /* XXX */
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
     sys_fcntl:
-      if (tmpu32 == F_GETLK)
+      if (tmpu32 == tdep->fcntl_F_GETLK)
 	{
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
@@ -546,9 +546,9 @@ record_linux_system_call (int num, struc
 		if (record_debug)
 		  fprintf_unfiltered (gdb_stdlog,
 				      "Process record: error reading memory "
-				      "at addr = 0x%s len = %d.\n",
+				      "at addr = 0x%s len = %lu.\n",
 				      paddr_nz (tmpu32),
-				      (int) sizeof (sel));
+				      (unsigned long)sizeof (sel));
 		return -1;
 	      }
 	    if (record_arch_list_add_mem (sel.inp, tdep->size_fd_set))
@@ -623,10 +623,10 @@ record_linux_system_call (int num, struc
 	target_terminal_ours ();
 	q =
 	  yquery (_("The next instruction is syscall munmap.  "
-		    "It will free the memory addr = 0x%s len = %d.  "
+		    "It will free the memory addr = 0x%s len = %u.  "
 		    "It will make record target get error.  "
 		    "Do you want to stop the program?"),
-		  paddr_nz (tmpu32), len);
+		  paddr_nz (tmpu32), (int)len);
 	target_terminal_inferior ();
 	if (q)
 	  return 1;
@@ -686,9 +686,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[1], tdep->size_sockaddr))
@@ -711,9 +711,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-				          (int) sizeof (a));
+				          (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[3], tdep->size_int))
@@ -736,9 +736,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (a[2])
@@ -750,9 +750,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %lu.\n",
 					      paddr_nz (a[2]),
-					      (int) sizeof (a[2]));
+					      (unsigned long)sizeof (a[2]));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[1], a[2]))
@@ -773,9 +773,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (a[2])
@@ -787,9 +787,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %lu.\n",
 					      paddr_nz (a[2]),
-					      (int) sizeof (a[2]));
+					      (unsigned long)sizeof (a[2]));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[1], a[2]))
@@ -819,9 +819,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (a[4])
@@ -833,9 +833,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %lu.\n",
 					      paddr_nz (a[4]),
-					      (int) sizeof (av));
+					      (unsigned long)sizeof (av));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem (a[3], av))
@@ -876,9 +876,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (tmpu32),
-					  (int) sizeof (a));
+					  (unsigned long)sizeof (a));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (a[1], tdep->size_msghdr))
@@ -892,9 +892,9 @@ record_linux_system_call (int num, struc
 			  fprintf_unfiltered (gdb_stdlog,
 					      "Process record: error reading "
 					      "memory at addr = 0x%s "
-					      "len = %d.\n",
+					      "len = %lu.\n",
 					      paddr_nz (a[1]),
-					      (int) sizeof (rec));
+					      (unsigned long)sizeof (rec));
 			return -1;
 		      }
 		    if (record_arch_list_add_mem
@@ -916,10 +916,10 @@ record_linux_system_call (int num, struc
 						      "Process record: error "
 						      "reading memory at "
 						      "addr = 0x%s "
-						      "len = %d.\n",
+						      "len = %lu.\n",
 						      paddr_nz (rec.
 								msg_iov),
-						      (int) sizeof (iov));
+						      (unsigned long)sizeof (iov));
 				return -1;
 			      }
 			    if (record_arch_list_add_mem
@@ -934,8 +934,8 @@ record_linux_system_call (int num, struc
 	  break;
 	default:
 	  printf_unfiltered (_("Process record and replay target "
-			       "doesn't support socketcall call 0x%08x\n"),
-			     tmpu32);
+			       "doesn't support socketcall call 0x%s\n"),
+			     phex_nz (tmpu32, 4));
 	  return -1;
 	  break;
 	}
@@ -1233,9 +1233,9 @@ record_linux_system_call (int num, struc
 		    if (record_debug)
 		      fprintf_unfiltered (gdb_stdlog,
 					  "Process record: error reading "
-					  "memory at addr = 0x%s len = %d.\n",
+					  "memory at addr = 0x%s len = %lu.\n",
 					  paddr_nz (vec),
-					  (int) sizeof (struct record_iovec));
+					  (unsigned long)sizeof (struct record_iovec));
 		    return -1;
 		  }
 		if (record_arch_list_add_mem (iov.iov_base, iov.iov_len))
@@ -1631,20 +1631,17 @@ record_linux_system_call (int num, struc
       /* sys_fcntl64 */
     case 221:
       regcache_raw_read (regcache, tdep->arg2, (gdb_byte *) & tmpu32);
-      switch (tmpu32)
-	{
-	case F_GETLK64:
+      if (tmpu32 == tdep->fcntl_F_GETLK64)
+        {
 	  regcache_raw_read (regcache, tdep->arg3,
 			     (gdb_byte *) & tmpu32);
 	  if (record_arch_list_add_mem (tmpu32, tdep->size_flock64))
 	    return -1;
-	  break;
-	case F_SETLK64:
-	case F_SETLKW64:
-	  break;
-	default:
+	}
+      else if (tmpu32 != tdep->fcntl_F_SETLK64
+	       && tmpu32 != tdep->fcntl_F_SETLKW64)
+        {
 	  goto sys_fcntl;
-	  break;
 	}
       break;
 
@@ -1785,8 +1782,9 @@ record_linux_system_call (int num, struc
 	      if (record_debug)
 		fprintf_unfiltered (gdb_stdlog,
 				    "Process record: error reading memory "
-				    "at addr = 0x%s len = %d.\n",
-				    paddr_nz (tmpu32), nr * tdep->size_int);
+				    "at addr = 0x%s len = %u.\n",
+				    paddr_nz (tmpu32),
+				    (int)(nr * tdep->size_int));
 	      return -1;
 	    }
 	  for (i = 0; i < nr; i++)
@@ -2195,8 +2193,8 @@ record_linux_system_call (int num, struc
 
     default:
       printf_unfiltered (_("Process record and replay target doesn't "
-			   "support syscall number 0x%08x\n"),
-			 tmpu32);
+			   "support syscall number %u\n"),
+			 (int)tmpu32);
       return -1;
       break;
     }
--- a/linux-record.h
+++ b/linux-record.h
@@ -158,6 +158,13 @@ struct linux_record_tdep
   int ioctl_TIOCSHAYESESP;
   int ioctl_FIOQSIZE;
 
+  /* The values of the second argument of system call "sys_fcntl"
+     and "sys_fcntl64".  */
+  int fcntl_F_GETLK;
+  int fcntl_F_GETLK64;
+  int fcntl_F_SETLK64;
+  int fcntl_F_SETLKW64;
+
   /* The number of the registers that are used as the arguments of
      a system call.  */
   int arg1;

Re: [Prec/RFA] fix build error of prec in cygwin

[Eli Zaretskii]

> Date: Fri, 22 May 2009 17:06:17 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
> 
> On Fri, May 22, 2009 at 14:55, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Thu, 21 May 2009 23:05:45 -0700
> >> From: Joel Brobecker <brobecker@adacore.com>
> >>
> >> > "%zu"
> >>
> >> Are we allowed to use %z?
> >
> > No, I don't think so.
> >
> >> I thought that %z is C99
> >
> > It is.
> >
> > I think the best we can do with C90 is use %lu and cast to
> > `unsigned long'.
> >
> I make a new patch use %lu and unsigned long.  Please help me review it.

Looks okay to me, as far as using sizeof in printf-like statements is
concerned.

Re: [Prec/RFA] fix build error of prec in cygwin

[Eli Zaretskii]

> Date: Fri, 22 May 2009 16:42:06 +0800
> From: Hui Zhu <teawater@gmail.com>
> Cc: gdb-patches@sourceware.org
> 
> int
> main(int argc,char *argv[],char *envp[])
> {
> 	printf ("%uz", sizeof (int));
> 
> 	return (0);
> }
> 
> gcc -std=c89 -Wall -pedantic 1.c
> 
> No warning.

You are placing too much faith into -std= and -pedantic.  From the GCC
manual, node "Warning Options":

     Some users try to use `-pedantic' to check programs for strict ISO
     C conformance.  They soon find that it does not do quite what they
     want: it finds some non-ISO practices, but not all--only those for
     which ISO C _requires_ a diagnostic, and some others for which
     diagnostics have been added.

     A feature to report any failure to conform to ISO C might be
     useful in some instances, but would require considerable
     additional work and would be quite different from `-pedantic'.  We
     don't have plans to support such a feature in the near future.

Re: [Prec/RFA] fix build error of prec in cygwin

[Jie Zhang]

Hui Zhu wrote:
> I tried with gcc.
> 
> cat 1.c
> #include <sys/types.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <errno.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdint.h>
> 
> int
> main(int argc,char *argv[],char *envp[])
> {
> 	printf ("%uz", sizeof (int));

I think there is a typo. If you use "%zu", you will see the expected 
warning.

> 
> 	return (0);
> }
> 
> gcc -std=c89 -Wall -pedantic 1.c
> 
> No warning.
> 
> Thanks,
> Hui
> 
> On Fri, May 22, 2009 at 14:55, Eli Zaretskii <eliz@gnu.org> wrote:
>>> Date: Thu, 21 May 2009 23:05:45 -0700
>>> From: Joel Brobecker <brobecker@adacore.com>
>>>
>>>> "%zu"
>>> Are we allowed to use %z?
>> No, I don't think so.
>>
>>> I thought that %z is C99
>> It is.
>>


Jie

Re: [Prec/RFA] fix build error of prec in cygwin

[Hui Zhu]

On Fri, May 22, 2009 at 17:30, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Fri, 22 May 2009 17:06:17 +0800
>> From: Hui Zhu <teawater@gmail.com>
>> Cc: Joel Brobecker <brobecker@adacore.com>, gdb-patches@sourceware.org
>>
>> On Fri, May 22, 2009 at 14:55, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> Date: Thu, 21 May 2009 23:05:45 -0700
>> >> From: Joel Brobecker <brobecker@adacore.com>
>> >>
>> >> > "%zu"
>> >>
>> >> Are we allowed to use %z?
>> >
>> > No, I don't think so.
>> >
>> >> I thought that %z is C99
>> >
>> > It is.
>> >
>> > I think the best we can do with C90 is use %lu and cast to
>> > `unsigned long'.
>> >
>> I make a new patch use %lu and unsigned long.  Please help me review it.
>
> Looks okay to me, as far as using sizeof in printf-like statements is
> concerned.
>

Checked in.  Thanks.

Hui

Re: [Prec/RFA] fix build error of prec in cygwin

[Christopher Faylor]

On Thu, May 21, 2009 at 11:05:45PM -0700, Joel Brobecker wrote:
>> "%zu"
>
>Are we allowed to use %z? (I thought that %z is C99, but I don't have
>a C90 manual to double-check)

Ah, good point.  I think you're right.

cgf