-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
AVRO-4107: [C++] Remove boost::algorithm #3284
Conversation
lang/c++/impl/avrogencpp.cc
Outdated
while (start < end && std::isspace(buf[start])) start++; | ||
while (start < end && std::isspace(buf[end - 1])) end--; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If buf[start]
or buf[end - 1]
is a negative char
other than EOF
, then passing it to std::isspace(int ch)
causes undefined behavior. To avoid the risk, this should cast the char
values to unsigned char
.
(In principle, this might also be expected to recognize and trim multibyte whitespace characters as defined by the current locale. But it seems unlikely that such characters would actually be used in the preprocessor directives for which this function searches. And boost::algorithm::trim
doesn't seem to recognize them either, as it checks each char
individually.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it is better to call std::isspace(buf[start], std::locale::classic())
instead of std::isspace(static_cast<unsigned char>(buf[start]))
I think the version that @wgtmac implemented (which uses locale) is the closest to what boost implemented. Boost's trim also uses the current locale for determining if something is a space. |
What is the purpose of the change
Remove boost::algorithm
Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
Documentation