Gerrit0 commented on code in PR #3167:
URL: https://github.com/apache/avro/pull/3167#discussion_r1770738406
##########
lang/c++/test/SchemaTests.cc:
##########
@@ -315,11 +436,13 @@ static void testRoundTrip(const char *schema) {
compiledSchema.toJson(os);
std::string result = os.str();
result.erase(std::remove_if(result.begin(), result.end(), ::isspace),
result.end()); // Remove whitespace
- BOOST_CHECK(result == std::string(schema));
+ std::string trimmedSchema = schema;
+ trimmedSchema.erase(std::remove_if(trimmedSchema.begin(),
trimmedSchema.end(), ::isspace), trimmedSchema.end());
Review Comment:
Every time I see someone do something like this I worry...
> [std::isspace](https://en.cppreference.com/w/cpp/string/byte/isspace)
> The behavior is undefined if the value of ch is not representable as
unsigned char and is not equal to [EOF](http://en.cppreference.com/w/cpp/io/c).
In this case it is (mostly) okay as the schemas all have chars which meet
these requirements, but if `toJson` ever has a bug which doesn't meet this
requirement, we now have undefined behavior.
```cpp
std::string trimmedSchema = schema;
boost::algorithm::replace_all(trimmedSchema, " ", "");
boost::algorithm::replace_all(trimmedSchema, "\n", "");
```
... and now that I've written that, I see you just followed the bad example
above...
--
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]
