[RFC 1/6 -V2] Fix display of tabulation character for mingw hosts.

["Pierre Muller" <pierre.muller@ics-cnrs.unistra.fr>]

  Hi Keith,

  Thanks a lot for this review.

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Keith Seitz
> Envoyé : mardi 1 octobre 2013 03:19
> À : Pierre Muller
> Cc : 'gdb-patches'
> Objet : Re: [RFC 1/6] Fix display of tabulation character for mingw hosts.
> 
> On 09/26/2013 12:56 PM, Pierre Muller wrote:
> > 2013-09-26  Pierre Muller  <muller@sourceware.org>
> >
> > 	Fix display of tabulation character for mingw hosts.
>                                                  ^^^^^
> Probably want to keep this a little formal and use "MinGW".

  
 
> >   	* gdb_wchar.h (gdb_iswprint): Declare as external function
> >   	if __MINGW32__ macro is set.
> > 	* mingw-hdep.c (gdb_iswprint): New function.
> 
> I think this is a reasonable approach, but it will break builds that
> don't have a working/good iconv (amongst other reasons). I noticed this
> attempting to build --build=i686-unknown-linux
> --target=--host=i686-pc-mingw32:
> 
> ../../gdb/gdb/mingw-hdep.c:88:5: error: 'isprint' redeclared without
> dllimport attribute: previous dllimport ignored [-Werror=attributes]

  Yes, I also noticed this, but too late.
I checked and the msvcvrt DLL function isprint does not return 1 for
tabulation...
So the fix is only needed for the wide char version.

 
> This happens because we end up in the very last section of gdb_wchar.h
> ("If we got here and have wchar_t support, we might be on a system with
> some problem.  So, we just disable everything.") and that does:
> 
> #define gdb_iswprint isprint
> 
>  > +/* Mingw specific version of iswprint to correct
> 
> "MinGW-specific"
  Whoops, I hadn't noticed that this was always capitalized like that...
 
>  > +   difference concerning the tabulation character:
>  > +   msvcrt dll iswprint returns 1 for '\t' while
>  > +   UNIX uiswprint function returns 0 for '\t'.  */
>  > +
>  > +int gdb_iswprint (gdb_wint_t wc)
>     ^^^^^^^^^^^^^^^^^
>  > +{
>  > +  return wc == LCST ('\t') ? 0 : iswprint (wc);
>  > +}
> 
> The return type should be on a line by itself and the function name
> should start in column zero. Also please double-check that there is a
> newline between the closing brace of safe_strerror and the comment for
> this new function. [Maybe your patch got munged a bit?]

  Yes, I must confess that I broke more coding style rules
than I usually do here, sorry about that...

 
> Does this fix the output of many tests in the test suite already? If
> not, it would be really, really nice to have a test to check (and
> demonstrate) this quirk.

It does fix 2 failures inside printcmds.exp

288-p ctable1[9]
289:$50 = 9 '\t'
290-(gdb) PASS: gdb.base/printcmds.exp: p ctable1[9]
291-p ctable1[10]
292-$51 = 10 '\n'
--
1869-$560 = (unsigned char *) <ctable1+1> "\001\002\003\004\005\006\a\b"...
1870-(gdb) PASS: gdb.base/printcmds.exp: p &ctable1[1]
1871-p &ctable1[1*8]
1872:$561 = (unsigned char *) <ctable1+8> "\b\t\n\v\f\r\016\017"...
1873-(gdb) PASS: gdb.base/printcmds.exp: p &ctable1[1*8]
1874-p &ctable1[2*8]
1875-$562 = (unsigned char *) <ctable1+16>
"\020\021\022\023\024\025\026\027"...

Is this enough?

  Here is a new patch version.
To avoid the very complicated preprocessor checks that decides if we
use wide chars inside gdb_wchar.h, I added a new macro
HAVE_MINGW_GDB_ISWPRINT.
  While the idea seems good to me, I am unsure about the choice
of this macro name, are there similar examples already inside the code?



Pierre

ChangeLog entry:

2013-10-01  Pierre Muller <muller@sourceware.org>

 	Fix display of tabulation character for MinGW hosts.
 	* gdb_wchar.h (gdb_iswprint): Declare as external function
 	if __MINGW32__ macro is set.
	(HAVE_MINGW_GDB_ISWPRINT): New macro, declared only for
 	MinGW hosts using wide characters.
 	* mingw-hdep.c (gdb_iswprint): New function.
	Implemented only if HAVE_MINGW_GDB_ISWPRINT macro is defined.

---
 gdb/gdb_wchar.h  |    5 +++++
 gdb/mingw-hdep.c |   13 +++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/gdb/gdb_wchar.h b/gdb/gdb_wchar.h
index 0e785e8..9a07da2 100644
--- a/gdb/gdb_wchar.h
+++ b/gdb/gdb_wchar.h
@@ -65,7 +65,12 @@ typedef wchar_t gdb_wchar_t;
 typedef wint_t gdb_wint_t;
 
 #define gdb_wcslen wcslen
+#ifdef __MINGW32__
+#define HAVE_MINGW_GDB_ISWPRINT
+extern int gdb_iswprint (gdb_wint_t);
+#else
 #define gdb_iswprint iswprint
+#endif
 #define gdb_iswdigit iswdigit
 #define gdb_btowc btowc
 #define gdb_WEOF WEOF
diff --git a/gdb/mingw-hdep.c b/gdb/mingw-hdep.c
index efc9848..86d0023 100644
--- a/gdb/mingw-hdep.c
+++ b/gdb/mingw-hdep.c
@@ -81,6 +81,19 @@ safe_strerror (int errnum)
   return buffer;
 }
 
+#ifdef HAVE_MINGW_GDB_ISWPRINT
+/* MinGW-specific version of iswprint to correct
+   difference concerning the tabulation character:
+   msvcrt dll iswprint returns 1 for '\t' while
+   UNIX uiswprint function returns 0 for '\t'.  */
+
+int
+gdb_iswprint (gdb_wint_t wc)
+{
+  return wc == LCST ('\t') ? 0 : iswprint (wc);
+}
+#endif
+
 /* Return an absolute file name of the running GDB, if possible, or
    ARGV0 if not.  The return value is in malloc'ed storage.  */
 
-- 
1.7.9