Re: [PATCH] [Windows] fix format string for 64 bit var in gdbserver

[Ozkan Sezer <sezeroz@gmail.com>]

On Sat, Jul 17, 2010 at 9:47 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Saturday, July 17, 2010 05:53:04 Ozkan Sezer wrote:
>> On Sat, Jul 17, 2010 at 12:42 PM, Ozkan Sezer <sezeroz@gmail.com> wrote:
>> > On Sat, Jul 17, 2010 at 12:37 PM, Mike Frysinger <vapier@gentoo.org>
> wrote:
>> >> On Friday, July 16, 2010 16:10:23 Ozkan Sezer wrote:
>> >>> For windows targets, (x86_64-w64-mingw32, i686-w64-mingw32)
>> >>> gcc complains:
>> >>>
>> >>> ../../../gdb-cvs/gdb/gdbserver/server.c: In function 'handle_query':
>> >>> ../../../gdb-cvs/gdb/gdbserver/server.c:1542: warning: unknown
>> >>> conversion type character 'l' in format
>> >>> ../../../gdb-cvs/gdb/gdbserver/server.c:1542: warning: too many
>> >>> arguments for format
>> >>> ../../../gdb-cvs/gdb/gdbserver/server.c:1566: warning: unknown
>> >>> conversion type character 'l' in format
>> >>> ../../../gdb-cvs/gdb/gdbserver/server.c:1566: warning: too many
>> >>> arguments for format
>> >>>
>> >>> This is due to the fact that MS printf doesn't support %lld, it uses
>> >>> its own %I64d which gcc already knows about. The attached patch
>> >>> changes that. OK for apply?
>> >>
>> >> ugh, no.  why not use a sane define like PRIx64 from inttypes.h ?
>> >
>> > I would happily do that, however that would require inttypes
>> > module merge from gnulib to gdb/gnulib, am I wrong?
>>
>> To be clear, I modified a patch I submitted before to not use
>> the inttypes PRI macros:
>> http://sourceware.org/ml/gdb-patches/2010-07/msg00244.html
>>
>> By analogy, I might use paddress() instead, but for that case
>> please see the issue I reported at
>> http://sourceware.org/ml/gdb-patches/2010-07/msg00254.html
>>
>> Comments?
>
> random #ifdefs like this are highly discouraged.  if you could find another
> way, that'd be great.  but i dont think importing a few small modules from

I do agree that the ifdefs are ugly. The cleanest way would be using
paddress() like this:

Index: server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.125
diff -u -p -r1.125 server.c
--- server.c	7 Jul 2010 16:14:04 -0000	1.125
+++ server.c	17 Jul 2010 18:54:41 -0000
@@ -1539,7 +1539,7 @@ handle_query (char *own_buf, int packet_

       if (err == 0)
 	{
-	  sprintf (own_buf, "%llx", address);
+	  sprintf (own_buf, "%s", paddress(address));
 	  return;
 	}
       else if (err > 0)
@@ -1563,7 +1563,7 @@ handle_query (char *own_buf, int packet_
       n = (*the_target->get_tib_address) (ptid, &tlb);
       if (n == 1)
 	{
-	  sprintf (own_buf, "%llx", tlb);
+	  sprintf (own_buf, "%s", paddress(tlb));
 	  return;
 	}
       else if (n == 0)


... which is non-intrusive and easy. However, as I reported here:
http://sourceware.org/ml/gdb-patches/2010-07/msg00254.html
... paddress() truncates its arguments and should be fixed.

> gnulib is a big deal ?

I might agree with that, but not everyone does ;)
If the above paddress() solution is not acceptable, I can easily
cook something using PRI macros (but someone else should
import inttypes from gnulib, that't not something I am comfortable
with.)

> -mike
>

Regards.

--
Ozkan