[RFA] Fix typo in type of parameter "w" in print_wchar...

[RFA] Fix typo in type of parameter "w" in print_wchar...

[Joel Brobecker]

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

I think this is the last issue I've found so far with character set
conversions and printing...

This one occurs on AIX while trying to print a simple ascii string.
The output looks like this:

    (gdb) p __gnat_version 
    $3 = "

The problem turned out to be pretty straightforward, once I got to
understand how things are supposed to work.

Basically, for every character in our string, we call print_wchar
to push the equivalent wchar on our itermediate wchar buffer.
The issue is that we had the wrong type for the wchar (gdb_wint_t
instead of gdb_wchar_t). gdb_wint_t is 4bytes long whereas gdb_wchar_t
is 2 bytes long. So when we do the pushing:

      obstack_grow (output, &w, sizeof (gdb_wchar_t));

We only push 2 bytes of the 4 bytes that w got accidently promoted to.
On ppc-aix, it's big endian, so we end up always pushing zeros...

2009-04-24  Joel Brobecker  <brobecker@adacore.com>

        * c-lang.c (print_wchar): Use the correct type for parameter "w".

I'm currently testing the patch on x86_64-linux...

-- 
Joel

[-- Attachment #2: c-lang.diff --]
[-- Type: text/x-diff, Size: 453 bytes --]

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 027e9b2..5fe04de 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -156,7 +156,7 @@ append_string_as_wide (const char *string, struct obstack *output)
    escapes across calls.  */
 
 static void
-print_wchar (gdb_wint_t w, const gdb_byte *orig, int orig_len,
+print_wchar (gdb_wchar_t w, const gdb_byte *orig, int orig_len,
 	     int width, struct obstack *output, int quoter,
 	     int *need_escapep)
 {

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Mark Kettenis]

> Date: Fri, 24 Apr 2009 17:36:58 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> I think this is the last issue I've found so far with character set
> conversions and printing...
> 
> This one occurs on AIX while trying to print a simple ascii string.
> The output looks like this:
> 
>     (gdb) p __gnat_version 
>     $3 = "
> 
> The problem turned out to be pretty straightforward, once I got to
> understand how things are supposed to work.
> 
> Basically, for every character in our string, we call print_wchar
> to push the equivalent wchar on our itermediate wchar buffer.
> The issue is that we had the wrong type for the wchar (gdb_wint_t
> instead of gdb_wchar_t). gdb_wint_t is 4bytes long whereas gdb_wchar_t
> is 2 bytes long. So when we do the pushing:
> 
>       obstack_grow (output, &w, sizeof (gdb_wchar_t));
> 
> We only push 2 bytes of the 4 bytes that w got accidently promoted to.
> On ppc-aix, it's big endian, so we end up always pushing zeros...
> 
> 2009-04-24  Joel Brobecker  <brobecker@adacore.com>
> 
>         * c-lang.c (print_wchar): Use the correct type for parameter "w".
> 
> I'm currently testing the patch on x86_64-linux...

I think this is wrong.  The type of a single wide character is wint_t
instead of wchar_t such that it can properly hold WEOF, much in the
same way as the "normal" character functions use int instead of char.

The proper/safer approach is probably to do the conversion to wchar_t
in print_wchar itself just before that obstack_grow() call.

> diff --git a/gdb/c-lang.c b/gdb/c-lang.c
> index 027e9b2..5fe04de 100644
> --- a/gdb/c-lang.c
> +++ b/gdb/c-lang.c
> @@ -156,7 +156,7 @@ append_string_as_wide (const char *string, struct obstack *output)
>     escapes across calls.  */
>  
>  static void
> -print_wchar (gdb_wint_t w, const gdb_byte *orig, int orig_len,
> +print_wchar (gdb_wchar_t w, const gdb_byte *orig, int orig_len,
>  	     int width, struct obstack *output, int quoter,
>  	     int *need_escapep)
>  {

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Joel Brobecker]

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

> I think this is wrong.  The type of a single wide character is wint_t
> instead of wchar_t such that it can properly hold WEOF, much in the
> same way as the "normal" character functions use int instead of char.

Thanks for the hint, Mark.  Does the following look correct to you?
I've just tested that it also fixes the issue on AIX, and I got no
regression on amd64-linux.

2009-05-07  Joel Brobecker  <brobecker@adacore.com>

        * c-lang.c (print_wchar): Convert w into a gdb_wchar_t before
        pushing it on the output obstack.

-- 
Joel

[-- Attachment #2: wchar.diff --]
[-- Type: text/x-diff, Size: 544 bytes --]

diff --git a/gdb/c-lang.c b/gdb/c-lang.c
index 027e9b2..f95c98f 100644
--- a/gdb/c-lang.c
+++ b/gdb/c-lang.c
@@ -166,9 +166,11 @@ print_wchar (gdb_wint_t w, const gdb_byte *orig, int orig_len,
 					    && w != LCST ('8')
 					    && w != LCST ('9'))))
     {
+      gdb_wchar_t wchar = (gdb_wchar_t) w;
+
       if (w == gdb_btowc (quoter) || w == LCST ('\\'))
 	obstack_grow_wstr (output, LCST ("\\"));
-      obstack_grow (output, &w, sizeof (gdb_wchar_t));
+      obstack_grow (output, &wchar, sizeof (gdb_wchar_t));
     }
   else
     {

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Mark Kettenis]

> Date: Thu, 7 May 2009 11:37:53 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> 
> > I think this is wrong.  The type of a single wide character is wint_t
> > instead of wchar_t such that it can properly hold WEOF, much in the
> > same way as the "normal" character functions use int instead of char.
> 
> Thanks for the hint, Mark.  Does the following look correct to you?
> I've just tested that it also fixes the issue on AIX, and I got no
> regression on amd64-linux.
> 
> 2009-05-07  Joel Brobecker  <brobecker@adacore.com>
> 
>         * c-lang.c (print_wchar): Convert w into a gdb_wchar_t before
>         pushing it on the output obstack.
> 

> +      gdb_wchar_t wchar = (gdb_wchar_t) w;

Is that cast necessary?

Otherwise, this looks excellent to me.

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Joel Brobecker]

> > +      gdb_wchar_t wchar = (gdb_wchar_t) w;
> 
> Is that cast necessary?

Probably not. I just assumed I needed one, I don't know why.

> Otherwise, this looks excellent to me.

Thanks! I'll wait for a few more days to give Tom some time to comment
on this as well, and then commit (with the cast removed).

-- 
Joel

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Joel Brobecker]

> 2009-05-07  Joel Brobecker  <brobecker@adacore.com>
> 
>         * c-lang.c (print_wchar): Convert w into a gdb_wchar_t before
>         pushing it on the output obstack.

FYI: Checked in.

-- 
Joel

Re: [RFA] Fix typo in type of parameter "w" in print_wchar...

[Joel Brobecker]

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

> > 2009-05-07  Joel Brobecker  <brobecker@adacore.com>
> > 
> >         * c-lang.c (print_wchar): Convert w into a gdb_wchar_t before
> >         pushing it on the output obstack.

I forgot to remove the unnecessary cast when I checked the patch in :-(.
Fixed thusly.

2009-05-13  Joel Brobecker  <brobecker@adacore.com>

        * c-lang.c (print_wchar): Remove unnecessary cast.

Really sorry about that...
-- 
Joel

[-- Attachment #2: c-lang.c.diff --]
[-- Type: text/x-diff, Size: 563 bytes --]

Index: c-lang.c
===================================================================
RCS file: /cvs/src/src/gdb/c-lang.c,v
retrieving revision 1.67
diff -u -p -r1.67 c-lang.c
--- c-lang.c	12 May 2009 08:05:52 -0000	1.67
+++ c-lang.c	13 May 2009 09:40:44 -0000
@@ -166,7 +166,7 @@ print_wchar (gdb_wint_t w, const gdb_byt
 					    && w != LCST ('8')
 					    && w != LCST ('9'))))
     {
-      gdb_wchar_t wchar = (gdb_wchar_t) w;
+      gdb_wchar_t wchar = w;
 
       if (w == gdb_btowc (quoter) || w == LCST ('\\'))
 	obstack_grow_wstr (output, LCST ("\\"));