lszskye commented on code in PR #181:
URL: https://github.com/apache/paimon-cpp/pull/181#discussion_r3719616191
##########
src/paimon/common/data/binary_string.cpp:
##########
@@ -75,16 +76,19 @@ std::string BinaryString::ToString() const {
}
int32_t BinaryString::NumBytesForFirstByte(char b) {
Review Comment:
This PR adds a new rule in docs/code-style.md: “do not rely on the
signedness of plain char”, and fixes NumBytesForFirstByte accordingly. However,
the case-conversion code in the same file still has this issue:
- :209 / :240: (*bytes)[0] = tolower(segment_and_offset.Value()); passes a
plain char directly to tolower. On x86_64, char is signed, so bytes >= 0x80
become negative. Passing such values to tolower / toupper is UB: the standard
requires the argument to be representable as unsigned char or be EOF. glibc
happens to tolerate -128..-1, so it does not crash. Note that this line is
executed before the ASCII check at :212, so non-ASCII input also goes through
it.
- :216 / :247: toupper(static_cast<int32_t>(b)) does not remove the negative
value; it only rewrites the same UB in a different form.
- :229 / :260: std::transform(str.begin(), str.end(), str.begin(),
::toupper) has the same issue, because dereferencing the iterator produces a
char.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
