diff mbox

Fix display of tabulation character for mingw hosts.

Message ID 1399282018-8921-1-git-send-email-yao@codesourcery.com
State Committed
Headers

Commit Message

Yao Qi May 5, 2014, 9:26 a.m. UTC 
  Pierre proposed this patch
https://sourceware.org/ml/gdb-patches/2013-10/msg00011.html and
Tom gave a suggestion that it's better to do check \t in print_wchar
<https://sourceware.org/ml/gdb-patches/2013-11/msg00148.html>
However, I don't see the follow-up to this discussion.

We encounter two fails in printcmds.exp on mingw host, and Pierre's
patch fixes them.  I pick it up, update a little per Tom's
comments, and post it here for review.  This patch fixes these fails
below on mingw32 host.

FAIL: gdb.base/charset.exp: print string in ASCII
FAIL: gdb.base/charset.exp: try printing '\t' in ASCII
FAIL: gdb.base/charset.exp: print string in ISO-8859-1
FAIL: gdb.base/charset.exp: try printing '\t' in ISO-8859-1
FAIL: gdb.base/charset.exp: print string in UTF-32
FAIL: gdb.base/charset.exp: try printing '\t' in UTF-32
FAIL: gdb.base/printcmds.exp: p ctable1[9]
FAIL: gdb.base/printcmds.exp: p &ctable1[1*8]

Also regression tested on x86_64-linux.  Is it OK?

gdb:

2014-05-05  Pierre Muller  <muller@sourceware.org>
	    Yao Qi  <yao@codesourcery.com>

	* valprint.c (print_wchar): Move the code on checking whether
	W is a printable wide char to the default branch of switch
	statement below.  Call wchar_printable instead of gdb_iswprint.
---
 gdb/valprint.c | 122 +++++++++++++++++++++++++++++----------------------------
 1 file changed, 63 insertions(+), 59 deletions(-)

Comments

Yao Qi May 13, 2014, 9:19 a.m. UTC | #1 
On 05/05/2014 05:26 PM, Yao Qi wrote:
> Pierre proposed this patch
> https://sourceware.org/ml/gdb-patches/2013-10/msg00011.html and
> Tom gave a suggestion that it's better to do check \t in print_wchar
> <https://sourceware.org/ml/gdb-patches/2013-11/msg00148.html>
> However, I don't see the follow-up to this discussion.
> 
> We encounter two fails in printcmds.exp on mingw host, and Pierre's
> patch fixes them.  I pick it up, update a little per Tom's
> comments, and post it here for review.  This patch fixes these fails
> below on mingw32 host.
> 
> FAIL: gdb.base/charset.exp: print string in ASCII
> FAIL: gdb.base/charset.exp: try printing '\t' in ASCII
> FAIL: gdb.base/charset.exp: print string in ISO-8859-1
> FAIL: gdb.base/charset.exp: try printing '\t' in ISO-8859-1
> FAIL: gdb.base/charset.exp: print string in UTF-32
> FAIL: gdb.base/charset.exp: try printing '\t' in UTF-32
> FAIL: gdb.base/printcmds.exp: p ctable1[9]
> FAIL: gdb.base/printcmds.exp: p &ctable1[1*8]
> 
> Also regression tested on x86_64-linux.  Is it OK?
> 
> gdb:
> 
> 2014-05-05  Pierre Muller  <muller@sourceware.org>
> 	    Yao Qi  <yao@codesourcery.com>
> 
> 	* valprint.c (print_wchar): Move the code on checking whether
> 	W is a printable wide char to the default branch of switch
> 	statement below.  Call wchar_printable instead of gdb_iswprint.

Ping.  https://sourceware.org/ml/gdb-patches/2014-05/msg00034.html
Tom Tromey May 15, 2014, 5:31 p.m. UTC | #2 
>> 2014-05-05  Pierre Muller  <muller@sourceware.org>
>> Yao Qi  <yao@codesourcery.com>
>> 
>> * valprint.c (print_wchar): Move the code on checking whether
>> W is a printable wide char to the default branch of switch
>> statement below.  Call wchar_printable instead of gdb_iswprint.

Yao> Ping.  https://sourceware.org/ml/gdb-patches/2014-05/msg00034.html

Thanks, this is ok.

Tom
Yao Qi May 16, 2014, 12:20 p.m. UTC | #3 
On 05/16/2014 01:31 AM, Tom Tromey wrote:
>>> 2014-05-05  Pierre Muller  <muller@sourceware.org>
>>> Yao Qi  <yao@codesourcery.com>
>>>
>>> * valprint.c (print_wchar): Move the code on checking whether
>>> W is a printable wide char to the default branch of switch
>>> statement below.  Call wchar_printable instead of gdb_iswprint.
> 
> Yao> Ping.  https://sourceware.org/ml/gdb-patches/2014-05/msg00034.html
> 
> Thanks, this is ok.
> 

Thanks for the review.  Patch is pushed in.
diff mbox

Patch

diff --git a/gdb/valprint.c b/gdb/valprint.c
index fe23530..f55b5db 100644
--- a/gdb/valprint.c
+++ b/gdb/valprint.c
@@ -1949,73 +1949,77 @@  print_wchar (gdb_wint_t w, const gdb_byte *orig,
   int need_escape = *need_escapep;
 
   *need_escapep = 0;
-  if (gdb_iswprint (w) && (!need_escape || (!gdb_iswdigit (w)
-					    && w != LCST ('8')
-					    && w != LCST ('9'))))
-    {
-      gdb_wchar_t wchar = w;
 
-      if (w == gdb_btowc (quoter) || w == LCST ('\\'))
-	obstack_grow_wstr (output, LCST ("\\"));
-      obstack_grow (output, &wchar, sizeof (gdb_wchar_t));
-    }
-  else
+  /* iswprint implementation on Windows returns 1 for tab character.
+     In order to avoid different printout on this host, we explicitly
+     use wchar_printable function.  */
+  switch (w)
     {
-      switch (w)
+      case LCST ('\a'):
+	obstack_grow_wstr (output, LCST ("\\a"));
+	break;
+      case LCST ('\b'):
+	obstack_grow_wstr (output, LCST ("\\b"));
+	break;
+      case LCST ('\f'):
+	obstack_grow_wstr (output, LCST ("\\f"));
+	break;
+      case LCST ('\n'):
+	obstack_grow_wstr (output, LCST ("\\n"));
+	break;
+      case LCST ('\r'):
+	obstack_grow_wstr (output, LCST ("\\r"));
+	break;
+      case LCST ('\t'):
+	obstack_grow_wstr (output, LCST ("\\t"));
+	break;
+      case LCST ('\v'):
+	obstack_grow_wstr (output, LCST ("\\v"));
+	break;
+      default:
 	{
-	case LCST ('\a'):
-	  obstack_grow_wstr (output, LCST ("\\a"));
-	  break;
-	case LCST ('\b'):
-	  obstack_grow_wstr (output, LCST ("\\b"));
-	  break;
-	case LCST ('\f'):
-	  obstack_grow_wstr (output, LCST ("\\f"));
-	  break;
-	case LCST ('\n'):
-	  obstack_grow_wstr (output, LCST ("\\n"));
-	  break;
-	case LCST ('\r'):
-	  obstack_grow_wstr (output, LCST ("\\r"));
-	  break;
-	case LCST ('\t'):
-	  obstack_grow_wstr (output, LCST ("\\t"));
-	  break;
-	case LCST ('\v'):
-	  obstack_grow_wstr (output, LCST ("\\v"));
-	  break;
-	default:
-	  {
-	    int i;
+	  if (wchar_printable (w) && (!need_escape || (!gdb_iswdigit (w)
+						       && w != LCST ('8')
+						       && w != LCST ('9'))))
+	    {
+	      gdb_wchar_t wchar = w;
 
-	    for (i = 0; i + width <= orig_len; i += width)
-	      {
-		char octal[30];
-		ULONGEST value;
+	      if (w == gdb_btowc (quoter) || w == LCST ('\\'))
+		obstack_grow_wstr (output, LCST ("\\"));
+	      obstack_grow (output, &wchar, sizeof (gdb_wchar_t));
+	    }
+	  else
+	    {
+	      int i;
+
+	      for (i = 0; i + width <= orig_len; i += width)
+		{
+		  char octal[30];
+		  ULONGEST value;
 
-		value = extract_unsigned_integer (&orig[i], width,
+		  value = extract_unsigned_integer (&orig[i], width,
 						  byte_order);
-		/* If the value fits in 3 octal digits, print it that
-		   way.  Otherwise, print it as a hex escape.  */
-		if (value <= 0777)
-		  xsnprintf (octal, sizeof (octal), "\\%.3o",
-			     (int) (value & 0777));
-		else
-		  xsnprintf (octal, sizeof (octal), "\\x%lx", (long) value);
-		append_string_as_wide (octal, output);
-	      }
-	    /* If we somehow have extra bytes, print them now.  */
-	    while (i < orig_len)
-	      {
-		char octal[5];
+		  /* If the value fits in 3 octal digits, print it that
+		     way.  Otherwise, print it as a hex escape.  */
+		  if (value <= 0777)
+		    xsnprintf (octal, sizeof (octal), "\\%.3o",
+			       (int) (value & 0777));
+		  else
+		    xsnprintf (octal, sizeof (octal), "\\x%lx", (long) value);
+		  append_string_as_wide (octal, output);
+		}
+	      /* If we somehow have extra bytes, print them now.  */
+	      while (i < orig_len)
+		{
+		  char octal[5];
 
-		xsnprintf (octal, sizeof (octal), "\\%.3o", orig[i] & 0xff);
-		append_string_as_wide (octal, output);
-		++i;
-	      }
+		  xsnprintf (octal, sizeof (octal), "\\%.3o", orig[i] & 0xff);
+		  append_string_as_wide (octal, output);
+		  ++i;
+		}
 
-	    *need_escapep = 1;
-	  }
+	      *need_escapep = 1;
+	    }
 	  break;
 	}
     }