8000 Fix tolower()/toupper() implementation to not lead to undefined behavior by rouault · Pull Request #4539 · OSGeo/PROJ · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix tolower()/toupper() implementation to not lead to undefined behavior #4539

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Jul 4, 2025
Merged

Fix tolower()/toupper() implementation to not lead to undefined behavior #4539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f43c123
Fix tolower()/toupper() implementation to not lead to undefined behavior
rouault Jul 3, 2025
127ba13
Avoid undefined behavior with isgraph() use
rouault Jul 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/dmstor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@ double dmstor_ctx(PJ_CONTEXT *ctx, const char *is, char **rs) {
* It is possible that a really odd input (like lots of leading zeros)
* could be truncated in copying into work. But ...
*/
while ((isgraph(*p) || *p == DEG_SIGN1 || *p == DEG_SIGN2) && --n)
while ((isgraph(static_cast<unsigned char>(*p)) || *p == DEG_SIGN1 ||
*p == DEG_SIGN2) &&
--n)
*s++ = *p++;
*s = '\0';
int sign = *(s = work);
Expand Down
10 changes: 6 additions & 4 deletions src/iso19111/internal.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,9 @@ std::string tolower(const std::string &str)

{
std::string ret(str);
for (size_t i = 0; i < ret.size(); i++)
ret[i] = static_cast<char>(::tolower(ret[i]));
for (char &ch : ret)
ch =
(ch >= 'A' && ch <= 'Z') ? static_cast<char>(ch + ('a' - 'A')) : ch;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will likely fail with EBCDIC but probably that's ok :-)

But more seriously, I wonder why the cast to unsigned char is bad. It allows ctype(3) implementations that use lookup tables and are thus fast, in general. (I get it that this isn't used much and that its performance isn't important.)

I don't object, though, and will test this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that use lookup tables and are thus fast,

I'm quite skeptical that lookup based code would be faster than branch testing. Modern processors are much better at doing branch prediction than fetching bytes from slow memory. Anyway this implementation or the alternate can only deal with ASCII, and that's fine given its intended purpose.

return ret;
}

Expand All @@ -144,8 +145,9 @@ std::string toupper(const std::string &str)

{
std::string ret(str);
for (size_t i = 0; i < ret.size(); i++)
ret[i] = static_cast<char>(::toupper(ret[i]));
for (char &ch : ret)
ch =
(ch >= 'a' && ch <= 'z') ? static_cast<char>(ch - ('a' - 'A')) : ch;
return ret;
}

Expand Down
0