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

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

[Ozkan Sezer]

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

Hi!

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?

gdb/gdbserver/

	* server.c (handle_query): For windows, use %I64d instead of %lld in
	the sprintf format string.

--
Ozkan

[-- Attachment #2: g2.diff --]
[-- Type: application/octet-stream, Size: 957 bytes --]

gdb/gdbserver/

	* server.c (handle_query): For windows, use %I64d instead of %lld in
	the sprintf format string.

Index: gdb/gdbserver/server.c
===================================================================
RCS file: /cvs/src/src/gdb/gdbserver/server.c,v
retrieving revision 1.125
diff -u -p -r1.125 server.c
--- gdb/gdbserver/server.c	7 Jul 2010 16:14:04 -0000	1.125
+++ gdb/gdbserver/server.c	16 Jul 2010 20:05:51 -0000
@@ -1539,7 +1539,11 @@ handle_query (char *own_buf, int packet_
 
       if (err == 0)
 	{
+#ifdef _WIN32
+	  sprintf (own_buf, "%I64d", address);
+#else
 	  sprintf (own_buf, "%llx", address);
+#endif
 	  return;
 	}
       else if (err > 0)
@@ -1563,7 +1567,11 @@ handle_query (char *own_buf, int packet_
       n = (*the_target->get_tib_address) (ptid, &tlb);
       if (n == 1)
 	{
+#ifdef _WIN32
+	  sprintf (own_buf, "%I64d", tlb);
+#else
 	  sprintf (own_buf, "%llx", tlb);
+#endif
 	  return;
 	}
       else if (n == 0)

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

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 861 bytes --]

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 ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Ozkan Sezer]

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 ?
> -mike
>

I would happily do that, however that would require inttypes
module merge from gnulib to gdb/gnulib, am I wrong?

--
Ozkan

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

[Ozkan Sezer]

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 ?
>> -mike
>>
>
> 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?

--
Ozkan

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

[Mark Kettenis]

> From: Mike Frysinger <vapier@gentoo.org>
> Date: Sat, 17 Jul 2010 05:37:43 -0400
> 
> On Friday, July 16, 2010 16:10:23 Ozkan Sezer wrote:
> > For windows targets, (x86_64-w64-mingw32, i686-w64-mingw32)
> > gcc complains:
> >=20
> > ../../../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
> >=20
> > 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 ?

"sane" isn't the word I'd use.  Not only do I find them to make printf
format strings to become unreadable, they also cause i18n issues.

<rant>
How on earth can people take this 64-bit Windows crap seriously?
Choosing LLP64 over LP64 when has been de de-facto standard for well
over a decade is, well, a bit odd.  But if "long long", which was
standardized with C99, is your only 64-bit integer type, but your
printf doesn't understand the C99 standard %ll format specifier,
you're shipping a crippled system.  To me that screams "we don't care
about portable software".  And my answer to that would be that I don't
care about Microsoft Windows.
</rant>

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

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 466 bytes --]

On Saturday, July 17, 2010 08:28:49 Mark Kettenis wrote:
> From: Mike Frysinger <vapier@gentoo.org>
> > ugh, no.  why not use a sane define like PRIx64 from inttypes.h ?
> 
> "sane" isn't the word I'd use.  Not only do I find them to make printf
> format strings to become unreadable, they also cause i18n issues.

the former is debatable, and while the latter is a valid point in the general 
case, i18n issues in this particular case do not apply.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 1810 bytes --]

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 
gnulib is a big deal ?
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Ozkan Sezer]

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

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

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 385 bytes --]

On Saturday, July 17, 2010 14:59:16 Ozkan Sezer wrote:
> ... 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.

sounds like that is the way to go.  think long term & correct, and you should 
have easy going with getting changes merged.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Ozkan Sezer]

On Sat, Jul 17, 2010 at 10:15 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> On Saturday, July 17, 2010 14:59:16 Ozkan Sezer wrote:
>> ... 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.
>
> sounds like that is the way to go.  think long term & correct, and you should
> have easy going with getting changes merged.
> -mike
>

Thanks.  Is the paddress() fix I suggested correct and can be
applied, then?

--
Ozkan

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

[Andreas Schwab]

Ozkan Sezer <sezeroz@gmail.com> writes:

> 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));

s/sprintf/strcpy/

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

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

[Ozkan Sezer]

On Sun, Jul 18, 2010 at 12:01 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Ozkan Sezer <sezeroz@gmail.com> writes:
>
>> 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));
>
> s/sprintf/strcpy/
>

Thanks for catching it, changing immediately to
strcpy (own_buf, paddress(address))

Still waiting for a review/comment on the fix for paddress() posted at
http://sourceware.org/ml/gdb-patches/2010-07/msg00254.html

> Andreas.
>
> --
> Andreas Schwab, schwab@linux-m68k.org
> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
> "And now for something completely different."
>

--
Ozkan

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

[Ozkan Sezer]

On Sun, Jul 18, 2010 at 12:05 AM, Ozkan Sezer <sezeroz@gmail.com> wrote:
> On Sun, Jul 18, 2010 at 12:01 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> Ozkan Sezer <sezeroz@gmail.com> writes:
>>
>>> 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));
>>
>> s/sprintf/strcpy/
>>
>
> Thanks for catching it, changing immediately to
> strcpy (own_buf, paddress(address))
>
> Still waiting for a review/comment on the fix for paddress() posted at
> http://sourceware.org/ml/gdb-patches/2010-07/msg00254.html
>

Applied with Andreas' suggestion integrated.


>> Andreas.
>>
>> --
>> Andreas Schwab, schwab@linux-m68k.org
>> GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
>> "And now for something completely different."
>>
>
> --
> Ozkan
>

--
Ozkan