Re: [PATCH 3/3] fhandler_pty_slave::setup_locale: respect charset == "UTF-8"

[Corinna Vinschen <corinna-cygwin@cygwin.com>]

Hi Takashi,

On Sep  7 13:45, Takashi Yano via Cygwin-patches wrote:
>  #if 0 /* Let's try this if setting codepage at pty open time is not enough */
> -  if (!cygheap->locale.term_code_page)
> -    cygheap->locale.term_code_page = __eval_codepage_from_internal_charset ();
> +  if (!get_ttyp ()->term_code_page)
> +    get_ttyp ()->term_code_page = __eval_codepage_from_internal_charset (NULL);
>  #endif

*If* we revert back to using setup_locale, these #if blocks would
go away.

> -__eval_codepage_from_internal_charset ()
> +__eval_codepage_from_internal_charset (const WCHAR *envblock)
>  {
> -  const char *charset = __locale_charset (__get_global_locale ());
> +  const char *charset;
> +  __locale_t *loc = NULL;
> +  if (__get_current_locale ()->lc_cat[LC_CTYPE].buf)
> +    charset = __locale_charset (__get_current_locale ());
> +  else
> +    {
> +      char locale[ENCODING_LEN + 1] = {0, };
> +      if (envblock)
> +	{
> +	  const WCHAR *lc_all = NULL, *lc_ctype = NULL, *lang = NULL;
> +	  for (const WCHAR *p = envblock; *p != L'\0'; p += wcslen (p) + 1)
> +	    if (wcsncmp (p, L"LC_ALL=", 7) == 0)
> +	      lc_all = p + 7;
> +	    else if (wcsncmp (p, L"LC_CTYPE=", 9) == 0)
> +	      lc_ctype = p + 9;
> +	    else if (wcsncmp (p, L"LANG=", 5) == 0)
> +	      lang = p + 5;
> +	  if (lc_all && *lc_all)
> +	    snprintf (locale, ENCODING_LEN + 1, "%ls", lc_all);
	    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
	    sys_wcstombs (locale, ENCODING_LEN + 1, lc_all);

OTOH, if you read these environment vars right from our current POSIX
env, you don't have to convert from mbs to wcs at all.  Just call
getenv("LC_ALL"), etc.  After all, envblock is just the wide char
copy of our current POSIX env.

> +	  else if (lc_ctype && *lc_ctype)
> +	    snprintf (locale, ENCODING_LEN + 1, "%ls", lc_ctype);
> +	  else if (lang && *lang)
> +	    snprintf (locale, ENCODING_LEN + 1, "%ls", lang);
> +	}
> +      if (!*locale)
> +	{
> +	  const char *env = __get_locale_env (_REENT, LC_CTYPE);
> +	  strncpy (locale, env, ENCODING_LEN);
> +	  locale[ENCODING_LEN] = '\0';
> +	}
> +      loc = duplocale (__get_current_locale ());
> +      __loadlocale (loc, LC_CTYPE, locale);
> +      charset = __locale_charset (loc);
> +    }

Oh, boy, this is really a lot.  I have some doubts this complexity is
really necessary.  It's a bit weird to go to such great lengths for
native applications.  Still, why not just do this once in the process
creating the pty rather than trying on every execve?

>      case 'I': /* ISO-8859-x */
> -      codepage = strtoul (charset + 9, NULL, 10);
> +      codepage = strtoul (charset + 9, NULL, 10) + 28590;

Oops, I just fixed that in my original patch already.


Corinna