Skip to content

Commit

Permalink
Merge #41235
Browse files Browse the repository at this point in the history
41235: libroach: fix memory issues in tests r=ajkr a=ajkr

msan and asan identified these issues.

Release justification: test cases should be runnable under sanitizers

Release note: None

Co-authored-by: Andrew Kryczka <andrew.kryczka2@gmail.com>
  • Loading branch information
craig[bot] and ajkr committed Oct 1, 2019
2 parents 5f1b177 + 9f407c3 commit cbbf40a
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 4 deletions.
15 changes: 13 additions & 2 deletions c-deps/libroach/ccl/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ TEST_F(CCLTest, DBOpen) {
"Invalid argument: encryption was used on this store before, but no encryption "
"flags specified. You need a CCL build and must fully specify the "
"--enterprise-encryption flag");
free(ret.data);
ASSERT_OK(rocksdb::Env::Default()->DeleteFile(dir.Path(kFileRegistryFilename)));
}

Expand Down Expand Up @@ -116,6 +117,7 @@ TEST_F(CCLTest, DBOpen) {
enginepbccl::EncryptionStatus enc_status;
ASSERT_TRUE(
enc_status.ParseFromArray(stats.encryption_status.data, stats.encryption_status.len));
free(stats.encryption_status.data);
EXPECT_STREQ(enc_status.active_store_key().key_id().c_str(), "plain");
EXPECT_STREQ(enc_status.active_data_key().key_id().c_str(), "plain");

Expand Down Expand Up @@ -146,6 +148,7 @@ TEST_F(CCLTest, ReadOnly) {
DBString value;
EXPECT_STREQ(DBGet(db, ToDBKey("foo"), &value).data, NULL);
EXPECT_STREQ(ToString(value).c_str(), "foo's value");
free(value.data);

DBClose(db);
}
Expand All @@ -161,9 +164,12 @@ TEST_F(CCLTest, ReadOnly) {
DBString ro_value;
EXPECT_STREQ(DBGet(db, ToDBKey("foo"), &ro_value).data, NULL);
EXPECT_STREQ(ToString(ro_value).c_str(), "foo's value");
free(ro_value.data);
// Try to write it again.
EXPECT_EQ(ToString(DBPut(db, ToDBKey("foo"), ToDBSlice("foo's value"))),
auto ret = DBPut(db, ToDBKey("foo"), ToDBSlice("foo's value"));
EXPECT_EQ(ToString(ret),
"Not implemented: Not supported operation in read only mode.");
free(ret.data);

DBClose(db);
}
Expand All @@ -183,9 +189,12 @@ TEST_F(CCLTest, ReadOnly) {
DBString ro_value;
EXPECT_STREQ(DBGet(db, ToDBKey("foo"), &ro_value).data, NULL);
EXPECT_STREQ(ToString(ro_value).c_str(), "foo's value");
free(ro_value.data);
// Try to write it again.
EXPECT_EQ(ToString(DBPut(db, ToDBKey("foo"), ToDBSlice("foo's value"))),
auto ret = DBPut(db, ToDBKey("foo"), ToDBSlice("foo's value"));
EXPECT_EQ(ToString(ret),
"Not implemented: Not supported operation in read only mode.");
free(ret.data);

DBClose(db);
}
Expand Down Expand Up @@ -328,9 +337,11 @@ TEST_F(CCLTest, EncryptionStats) {

enginepbccl::DataKeysRegistry key_registry;
ASSERT_TRUE(key_registry.ParseFromArray(result.key_registry.data, result.key_registry.len));
free(result.key_registry.data);

enginepb::FileRegistry file_registry;
ASSERT_TRUE(file_registry.ParseFromArray(result.file_registry.data, result.file_registry.len));
free(result.file_registry.data);

// Check some registry contents.
EXPECT_STRNE(key_registry.active_store_key_id().c_str(), "plain");
Expand Down
4 changes: 2 additions & 2 deletions c-deps/libroach/ccl/encrypted_env_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ namespace {

rocksdb::Status checkFileEntry(FileRegistry& registry, const std::string& filename,
enginepbccl::EncryptionType enc_type) {
auto entry = registry.GetFileEntry(filename).get();
auto entry = registry.GetFileEntry(filename);
if (entry == nullptr) {
return rocksdb::Status::InvalidArgument(
fmt::StringPrintf("file %s has no entry", filename.c_str()));
Expand All @@ -99,7 +99,7 @@ rocksdb::Status checkFileEntry(FileRegistry& registry, const std::string& filena
}

rocksdb::Status checkNoFileEntry(FileRegistry& registry, const std::string& filename) {
auto entry = registry.GetFileEntry(filename).get();
auto entry = registry.GetFileEntry(filename);
if (entry != nullptr) {
return rocksdb::Status::InvalidArgument(
fmt::StringPrintf("file %s has an unexpected entry", filename.c_str()));
Expand Down
2 changes: 2 additions & 0 deletions c-deps/libroach/db_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ TEST(Libroach, DBOpen) {
"Invalid argument: encryption was used on this store before, but no encryption "
"flags specified. You need a CCL build and must fully specify the "
"--enterprise-encryption flag");
free(ret.data);
ASSERT_OK(rocksdb::Env::Default()->DeleteFile(dir.Path(kFileRegistryFilename)));
}
{
Expand All @@ -98,6 +99,7 @@ TEST(Libroach, DBOpen) {
auto ret = DBOpen(&db, ToDBSlice(dir.Path("")), db_opts);
EXPECT_STREQ(std::string(ret.data, ret.len).c_str(),
"Invalid argument: encryption options are not supported in OSS builds");
free(ret.data);
ASSERT_OK(rocksdb::Env::Default()->DeleteFile(dir.Path(kFileRegistryFilename)));
}
}
Expand Down

0 comments on commit cbbf40a

Please sign in to comment.