Re: VS 2015 support in src/tools/msvc

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