On 10/04/16 20:47, Christian Ullrich wrote:
* Andrew Dunstan:

On 04/09/2016 08:43 AM, Christian Ullrich wrote:

Michael Paquier wrote:

I don't think that's good to use malloc in those code paths, and I

Oh?

+#if (_MSC_VER >= 1900)
+    uint32    cp;
+
+    if (GetLocaleInfoEx(ctype,
+            LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
+            (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
+    {
+        r = malloc(16);            /* excess */
+        if (r != NULL)
+            sprintf(r, "CP%u", cp);
+    }
+    else
+    {
+#endif


But r is return value so it has to be allocated. The intermediate variables are function local.

don't think we need to be too precious about saving a byte or two
here. Can one or other of you please prepare a replacement patch based
in this discussion?

Sorry, I don't think I'm up to that (at least not for another week or
so). I have to read up on the issue first; right now I'm not sure what
exactly the problem is.

What I can say is that the existing patch needs more work, because
GetLocaleInfoEx() expects a locale name ("de-DE") as its first argument,
but the patched win32_langinfo() is giving it a locale identifier
("German_Germany.1252"). At least it does for me.

That really depends on what you set in config/commandline. The current code supports both (that's why there is the else fallback to old code which handles the "German_Germany.1252" format).

As for missing code page information in the _locale_t type, ISTM it
isn't hidden any better in UCRT than it was before:

int main()
{
     /* Set locale with nonstandard code page */
     _locale_t loc = _create_locale(LC_ALL, "German_Germany.850");

     __crt_locale_data_public* pub =
(__crt_locale_data_public*)(loc->locinfo);
     printf("CP: %d\n", pub->_locale_lc_codepage);    // "CP: 850"
     return 0;
}


This is basically same as the approach of fixing _locale_t that was also considered upthread but nobody was too happy with it. I guess the worry is that given that the header file is obviously unfinished in the area where this is defined and also the fact that this looks like something that's not meant to be used outside of that header makes people worry that it might change between point releases of the SDK.

--
  Petr Jelinek                  http://www.2ndQuadrant.com/
  PostgreSQL Development, 24x7 Support, Training & Services


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to