From: | Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> |
---|---|
To: | davinder singh <davindersingh2692(at)gmail(dot)com> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PG compilation error with Visual Studio 2015/2017/2019 |
Date: | 2020-04-15 14:45:40 |
Message-ID: | CAEudQAqohr5ij4eWQatcKRyygQbQGcvjjyD0d_-nr-tKX5MXdg@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Em qua., 15 de abr. de 2020 às 03:08, davinder singh <
davindersingh2692(at)gmail(dot)com> escreveu:
>
> Thanks for the review comments.
>
> On Tue, Apr 14, 2020 at 9:12 PM Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
>
>> >>I m still working on testing this patch. If anyone has Idea please
>> suggest.
>> I still see problems with this patch.
>>
>> 1. Variable loct have redundant initialization, it would be enough to
>> declare so: _locale_t loct;
>> 2. Style white space in variable rc declaration.
>> 3. Style variable cp_index can be reduced.
>> if (tmp != NULL) {
>> size_t cp_index;
>>
>> cp_index = (size_t)(tmp - winlocname);
>> strncpy(loc_name, winlocname, cp_index);
>> loc_name[cp_index] = '\0';
>> 4. Memory leak if _WIN32_WINNT >= 0x0600 is true, _free_locale(loct); is
>> not called.
>>
> I resolved the above comments.
>
Ok, thanks.
>
>> 5. Why call _create_locale if _WIN32_WINNT >= 0x0600 is true and loct is
>> not used?
>>
> _create_locale can take bigger input than GetLocaleInfoEx. But we are
> interested in
> *language[_country-region[.code-page]]*. We are using _create_locale to
> validate
> the given input. The reason is we can't verify the locale name if it is
> appended with
> code-page by using GetLocaleInfoEx. So before parsing, we verify if the
> whole input
> locale name is valid by using _create_locale. I hope that answers your
> question.
>
Understood. In this case, _create_locale, is being used only to validate
the input.
Perhaps, in addition, you could create an additional function, which only
validates winlocname, without having to create structures or use malloc, to
be used when _WIN32_WINNT> = 0x0600 is true, but it is only a suggestion,
if you think it is necessary.
But have a last problem, in case GetLocaleInfoEx fail, there is still one
memory leak,
before return NULL is needed call: _free_locale(loct);
Another suggestion, if GetLocaleInfoEx fail wouldn't it be good to log the
error and the error number?
regards,
Ranier Vilela
From | Date | Subject | |
---|---|---|---|
Next Message | Tomas Vondra | 2020-04-15 14:47:12 | Re: sqlsmith crash incremental sort |
Previous Message | Robert Haas | 2020-04-15 14:38:24 | Re: wrong relkind error messages |