Thread: Re: Refactoring identifier checks to consistently usestrcmp

Re: Refactoring identifier checks to consistently usestrcmp

From
Alvaro Herrera
Date:
Daniel Gustafsson wrote:
> Testing DefElem options is done with both strcmp() and pg_strcasecmp() a bit
> mixed.  Since the option defnames are all lowercased, either via IDENT, keyword
> rules or “by hand” with makeString(), using strcmp() is safe (and assumed to be
> so in quite a lot of places).
> 
> While it’s not incorrect per se to use pg_strcasecmp(), it has the potential to
> hide a DefElem created with a mixed-case defname where it in other places is
> expected to be in lowercase, which may lead to subtle bugs.
> 
> The attached patch refactors to use strcmp() consistently for option processing
> in the command code as a pre-emptive belts+suspenders move against such subtle
> bugs and to make the code more consistent.  Also reorders a few checks to have
> all in the same “format” and removes a comment related to the above.
> 
> Tested with randomizing case on options in make check (not included in patch).

Does it work correctly in the Turkish locale?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services



Re: Refactoring identifier checks to consistently use strcmp

From
Daniel Gustafsson
Date:
> On 04 Apr 2017, at 05:52, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> Daniel Gustafsson wrote:
>> Testing DefElem options is done with both strcmp() and pg_strcasecmp() a bit
>> mixed.  Since the option defnames are all lowercased, either via IDENT, keyword
>> rules or “by hand” with makeString(), using strcmp() is safe (and assumed to be
>> so in quite a lot of places).
>>
>> While it’s not incorrect per se to use pg_strcasecmp(), it has the potential to
>> hide a DefElem created with a mixed-case defname where it in other places is
>> expected to be in lowercase, which may lead to subtle bugs.
>>
>> The attached patch refactors to use strcmp() consistently for option processing
>> in the command code as a pre-emptive belts+suspenders move against such subtle
>> bugs and to make the code more consistent.  Also reorders a few checks to have
>> all in the same “format” and removes a comment related to the above.
>>
>> Tested with randomizing case on options in make check (not included in patch).
>
> Does it work correctly in the Turkish locale?

Yes, running make check with random case defnames under tr_TR works fine.

cheers ./daniel