Обсуждение: Re: Refactoring identifier checks to consistently usestrcmp
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
> 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