Re: VS 2015 support in src/tools/msvc - Mailing list pgsql-hackers

From Petr Jelinek
Subject Re: VS 2015 support in src/tools/msvc
Date
Msg-id 570ABFA0.4090403@2ndquadrant.com
Whole thread Raw
In response to Re: VS 2015 support in src/tools/msvc  (Christian Ullrich <chris@chrullrich.net>)
Responses Re: VS 2015 support in src/tools/msvc  (Michael Paquier <michael.paquier@gmail.com>)
List pgsql-hackers
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



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Move PinBuffer and UnpinBuffer to atomics
Next
From: Tom Lane
Date:
Subject: Weird irreproducible behaviors in plpython tests