Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows

[Eli Zaretskii <eliz at gnu dot org>]

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Sun, 31 Mar 2019 22:39:01 +0300
> Cc: gdb-patches@sourceware.org
> 
> > If so, would they need to
> > control that by setting environment variables?  It sounds less
> > convenient than it could have been, I think.  Why not a GDB variable
> > instead?
> 
> Environment variables are used in gettext anyway, I think single
> source for localisation language is convenient.

I was not talking about _using_ the environment variables, I was
talking about making them the (only) way of changing the locale.

> +  static HMODULE hMlang = LoadLibrary ("Mlang.dll");
> +  HMODULE hKernel32 = GetModuleHandle ("kernel32.dll");

I think this should not be an initialization, this should be a
run-timer assignment, like this:

  if (!hMlang)
    hMlang = LoadLibrary ("Mlang.dll");

Also, we should only try to load mlang.dll if LocaleNameToLCID is not
available.

> +  RFC1766TOLCID Rfc1766ToLcid = NULL;
> +  LOCALENAMETOLCID LocaleNameToLCID = NULL;

These two should also be static.
> +  strncpy (buf, localeName, COUNTOF (buf) - 1);
> +  ptr = strchr (buf, '.');      /* cut at "." */
> +  if (ptr != NULL)
> +    *ptr = 0x00;
> +
> +  ptr = strchr (buf, '_');      /* replace "_" */
> +  if (ptr != NULL)
> +    *ptr = '-';

Can you explain in a comment why these modifications of the
environment variable's value are needed?  I don't think the reasons
are trivially clear to everyone.

> +  if (LocaleNameToLCID != NULL)
> +  {
> +    MultiByteToWideChar (CP_ACP,
> +                        0,
> +                        buf,
> +                        -1,
> +                        wbuf,
> +                        COUNTOF (wbuf) - 1);
> +    lcid = LocaleNameToLCID (wbuf, 0);

This assumes that the code is compiled with UNICODE defined, which
will then call LocaleNameToLCIDW.  But the code in question is common
to Cygwin and MinGW compilations, and the latter defaults to UNICODE
undefined.  So I think you need to call LocaleNameToLCIDW explicitly
here.

Thanks.