Re: [RFC-v5] Handle cygwin wchar_t specifics

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Tom> I don't think DEFAULT_INTERMEDIATE_ENCODING is needed.

Pierre>   I assumed you ment: not necessary if PHONY_ICONV is defined,
Pierre> and this is what I changed below.
Pierre> (I would personally have favored to completely remove
Pierre> INTERMEDIATE_ENCODING macro and call the function directly.)
 
Sorry, that isn't what I meant.

All this new code is needed only in the __STDC_ISO_10646__ case.
All other cases are already handled ok.
So, I think it is best to only introduce new code along the
__STDC_ISO_10646__ branches.  Thus far your patches have touched all the
other branches -- but there is no reason to do that, and I think it just
makes it more complicated without an associated benefit.

Pierre> +#ifdef __STDC_ISO_10646__
Pierre> +  if (sizeof (gdb_wchar_t) == 2)

You might as well unify the 2 and 4 byte cases like I said earlier, and
just die for any other value.  You can use a static assert trick to make
it die during compilation, which I think is better than dying at
runtime.  E.g.:

extern char your_platform_is_bogus[(sizeof (gdb_wchar_t) == 2
                                    || sizeof (gdb_wchar_t) == 4)
                                    ? 1 : -1];

Pierre> +      /* Check that the name is in the list of handled charsets.  */
Pierre> +      for (i = 0; charset_enum[i]; i++)

I don't think this is really needed either.
Or, if you really want to do the check, do it by calling iconv_open at
initialization, and then just make gdb die early -- whatever platform
does this is really messed up.

Pierre> +  /* if gdb_wchar_t is not of size 2, or if "UTF-16XE" and "UCS-2XE" are
Pierre> +     not known, use DEFAULT_INTERMEDIATE_ENCODING macro.  */
Pierre> +  return DEFAULT_INTERMEDIATE_ENCODING;

I don't think this will generally do the right thing.
For example, your patch defines DEFAULT_INTERMEDIATE_ENCODING to
"UCS-4LE" in the !WORDS_BIGENDIAN case.  But we already know that
gdb_wchar_t has 2 bytes.  So I think this will just result in the same
bug as today.

Tom