Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
@ 2019-03-23 11:59 Владимир Мартьянов
  2019-03-23 13:11 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-23 11:59 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 497 bytes --]

The fourth argument of FormatMessage() in strwinerror() in file
win32-low.c is set to zero. Thus, Windows could use system default
locale to translate error codes to error message. At the same time,
gdbserver have hardcoded English error messages ("Error creating
process..." for example). As a result, server could display error
messages with mixed languages.

I made a patch to use en_US locale in FormatMessage, so all error
messages will be in English.

Changelog and patch files are attached

[-- Attachment #2: Changelog.24377 --]
[-- Type: application/octet-stream, Size: 219 bytes --]

gdb/gdbserver/ChangeLog:
2019-03-23  Vladimir Martyanov  <vilgeforce@gmail.com>

	PR server/24377
	* win32-low.c (strwinerror): Changed system deafult locale to
	en_US when translating Windows error code to message

[-- Attachment #3: 0001-Fix-mixing-English-and-system-default-languages-in-e.patch --]
[-- Type: application/octet-stream, Size: 809 bytes --]

From 1bcfc9bdc14399f04c802ac5e4f4591538b9d2fd Mon Sep 17 00:00:00 2001
From: Vladimir Martyanov <vilgeforce@gmail.com>
Date: Sat, 23 Mar 2019 14:31:33 +0300
Subject: [PATCH] Fix mixing English and system default languages in error
 messages on Windows

---
 gdb/gdbserver/win32-low.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 1a50141c12..226e0126d8 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -516,7 +516,7 @@ strwinerror (DWORD error)
 			       | FORMAT_MESSAGE_ALLOCATE_BUFFER,
 			       NULL,
 			       error,
-			       0, /* Default language */
+			       MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US),
 			       (LPTSTR) &msgbuf,
 			       0,
 			       NULL);
-- 
2.16.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-23 11:59 [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows Владимир Мартьянов
@ 2019-03-23 13:11 ` Eli Zaretskii
  2019-03-28 20:36   ` Владимир Мартьянов
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-03-23 13:11 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Sat, 23 Mar 2019 14:59:05 +0300
> 
> The fourth argument of FormatMessage() in strwinerror() in file
> win32-low.c is set to zero. Thus, Windows could use system default
> locale to translate error codes to error message. At the same time,
> gdbserver have hardcoded English error messages ("Error creating
> process..." for example). As a result, server could display error
> messages with mixed languages.
> 
> I made a patch to use en_US locale in FormatMessage, so all error
> messages will be in English.

Thanks, but isn't this a step backward?  Would it make sense to have
the hard-coded messages be translated instead?  Or is that impossible
for some reason?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-23 13:11 ` Eli Zaretskii
@ 2019-03-28 20:36   ` Владимир Мартьянов
  2019-03-29  8:27     ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-28 20:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

сб, 23 мар. 2019 г. в 16:11, Eli Zaretskii <eliz@gnu.org>:
> Thanks, but isn't this a step backward?  Would it make sense to have
> the hard-coded messages be translated instead?  Or is that impossible
> for some reason?

There is _nl_locale_name function in intl/localename.c, it could be
used to retrieve current locale name, which, in turn, could be used to
translate messages in FormatMessage. But when I include gettextP.h in
win32-low.c I got an error:
In file included from win32-low.c:38:0:
../../intl/gettextP.h:71:8: error: inline variables are only available
with -std=c++1z or -std=gnu++1z [-Werror]

I can't realise what's wrong


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-28 20:36   ` Владимир Мартьянов
@ 2019-03-29  8:27     ` Eli Zaretskii
  2019-03-29  8:42       ` Владимир Мартьянов
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-03-29  8:27 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Thu, 28 Mar 2019 23:36:05 +0300
> Cc: gdb-patches@sourceware.org
> 
> сб, 23 мар. 2019 г. в 16:11, Eli Zaretskii <eliz@gnu.org>:
> > Thanks, but isn't this a step backward?  Would it make sense to have
> > the hard-coded messages be translated instead?  Or is that impossible
> > for some reason?
> 
> There is _nl_locale_name function in intl/localename.c, it could be
> used to retrieve current locale name, which, in turn, could be used to
> translate messages in FormatMessage. But when I include gettextP.h in
> win32-low.c I got an error:
> In file included from win32-low.c:38:0:
> ../../intl/gettextP.h:71:8: error: inline variables are only available
> with -std=c++1z or -std=gnu++1z [-Werror]

Can you tell why you needed to retrieve the current locale's name?

If gdbserver supports NLS and gettext, all you need is to wrap the
untranslated messages in _().


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-29  8:27     ` Eli Zaretskii
@ 2019-03-29  8:42       ` Владимир Мартьянов
  2019-03-29  9:29         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-29  8:42 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Just try to use gettext with FormatMessage. I don't have idea how to
do it, could you please give me a code sample?

пт, 29 мар. 2019 г. в 11:27, Eli Zaretskii <eliz@gnu.org>:
>
> > From: Владимир Мартьянов <vilgeforce@gmail.com>
> > Date: Thu, 28 Mar 2019 23:36:05 +0300
> > Cc: gdb-patches@sourceware.org
> >
> > сб, 23 мар. 2019 г. в 16:11, Eli Zaretskii <eliz@gnu.org>:
> > > Thanks, but isn't this a step backward?  Would it make sense to have
> > > the hard-coded messages be translated instead?  Or is that impossible
> > > for some reason?
> >
> > There is _nl_locale_name function in intl/localename.c, it could be
> > used to retrieve current locale name, which, in turn, could be used to
> > translate messages in FormatMessage. But when I include gettextP.h in
> > win32-low.c I got an error:
> > In file included from win32-low.c:38:0:
> > ../../intl/gettextP.h:71:8: error: inline variables are only available
> > with -std=c++1z or -std=gnu++1z [-Werror]
>
> Can you tell why you needed to retrieve the current locale's name?
>
> If gdbserver supports NLS and gettext, all you need is to wrap the
> untranslated messages in _().


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-29  8:42       ` Владимир Мартьянов
@ 2019-03-29  9:29         ` Eli Zaretskii
  2019-03-29  9:39           ` Владимир Мартьянов
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-03-29  9:29 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Fri, 29 Mar 2019 11:42:36 +0300
> Cc: gdb-patches@sourceware.org
> 
> Just try to use gettext with FormatMessage. I don't have idea how to
> do it, could you please give me a code sample?

I believe this is a misunderstanding of some kind, because there's
nothing wrong with FormatMessage calls.  As you originally pointed
out, the 4th argument to FormatMessage is zero, which means use the
system default locale.  The problem you raised was that gdbserver has
messages where English is hard-coded, so this makes gdbserver
sometimes talk in English and sometimes in the default locale's
language.

My proposal was to make gdbserver _always_ talk in the current
locale's language, which means the FormatMessage call should be left
alone, and instead those gdbserver messages that use English
hard-coded should be translated by using gettext.

That is, instead of

      error ("Error creating process \"%s%s\", (error %d): %s\n",
	     program, args, (int) err, strwinerror (err));

gdbserver should use this:

      error (_("Error creating process \"%s%s\", (error %d): %s\n"),
	     program, args, (int) err, strwinerror (err));

A question to other GDB maintainers: does gdbserver use gettext?  Or
is gdbserver supposed to talk to users only in English?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-29  9:29         ` Eli Zaretskii
@ 2019-03-29  9:39           ` Владимир Мартьянов
  2019-03-29 12:32             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-29  9:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

Using system default locale is not a good idea, user must have a way
to change message language. If somebody have to debug on chinese
Windows, he'll get Chinese messages. And this way mustn't affect any
other applications or the system itself. LC_ALL and other env.
variables will affect only gettext, not FormatMessage in the current
implementation.

пт, 29 мар. 2019 г. в 12:29, Eli Zaretskii <eliz@gnu.org>:
>
> > From: Владимир Мартьянов <vilgeforce@gmail.com>
> > Date: Fri, 29 Mar 2019 11:42:36 +0300
> > Cc: gdb-patches@sourceware.org
> >
> > Just try to use gettext with FormatMessage. I don't have idea how to
> > do it, could you please give me a code sample?
>
> I believe this is a misunderstanding of some kind, because there's
> nothing wrong with FormatMessage calls.  As you originally pointed
> out, the 4th argument to FormatMessage is zero, which means use the
> system default locale.  The problem you raised was that gdbserver has
> messages where English is hard-coded, so this makes gdbserver
> sometimes talk in English and sometimes in the default locale's
> language.
>
> My proposal was to make gdbserver _always_ talk in the current
> locale's language, which means the FormatMessage call should be left
> alone, and instead those gdbserver messages that use English
> hard-coded should be translated by using gettext.
>
> That is, instead of
>
>       error ("Error creating process \"%s%s\", (error %d): %s\n",
>              program, args, (int) err, strwinerror (err));
>
> gdbserver should use this:
>
>       error (_("Error creating process \"%s%s\", (error %d): %s\n"),
>              program, args, (int) err, strwinerror (err));
>
> A question to other GDB maintainers: does gdbserver use gettext?  Or
> is gdbserver supposed to talk to users only in English?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-29  9:39           ` Владимир Мартьянов
@ 2019-03-29 12:32             ` Eli Zaretskii
  2019-03-30 21:26               ` Владимир Мартьянов
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-03-29 12:32 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Fri, 29 Mar 2019 12:38:50 +0300
> Cc: gdb-patches@sourceware.org
> 
> Using system default locale is not a good idea, user must have a way
> to change message language. If somebody have to debug on chinese
> Windows, he'll get Chinese messages. And this way mustn't affect any
> other applications or the system itself. LC_ALL and other env.
> variables will affect only gettext, not FormatMessage in the current
> implementation.

That completes the full circle and gets us right back to square one.
Where you wanted to force FormatMessage to use the English language
and I asked whether this isn't a step backward.

Anyway, since we have reviewed the various ways of fixing this, I
think you now have all the information you need to produce a patch.
Telling FormatMessage to use English is simple, and letting users
choose a different locale is only meaningful if the other messages,
those which aren't output via FormatMessage, can also be translated to
other languages, which requires to use _() and Gettext.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-29 12:32             ` Eli Zaretskii
@ 2019-03-30 21:26               ` Владимир Мартьянов
  2019-03-31 14:45                 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-30 21:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 224 bytes --]

I'm proposing this patch to translate messages in strwinerror.

Successfully built in Cygwin x86 and Cygwin x64

I didn't check locale name against "C" value because don't know what
locale to use: English or system default.

[-- Attachment #2: 0001-gettext-like-localisation-of-error-messages-added-to.patch --]
[-- Type: application/x-patch, Size: 2251 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-30 21:26               ` Владимир Мартьянов
@ 2019-03-31 14:45                 ` Eli Zaretskii
  2019-03-31 19:39                   ` Владимир Мартьянов
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-03-31 14:45 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Sun, 31 Mar 2019 00:26:30 +0300
> Cc: gdb-patches@sourceware.org
> 
> I'm proposing this patch to translate messages in strwinerror.

Thanks, I have a few comments.

> I didn't check locale name against "C" value because don't know what
> locale to use: English or system default.

The "C" locale is more or less identical to the English locale, not to
the system default.

> +  //retrieve LCID

I believe the convention is to use comments /* like this */.

> +  typedef HRESULT (WINAPI *RFC1766TOLCID)(LCID *pLocale, LPTSTR pszRfc1766);
> +  RFC1766TOLCID Rfc1766ToLcid;

This is an undocumented function.  I think using it is only justified
on XP, because since Vista there's the documented LocaleNameToLCID.
If LocaleNameToLCID is available, we should use it in preference to
Rfc1766ToLcid.

> +  HMODULE hMlang = LoadLibrary("Mlang.dll");
> +  if (hMlang != NULL)
> [...]
> +    FreeLibrary(hMlang);

Instead of loading the DLL every time this function is called, make
hMlang a static variable, and then you need to load the DLL and obtain
the function's entry address only once, the first time the function is
called.

> +      localeName = getenv ("LC_ALL");
> +      if (localeName == NULL || localeName[0] == '\0')
> +      {
> +        /* Next comes the name of the desired category.  */
> +        localeName = getenv ("LC_MESSAGES");
> +        if (localeName == NULL || localeName[0] == '\0')
> +        {
> +          /* Last possibility is the LANG environment variable.  */
> +          localeName = getenv ("LANG");

Bother: do we want to behave like a Posix platform here, or like a
Windows system?  Windows doesn't have LC_MESSAGES as a locale
category.

> +        }
> +      }
> +    }
> +
> +    if (localeName != NULL && localeName[0] != '\0'){   //have something to convert
> +      strncpy(buf, localeName, (COUNTOF (buf)) - 1);
> +
> +      ptr = strchr(buf, '.');		//cut at "."
> +      if (ptr != 0) {
> +        *ptr = 0x00;
> +      }

There are some stylistic issues here: the braces should be on separate
lines, there should be a blank between a function/macro name and the
opening parenthesis, and I believe we use NULL, not zero for a null
pointer.  Also, a single-line block doesn't need to use braces.

Finally, a more general point: I'm not sure I understand the purpose
of this change.  Is the purpose to let users control the language of
the gdbserver system error messages?  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?

Thanks.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-31 14:45                 ` Eli Zaretskii
@ 2019-03-31 19:39                   ` Владимир Мартьянов
  2019-04-01  4:47                     ` Eli Zaretskii
  2019-04-01 22:08                     ` Jon Turney
  0 siblings, 2 replies; 23+ messages in thread
From: Владимир Мартьянов @ 2019-03-31 19:39 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

вс, 31 мар. 2019 г. в 17:45, Eli Zaretskii <eliz@gnu.org>:

> Bother: do we want to behave like a Posix platform here, or like a
> Windows system?  Windows doesn't have LC_MESSAGES as a locale
> category.

I just saw LC_MESSAGES usage in gettext...

> Finally, a more general point: I'm not sure I understand the purpose
> of this change.  Is the purpose to let users control the language of
> the gdbserver system error messages?

Yes, it's the purprose.

> 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 corrected issues you mentioned (I hope!) and made a function to get locale ID.

[-- Attachment #2: 0001-gettext-like-localisation-of-error-messages-added-to.patch --]
[-- Type: application/octet-stream, Size: 3285 bytes --]

From 85f46d36a2043c58248d2d6cb01e03acb66202fa Mon Sep 17 00:00:00 2001
From: Vladimir Martyanov <vilgeforce@gmail.com>
Date: Sun, 31 Mar 2019 22:18:24 +0300
Subject: [PATCH] gettext-like localisation of error messages added to
 strwinerror

---
 gdb/gdbserver/win32-low.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 69 insertions(+), 1 deletion(-)

diff --git a/gdb/gdbserver/win32-low.c b/gdb/gdbserver/win32-low.c
index 1a50141c12..fe8c1453a3 100644
--- a/gdb/gdbserver/win32-low.c
+++ b/gdb/gdbserver/win32-low.c
@@ -496,6 +496,73 @@ child_store_inferior_registers (struct regcache *regcache, int r)
       (*the_low_target.store_inferior_register) (regcache, th, regno);
 }
 
+/* Return locale ID from environment variables (LC_ALL and LANG)
+    or 0 (which means system default). */
+LCID
+get_lcid(void)
+{
+  typedef HRESULT (WINAPI *RFC1766TOLCID)(LCID *pLocale, LPTSTR pszRfc1766);
+  typedef LCID (WINAPI *LOCALENAMETOLCID)(LPCWSTR lpName, DWORD dwFlags);
+
+  static HMODULE hMlang = LoadLibrary ("Mlang.dll");
+  HMODULE hKernel32 = GetModuleHandle ("kernel32.dll");
+
+  LCID lcid = 0; /* System default */
+  char buf[256];
+  WCHAR wbuf[256];
+  char *localeName = NULL;
+  char *ptr = NULL;
+  RFC1766TOLCID Rfc1766ToLcid = NULL;
+  LOCALENAMETOLCID LocaleNameToLCID = NULL;
+
+  LocaleNameToLCID = (LOCALENAMETOLCID) GetProcAddress (hKernel32,
+                                                       "LocaleNameToLCID");
+  if (LocaleNameToLCID == NULL)
+  {
+    if (hMlang != NULL)
+      Rfc1766ToLcid = (RFC1766TOLCID) GetProcAddress (hMlang,
+                                                      "Rfc1766ToLcidW");
+    if (Rfc1766ToLcid == NULL)
+      return lcid;  /* No conversion funcions - return system default */
+  }
+
+  /* Setting of LC_ALL overwrites all other. */
+  localeName = getenv ("LC_ALL");
+  if (localeName == NULL || localeName[0] == '\0')
+    localeName = getenv ("LANG");
+
+
+  if (localeName == NULL || localeName[0] == '\0')
+    return lcid;
+
+  if (!strcmp (localeName, "C"))
+    return MAKELANGID (LANG_ENGLISH, SUBLANG_ENGLISH_US);
+
+  strncpy (buf, localeName, COUNTOF (buf) - 1);
+  ptr = strchr (buf, '.');      /* cut at "." */
+  if (ptr != NULL)
+    *ptr = 0x00;
+
+  ptr = strchr (buf, '_');      /* replace "_" */
+  if (ptr != NULL)
+    *ptr = '-';
+
+  if (LocaleNameToLCID != NULL)
+  {
+    MultiByteToWideChar (CP_ACP,
+                        0,
+                        buf,
+                        -1,
+                        wbuf,
+                        COUNTOF (wbuf) - 1);
+    lcid = LocaleNameToLCID (wbuf, 0);
+  }
+  else
+    Rfc1766ToLcid (&lcid, buf);
+
+  return lcid;
+}
+
 /* Map the Windows error number in ERROR to a locale-dependent error
    message string and return a pointer to it.  Typically, the values
    for ERROR come from GetLastError.
@@ -512,11 +579,12 @@ strwinerror (DWORD error)
   static char buf[1024];
   TCHAR *msgbuf;
   DWORD lasterr = GetLastError ();
+
   DWORD chars = FormatMessage (FORMAT_MESSAGE_FROM_SYSTEM
 			       | FORMAT_MESSAGE_ALLOCATE_BUFFER,
 			       NULL,
 			       error,
-			       0, /* Default language */
+			       get_lcid (),
 			       (LPTSTR) &msgbuf,
 			       0,
 			       NULL);
-- 
2.16.2.windows.1


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-31 19:39                   ` Владимир Мартьянов
@ 2019-04-01  4:47                     ` Eli Zaretskii
  2019-04-02 20:08                       ` Владимир Мартьянов
  2019-04-01 22:08                     ` Jon Turney
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-04-01  4:47 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> 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.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-03-31 19:39                   ` Владимир Мартьянов
  2019-04-01  4:47                     ` Eli Zaretskii
@ 2019-04-01 22:08                     ` Jon Turney
  2019-04-02 20:57                       ` Владимир Мартьянов
  1 sibling, 1 reply; 23+ messages in thread
From: Jon Turney @ 2019-04-01 22:08 UTC (permalink / raw)
  To: Владимир
	Мартьянов,
	gdb-patches

On 31/03/2019 20:39, Владимир Мартьянов wrote:
> вс, 31 мар. 2019 г. в 17:45, Eli Zaretskii <eliz@gnu.org>:
> 
>> Bother: do we want to behave like a Posix platform here, or like a
>> Windows system?  Windows doesn't have LC_MESSAGES as a locale
>> category.
> 
> I just saw LC_MESSAGES usage in gettext...

Having LC_MESSAGES be controlling is definitely correct for Cygwin.

I'm not sure about a MinGW build of gdb.

>> Finally, a more general point: I'm not sure I understand the purpose
>> of this change.  Is the purpose to let users control the language of
>> the gdbserver system error messages?
> 
> Yes, it's the purprose.

I'm wondering why this problem doesn't crop up in gdb itself?  Are there 
no uses of FormatMessage() in that?

>> 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 corrected issues you mentioned (I hope!) and made a function to get locale ID.

A few comments on your get_lcid() function:

- It looks like this is going to ignore LC_ALL etc. if LocaleNameToLCID 
and Rfc1766ToLcidW can't found.  This doesn't seem correct as the 
environment variable should still have an effect.

- You're not checking all the environment variables which might control 
the locale for the message locale category (See e.g. [1]). I don't think 
there's any need to do this by hand, since you should be able to use the 
result of setlocale(LC_MESSAGE, NULL)?

[1] 
https://www.gnu.org/software/gettext/manual/html_node/Locale-Environment-Variables.html



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-01  4:47                     ` Eli Zaretskii
@ 2019-04-02 20:08                       ` Владимир Мартьянов
  2019-04-03  4:30                         ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: Владимир Мартьянов @ 2019-04-02 20:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches

пн, 1 апр. 2019 г. в 07:47, Eli Zaretskii <eliz@gnu.org>:
>
> > +  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.

Unfortunately, there is no LocaleNameToLCIDA and LocaleNameToLCIDW,
there is only LocaleNameToLCID which accept widechar strings only. At
least, I'm not able to find LocaleNameToLCIDA or LocaleNameToLCIDW in
MSDN or in kernel32.dll exports. That's the reason I used not
recommended Rfc1766ToLcid function.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-01 22:08                     ` Jon Turney
@ 2019-04-02 20:57                       ` Владимир Мартьянов
  2019-04-02 21:10                         ` Jon Turney
  2019-04-03  4:48                         ` Eli Zaretskii
  0 siblings, 2 replies; 23+ messages in thread
From: Владимир Мартьянов @ 2019-04-02 20:57 UTC (permalink / raw)
  To: Jon Turney; +Cc: gdb-patches

вт, 2 апр. 2019 г. в 01:08, Jon Turney <jon.turney@dronecode.org.uk>:
>
> I'm wondering why this problem doesn't crop up in gdb itself?  Are there
> no uses of FormatMessage() in that?

FormatMessage is used in gdb/gdb-dlfcn.c, gdb/common/mingw-strerror.c,
gdb/gdbserver/gdbreplay.c and in gdb/gdbserver/win32-low.c. If I
passed a dummy file name to gdb "gdb.exe foo" it shows me a
non-WIndows message "No such file or directory". I think it's from
libiberty.

> A few comments on your get_lcid() function:
>
> - It looks like this is going to ignore LC_ALL etc. if LocaleNameToLCID
> and Rfc1766ToLcidW can't found.  This doesn't seem correct as the
> environment variable should still have an effect.

it will be just the current behaviour - to use system default locale
in FormatMessage.

> - You're not checking all the environment variables which might control
> the locale for the message locale category (See e.g. [1]). I don't think
> there's any need to do this by hand, since you should be able to use the
> result of setlocale(LC_MESSAGE, NULL)?

Yes, you are right, I missed "LANGUAGE" var because it's in another
file in /intl/
Wish I had a function for this. _nl_locale_name from intl/localename.c
looks good,but it doesn't read LANGUAGE var and I have errors when
include intl/gettextP.h

I'm able to use results from setlocale(), but how can the user
influenсe the result of setlocale?
I wrote i simple program to display it's result:
void main(int argc, char* argv[])
{
    printf("%s\n", setlocale(LC_ALL, 0));
}
Then I run it from Cygwin console:
$ LC_ALL=en_US /cygdrive/c/Programming/Test/Debug/test.exe
C
$ LC_ALL=de_DE /cygdrive/c/Programming/Test/Debug/test.exe
C

Changing env. var doesn't affect setlocale() result.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-02 20:57                       ` Владимир Мартьянов
@ 2019-04-02 21:10                         ` Jon Turney
  2019-04-02 21:18                           ` Владимир Мартьянов
  2019-04-03  4:48                         ` Eli Zaretskii
  1 sibling, 1 reply; 23+ messages in thread
From: Jon Turney @ 2019-04-02 21:10 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

On 02/04/2019 21:57, Владимир Мартьянов wrote:
> I'm able to use results from setlocale(), but how can the user
> influenсe the result of setlocale?
> I wrote i simple program to display it's result:
> void main(int argc, char* argv[])
> {
>      printf("%s\n", setlocale(LC_ALL, 0));
> }
> Then I run it from Cygwin console:
> $ LC_ALL=en_US /cygdrive/c/Programming/Test/Debug/test.exe
> C
> $ LC_ALL=de_DE /cygdrive/c/Programming/Test/Debug/test.exe
> C
> 
> Changing env. var doesn't affect setlocale() result.

This test program isn't correct.  You must first call setlocale(LC_ALL, 
"") to change the locale from the default "C" locale.

See:
http://pubs.opengroup.org/onlinepubs/9699919799/functions/setlocale.html
http://man7.org/linux/man-pages/man3/setlocale.3.html


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-02 21:10                         ` Jon Turney
@ 2019-04-02 21:18                           ` Владимир Мартьянов
  0 siblings, 0 replies; 23+ messages in thread
From: Владимир Мартьянов @ 2019-04-02 21:18 UTC (permalink / raw)
  To: Jon Turney; +Cc: gdb-patches

ср, 3 апр. 2019 г. в 00:10, Jon Turney <jon.turney@dronecode.org.uk>:
>
> This test program isn't correct.  You must first call setlocale(LC_ALL,
> "") to change the locale from the default "C" locale.

You are right, but how can I do it if I have compiled gdbserver.exe?


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-02 20:08                       ` Владимир Мартьянов
@ 2019-04-03  4:30                         ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2019-04-03  4:30 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Tue, 2 Apr 2019 23:08:38 +0300
> Cc: gdb-patches@sourceware.org
> 
> пн, 1 апр. 2019 г. в 07:47, Eli Zaretskii <eliz@gnu.org>:
> >
> > > +  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.
> 
> Unfortunately, there is no LocaleNameToLCIDA and LocaleNameToLCIDW,
> there is only LocaleNameToLCID which accept widechar strings only.

You are right, it was my mistake.  Sorry.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-02 20:57                       ` Владимир Мартьянов
  2019-04-02 21:10                         ` Jon Turney
@ 2019-04-03  4:48                         ` Eli Zaretskii
  2019-04-03  4:58                           ` LRN
  1 sibling, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-04-03  4:48 UTC (permalink / raw)
  To: Владимир
	Мартьянов
  Cc: jon.turney, gdb-patches

> From: Владимир Мартьянов <vilgeforce@gmail.com>
> Date: Tue, 2 Apr 2019 23:57:35 +0300
> Cc: gdb-patches@sourceware.org
> 
> > - You're not checking all the environment variables which might control
> > the locale for the message locale category (See e.g. [1]). I don't think
> > there's any need to do this by hand, since you should be able to use the
> > result of setlocale(LC_MESSAGE, NULL)?
> 
> Yes, you are right, I missed "LANGUAGE" var because it's in another
> file in /intl/
> Wish I had a function for this. _nl_locale_name from intl/localename.c
> looks good,but it doesn't read LANGUAGE var and I have errors when
> include intl/gettextP.h
> 
> I'm able to use results from setlocale(), but how can the user
> influenсe the result of setlocale?

Beware: only the Cygwin setlocale supports environment variables, the
native MS-Windows implementation used by MinGW does not.


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-03  4:48                         ` Eli Zaretskii
@ 2019-04-03  4:58                           ` LRN
  2019-04-03  7:37                             ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: LRN @ 2019-04-03  4:58 UTC (permalink / raw)
  To: gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 422 bytes --]

On 03.04.2019 7:47, Eli Zaretskii wrote:
> On Tue, 2 Apr 2019 23:57:35 +0300 Владимир Мартьянов wrote:
>>
>> I'm able to use results from setlocale(), but how can the user
>> influenсe the result of setlocale?
> 
> Beware: only the Cygwin setlocale supports environment variables, the
> native MS-Windows implementation used by MinGW does not.
> 

That's what gettext is for. And/or gnulib.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-03  4:58                           ` LRN
@ 2019-04-03  7:37                             ` Eli Zaretskii
  2019-04-03 11:32                               ` LRN
  0 siblings, 1 reply; 23+ messages in thread
From: Eli Zaretskii @ 2019-04-03  7:37 UTC (permalink / raw)
  To: LRN; +Cc: gdb-patches

> From: LRN <lrn1986@gmail.com>
> Date: Wed, 3 Apr 2019 07:58:44 +0300
> 
> > Beware: only the Cygwin setlocale supports environment variables, the
> > native MS-Windows implementation used by MinGW does not.
> > 
> 
> That's what gettext is for. And/or gnulib.

Won't happen unless Gnulib's setlocale is imported.  And the proposed
patch didn't do that, AFAIU.  (I don't see 'setlocale' in the Gettext
library, FWIW, so I don't think Gettext alone will solve this.)


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-03  7:37                             ` Eli Zaretskii
@ 2019-04-03 11:32                               ` LRN
  2019-04-03 12:04                                 ` Eli Zaretskii
  0 siblings, 1 reply; 23+ messages in thread
From: LRN @ 2019-04-03 11:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: gdb-patches


[-- Attachment #1.1: Type: text/plain, Size: 636 bytes --]

On 03.04.2019 10:38, Eli Zaretskii wrote:
> On Wed, 3 Apr 2019 07:58:44 +0300 LRN wrote:
>>
>>> Beware: only the Cygwin setlocale supports environment variables, the
>>> native MS-Windows implementation used by MinGW does not.
>>>
>>
>> That's what gettext is for. And/or gnulib.
> 
> Won't happen unless Gnulib's setlocale is imported.  And the proposed
> patch didn't do that, AFAIU.  (I don't see 'setlocale' in the Gettext
> library, FWIW, so I don't think Gettext alone will solve this.)
> 

Gettext (GNU libintl, at the very least) does import setlocale.c from gnulib
(among other things). Does gdb use libintl?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows
  2019-04-03 11:32                               ` LRN
@ 2019-04-03 12:04                                 ` Eli Zaretskii
  0 siblings, 0 replies; 23+ messages in thread
From: Eli Zaretskii @ 2019-04-03 12:04 UTC (permalink / raw)
  To: LRN; +Cc: gdb-patches

> Cc: gdb-patches@sourceware.org
> From: LRN <lrn1986@gmail.com>
> Date: Wed, 3 Apr 2019 14:31:53 +0300
> 
> Does gdb use libintl?

If GDB was configure with NLS, yes.

But we were talking about gdbserver, I think, not about GDB.  And
gdbserver is not linked with libintl, AFAIK.


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-04-03 12:04 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-23 11:59 [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows Владимир Мартьянов
2019-03-23 13:11 ` Eli Zaretskii
2019-03-28 20:36   ` Владимир Мартьянов
2019-03-29  8:27     ` Eli Zaretskii
2019-03-29  8:42       ` Владимир Мартьянов
2019-03-29  9:29         ` Eli Zaretskii
2019-03-29  9:39           ` Владимир Мартьянов
2019-03-29 12:32             ` Eli Zaretskii
2019-03-30 21:26               ` Владимир Мартьянов
2019-03-31 14:45                 ` Eli Zaretskii
2019-03-31 19:39                   ` Владимир Мартьянов
2019-04-01  4:47                     ` Eli Zaretskii
2019-04-02 20:08                       ` Владимир Мартьянов
2019-04-03  4:30                         ` Eli Zaretskii
2019-04-01 22:08                     ` Jon Turney
2019-04-02 20:57                       ` Владимир Мартьянов
2019-04-02 21:10                         ` Jon Turney
2019-04-02 21:18                           ` Владимир Мартьянов
2019-04-03  4:48                         ` Eli Zaretskii
2019-04-03  4:58                           ` LRN
2019-04-03  7:37                             ` Eli Zaretskii
2019-04-03 11:32                               ` LRN
2019-04-03 12:04                                 ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox