From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 110646 invoked by alias); 31 Mar 2019 14:45:55 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 110538 invoked by uid 89); 31 Mar 2019 14:45:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=AWL,BAYES_40,BODY_8BITS,GARBLED_BODY,SPF_PASS,URIBL_BLOCKED autolearn=no version=3.3.1 spammy=HContent-type:plain, H*f:sk:CAL5iTP, H*f:sk:K_aE4fk, H*f:CAL5iTP X-HELO: eggs.gnu.org Received: from eggs.gnu.org (HELO eggs.gnu.org) (209.51.188.92) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sun, 31 Mar 2019 14:45:46 +0000 Received: from fencepost.gnu.org ([2001:470:142:3::e]:56874) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hAbiS-0004XX-Cn; Sun, 31 Mar 2019 10:45:44 -0400 Received: from [176.228.60.248] (port=1361 helo=home-c4e4a596f7) by fencepost.gnu.org with esmtpsa (TLS1.2:RSA_AES_256_CBC_SHA1:256) (Exim 4.82) (envelope-from ) id 1hAbiR-0004MS-Bw; Sun, 31 Mar 2019 10:45:43 -0400 Date: Sun, 31 Mar 2019 14:45:00 -0000 Message-Id: <8336n3hzkx.fsf@gnu.org> From: Eli Zaretskii To: =?utf-8?B?0JLQu9Cw0LTQuNC80LjRgCDQnNCw0YDRgtGM0Y/QvdC+0LI=?= CC: gdb-patches@sourceware.org In-reply-to: (message from =?utf-8?B?0JLQu9Cw0LTQuNC80LjRgCDQnNCw0YDRgtGM0Y/QvdC+0LI=?= on Sun, 31 Mar 2019 00:26:30 +0300) Subject: Re: [PATCH][PR server/24377] Fix mixing English and system default languages in error messages on Windows References: <83mullpwg6.fsf@gnu.org> <83ef6qjdbb.fsf@gnu.org> <838swyjafd.fsf@gnu.org> <837echkgjo.fsf@gnu.org> MIME-version: 1.0 Content-type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-IsSubscribed: yes X-SW-Source: 2019-03/txt/msg00776.txt.bz2 > From: Владимир Мартьянов > 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.