Skip to content

Commit

Permalink
ARROW-14247: [C++] Fix Valgrind errors in parquet-arrow-test
Browse files Browse the repository at this point in the history
Googletest prints all test parameter objects up front. When there's no custom printer defined, it just prints the bytes of the struct. This happened to go into uninitialized memory in (apparently) std::string, tripping Valgrind.

See a discussion by Chromium developers on a similar issue: https://bugs.chromium.org/p/chromium/issues/detail?id=64887

Closes #11356 from lidavidm/arrow-14247

Authored-by: David Li <li.davidm96@gmail.com>
Signed-off-by: Antoine Pitrou <antoine@python.org>
  • Loading branch information
lidavidm authored and pitrou committed Oct 7, 2021
1 parent f92e1d1 commit 3cacc85
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 1 deletion.
2 changes: 1 addition & 1 deletion cpp/src/parquet/arrow/arrow_schema_test.cc
Expand Up @@ -1338,7 +1338,7 @@ TEST(TestFromParquetSchema, CorruptMetadata) {
::arrow::Result<std::deque<LevelInfo>> RootToTreeLeafLevels(
const SchemaManifest& manifest, int column_number) {
std::deque<LevelInfo> out;
const SchemaField* field;
const SchemaField* field = nullptr;
RETURN_NOT_OK(manifest.GetColumnField(column_number, &field));
while (field != nullptr) {
out.push_front(field->level_info);
Expand Down
10 changes: 10 additions & 0 deletions cpp/src/parquet/arrow/arrow_statistics_test.cc
Expand Up @@ -48,6 +48,16 @@ struct StatisticsTestParam {
std::string expected_max;
};

// Define a custom print since the default Googletest print trips Valgrind
void PrintTo(const StatisticsTestParam& param, std::ostream* os) {
(*os) << "StatisticsTestParam{"
<< "table.schema=" << param.table->schema()->ToString()
<< ", expected_null_count=" << param.expected_null_count
<< ", expected_value_count=" << param.expected_value_count
<< ", expected_min=" << param.expected_min
<< ", expected_max=" << param.expected_max << "}";
}

class ParameterizedStatisticsTest : public ::testing::TestWithParam<StatisticsTestParam> {
};

Expand Down

0 comments on commit 3cacc85

Please sign in to comment.