Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: don't use locale-sensitive strcasecmp() (v4.x) #7660

Closed

Conversation

bnoordhuis
Copy link
Member

@bnoordhuis bnoordhuis commented Jul 11, 2016

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jul 11, 2016
@addaleax addaleax added the v4.x label Jul 11, 2016
@MylesBorins MylesBorins self-assigned this Jul 11, 2016
@MylesBorins
Copy link
Contributor

@bnoordhuis can you rebase against v4.x-staging one more time... had to rebase in a missing commit. Should be close enough a tree that the CI run is still relevant

strcasecmp() is not used in src/node_http_parser.cc so there is no need
to include its header file.

PR-URL: nodejs#6582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
strcasecmp() is affected by the current locale as configured through
e.g. the LC_ALL environment variable and the setlocale() libc function.

It can result in unpredictable results across systems so replace it with
a function that isn't susceptible to that.

PR-URL: nodejs#6582
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bnoordhuis
Copy link
Member Author

@thealphanerd Done. New CI: https://ci.nodejs.org/job/node-test-pull-request/3255/

@MylesBorins
Copy link
Contributor

landed in 583a80d...a881986

@MylesBorins MylesBorins removed their assignment Dec 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants