From 3c3df7402f8e4a95da6f417cb8545e500659f07a Mon Sep 17 00:00:00 2001 From: Kosie van der Merwe Date: Thu, 17 Jan 2013 10:04:45 -0800 Subject: [PATCH] Fixed issues Valgrind found. Summary: Found issues with `db_test` and `db_stress` when running valgrind. `DBImpl` had an issue where if an compaction failed then it will use the uninitialised file size of an output file is used. This manifested as the final call to output to the log in `DoCompactionWork()` branching on uninitialized memory (all the way down in printf's innards). Test Plan: Ran `valgrind --track_origins=yes ./db_test` and `valgrind ./db_stress` to see if issues disappeared. Ran `make check` to see if there were no regressions. Reviewers: vamsi, dhruba Reviewed By: dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D8001 --- db/db_impl.cc | 11 +++++++++-- tools/db_stress.cc | 10 +++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/db/db_impl.cc b/db/db_impl.cc index c22276675e..d95481079e 100644 --- a/db/db_impl.cc +++ b/db/db_impl.cc @@ -1717,7 +1717,14 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) { stats.files_in_leveln = compact->compaction->num_input_files(0); stats.files_in_levelnp1 = compact->compaction->num_input_files(1); - stats.files_out_levelnp1 = compact->outputs.size(); + + int num_output_files = compact->outputs.size(); + if (compact->builder != NULL) { + // An error occured so ignore the last output. + assert(num_output_files > 0); + --num_output_files; + } + stats.files_out_levelnp1 = num_output_files; for (int i = 0; i < compact->compaction->num_input_files(0); i++) stats.bytes_readn += compact->compaction->input(0, i)->file_size; @@ -1725,7 +1732,7 @@ Status DBImpl::DoCompactionWork(CompactionState* compact) { for (int i = 0; i < compact->compaction->num_input_files(1); i++) stats.bytes_readnp1 += compact->compaction->input(1, i)->file_size; - for (size_t i = 0; i < compact->outputs.size(); i++) { + for (int i = 0; i < num_output_files; i++) { stats.bytes_written += compact->outputs[i].file_size; } diff --git a/tools/db_stress.cc b/tools/db_stress.cc index 69c578079a..02934c85e3 100644 --- a/tools/db_stress.cc +++ b/tools/db_stress.cc @@ -678,19 +678,19 @@ class StressTest { fprintf(stdout, "Num keys per lock : %d\n", 1 << FLAGS_log2_keys_per_lock); - char* compression = (char *)std::string("").c_str(); + const char* compression = ""; switch (FLAGS_compression_type) { case leveldb::kNoCompression: - compression = (char *)std::string("none").c_str(); + compression = "none"; break; case leveldb::kSnappyCompression: - compression = (char *)std::string("snappy").c_str(); + compression = "snappy"; break; case leveldb::kZlibCompression: - compression = (char *)std::string("zlib").c_str(); + compression = "zlib"; break; case leveldb::kBZip2Compression: - compression = (char *)std::string("bzip2").c_str(); + compression = "bzip2"; break; }