kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From a...@apache.org
Subject [2/2] kudu git commit: Add a macro form for ScopedCleanup
Date Tue, 31 Oct 2017 01:04:33 GMT
Add a macro form for ScopedCleanup

This adds a new macro SCOPED_CLEANUP({ ... }) which is a shorter
form of 'auto cleanup = MakeScopedCleanup([&] { ... })

I also changed over a bunch of call sites to use it (those that were
easily discovered by a perl regex). We can switch over the remaining
ones as we run across them.

Change-Id: Idb72c27824def3c6a6d98c56b46cdd620f651e0e
Reviewed-on: http://gerrit.cloudera.org:8080/8416
Tested-by: Kudu Jenkins
Reviewed-by: Adar Dembo <adar@cloudera.com>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/563f41b5
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/563f41b5
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/563f41b5

Branch: refs/heads/master
Commit: 563f41b5cb2c3b490377d1c1cf6a26c24811d035
Parents: cb4b1c0
Author: Todd Lipcon <todd@apache.org>
Authored: Mon Oct 30 12:53:18 2017 -0700
Committer: Adar Dembo <adar@cloudera.com>
Committed: Tue Oct 31 01:04:11 2017 +0000

----------------------------------------------------------------------
 src/kudu/fs/log_block_manager.cc                    |  2 +-
 src/kudu/integration-tests/alter_table-test.cc      |  2 +-
 .../client-negotiation-failover-itest.cc            |  4 ++--
 .../raft_consensus_nonvoter-itest.cc                |  2 +-
 .../integration-tests/security-unknown-tsk-itest.cc |  2 +-
 .../tombstoned_voting-stress-test.cc                |  2 +-
 src/kudu/master/catalog_manager.cc                  |  2 +-
 src/kudu/security/ca/cert_management.cc             |  2 +-
 src/kudu/security/init.cc                           | 14 +++++++-------
 src/kudu/security/openssl_util.cc                   |  2 +-
 src/kudu/util/env-test.cc                           |  4 ++--
 src/kudu/util/maintenance_manager.cc                |  2 +-
 src/kudu/util/net/net_util.cc                       |  2 +-
 src/kudu/util/scoped_cleanup-test.cc                | 11 +++++++++++
 src/kudu/util/scoped_cleanup.h                      | 16 ++++++++++++++++
 src/kudu/util/threadpool-test.cc                    | 10 +++++-----
 16 files changed, 53 insertions(+), 26 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/fs/log_block_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/fs/log_block_manager.cc b/src/kudu/fs/log_block_manager.cc
index 2dc1a1a..41f9b3e 100644
--- a/src/kudu/fs/log_block_manager.cc
+++ b/src/kudu/fs/log_block_manager.cc
@@ -1482,7 +1482,7 @@ Status LogWritableBlock::Finalize() {
     return Status::OK();
   }
 
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     container_->FinalizeBlock(block_offset_, block_length_);
     state_ = FINALIZED;
   });

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/integration-tests/alter_table-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/alter_table-test.cc b/src/kudu/integration-tests/alter_table-test.cc
index 815ed34..f771209 100644
--- a/src/kudu/integration-tests/alter_table-test.cc
+++ b/src/kudu/integration-tests/alter_table-test.cc
@@ -2082,7 +2082,7 @@ TEST_F(AlterTableTest, TestRenameStillCreatingTable) {
              .num_replicas(1)
              .Create());
   });
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     creator_thread.join();
   });
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/integration-tests/client-negotiation-failover-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/client-negotiation-failover-itest.cc b/src/kudu/integration-tests/client-negotiation-failover-itest.cc
index 5ff646d..bc21e8a 100644
--- a/src/kudu/integration-tests/client-negotiation-failover-itest.cc
+++ b/src/kudu/integration-tests/client-negotiation-failover-itest.cc
@@ -173,7 +173,7 @@ TEST_F(ClientFailoverOnNegotiationTimeoutITest, Kudu1580ConnectToTServer)
{
       });
     // An automatic clean-up to handle both success and failure cases
     // in the code below.
-    auto cleanup = MakeScopedCleanup([&]() {
+    SCOPED_CLEANUP({
         resume_thread.join();
       });
 
@@ -268,7 +268,7 @@ TEST_F(ClientFailoverOnNegotiationTimeoutITest, Kudu2021NegotiateWithMaster)
{
       CHECK_OK(m->Pause())
     });
   // An automatic clean-up to handle both success and failure cases.
-  auto pause_thread_cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       pause_thread.join();
       CHECK_OK(m->Resume());
     });

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc b/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
index 6b68914..4722185 100644
--- a/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
+++ b/src/kudu/integration-tests/raft_consensus_nonvoter-itest.cc
@@ -504,7 +504,7 @@ TEST_F(RaftConsensusNonVoterITest, NonVoterReplicasDoNotVote) {
     ASSERT_OK(GetTermMetricValue(leader_ts, &term_leader));
 
     ASSERT_OK(leader_ts->Pause());
-    auto cleanup = MakeScopedCleanup([&]() {
+    SCOPED_CLEANUP({
       ASSERT_OK(leader_ts->Resume());
     });
     TServerDetails* new_leader;

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/integration-tests/security-unknown-tsk-itest.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/security-unknown-tsk-itest.cc b/src/kudu/integration-tests/security-unknown-tsk-itest.cc
index 2c13977..bf6416f 100644
--- a/src/kudu/integration-tests/security-unknown-tsk-itest.cc
+++ b/src/kudu/integration-tests/security-unknown-tsk-itest.cc
@@ -324,7 +324,7 @@ TEST_F(SecurityUnknownTskTest, ErrorUnavailableCommonOperations) {
     });
 
   // An automatic clean-up to handle failure cases in the code below.
-  auto importer_cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       importer.join();
     });
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
index f12ba59..ba71f8e 100644
--- a/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
+++ b/src/kudu/integration-tests/tombstoned_voting-stress-test.cc
@@ -272,7 +272,7 @@ TEST_F(TombstonedVotingStressTest, TestTombstonedVotingUnderStress) {
   // Startup the voting thread.
   LOG(INFO) << "starting stress thread...";
   thread voter_thread([this] { RunVoteRequestLoop(); });
-  auto cleanup = MakeScopedCleanup([&] {
+  SCOPED_CLEANUP({
     SetState(kTestComplete);
     voter_thread.join();
   });

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/master/catalog_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/catalog_manager.cc b/src/kudu/master/catalog_manager.cc
index 4f0a6d5..6702750 100644
--- a/src/kudu/master/catalog_manager.cc
+++ b/src/kudu/master/catalog_manager.cc
@@ -1359,7 +1359,7 @@ Status CatalogManager::CreateTable(const CreateTableRequestPB* orig_req,
   }
 
   // Ensure that we drop the name reservation upon return.
-  auto cleanup = MakeScopedCleanup([&] () {
+  SCOPED_CLEANUP({
     std::lock_guard<LockType> l(lock_);
     CHECK_EQ(1, reserved_table_names_.erase(req.name()));
   });

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/security/ca/cert_management.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/ca/cert_management.cc b/src/kudu/security/ca/cert_management.cc
index 3647e8c..5144216 100644
--- a/src/kudu/security/ca/cert_management.cc
+++ b/src/kudu/security/ca/cert_management.cc
@@ -315,7 +315,7 @@ Status CertSigner::CopyExtensions(X509_REQ* req, X509* x) {
   CHECK(req);
   CHECK(x);
   STACK_OF(X509_EXTENSION)* exts = X509_REQ_get_extensions(req);
-  auto exts_cleanup = MakeScopedCleanup([&exts]() {
+  SCOPED_CLEANUP({
     sk_X509_EXTENSION_pop_free(exts, X509_EXTENSION_free);
   });
   for (size_t i = 0; i < sk_X509_EXTENSION_num(exts); ++i) {

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/security/init.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/init.cc b/src/kudu/security/init.cc
index 47c2c42..6ce2135 100644
--- a/src/kudu/security/init.cc
+++ b/src/kudu/security/init.cc
@@ -213,7 +213,7 @@ Status Krb5UnparseName(krb5_principal princ, string* name) {
   char* c_name;
   KRB5_RETURN_NOT_OK_PREPEND(krb5_unparse_name(g_krb5_ctx, princ, &c_name),
                              "krb5_unparse_name");
-  auto cleanup_name = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       krb5_free_unparsed_name(g_krb5_ctx, c_name);
     });
   *name = c_name;
@@ -278,7 +278,7 @@ Status KinitContext::DoRenewal() {
   // Setup a cursor to iterate through the credential cache.
   KRB5_RETURN_NOT_OK_PREPEND(krb5_cc_start_seq_get(g_krb5_ctx, ccache_, &cursor),
                              "Failed to peek into ccache");
-  auto cleanup_cursor = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       krb5_cc_end_seq_get(g_krb5_ctx, ccache_, &cursor); });
 
   krb5_creds creds;
@@ -287,7 +287,7 @@ Status KinitContext::DoRenewal() {
   krb5_error_code rc;
   // Iterate through the credential cache.
   while (!(rc = krb5_cc_next_cred(g_krb5_ctx, ccache_, &cursor, &creds))) {
-    auto cleanup_creds = MakeScopedCleanup([&]() {
+    SCOPED_CLEANUP({
         krb5_free_cred_contents(g_krb5_ctx, &creds); });
     if (krb5_is_config_principal(g_krb5_ctx, creds.server)) continue;
 
@@ -302,7 +302,7 @@ Status KinitContext::DoRenewal() {
 
     krb5_creds new_creds;
     memset(&new_creds, 0, sizeof(krb5_creds));
-    auto cleanup_new_creds = MakeScopedCleanup([&]() {
+    SCOPED_CLEANUP({
         krb5_free_cred_contents(g_krb5_ctx, &new_creds); });
     // Acquire a new ticket using the keytab. This ticket will automatically be put into
the
     // credential cache.
@@ -356,7 +356,7 @@ Status KinitContext::Kinit(const string& keytab_path, const string&
principal) {
                                                         0 /* valid from now */,
                                                         nullptr /* TKT service name */, opts_),
                              "unable to login from keytab");
-  auto cleanup_creds = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       krb5_free_cred_contents(g_krb5_ctx, &creds); });
 
   ticket_end_timestamp_ = creds.times.endtime;
@@ -413,7 +413,7 @@ Status CanonicalizeKrb5Principal(std::string* principal) {
   krb5_principal princ;
   KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(g_krb5_ctx, principal->c_str(), &princ),
                              "could not parse principal");
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       krb5_free_principal(g_krb5_ctx, princ);
     });
   RETURN_NOT_OK_PREPEND(Krb5UnparseName(princ, principal),
@@ -426,7 +426,7 @@ Status MapPrincipalToLocalName(const std::string& principal, std::string*
local_
   krb5_principal princ;
   KRB5_RETURN_NOT_OK_PREPEND(krb5_parse_name(g_krb5_ctx, principal.c_str(), &princ),
                              "could not parse principal");
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       krb5_free_principal(g_krb5_ctx, princ);
     });
   char buf[1024];

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/security/openssl_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/security/openssl_util.cc b/src/kudu/security/openssl_util.cc
index 98ee245..3309ca6 100644
--- a/src/kudu/security/openssl_util.cc
+++ b/src/kudu/security/openssl_util.cc
@@ -146,7 +146,7 @@ STACK_OF(X509)* PEM_read_STACK_OF_X509(BIO* bio, void* /* unused */, pem_passwor
   // Extract information from the chain certificate.
   STACK_OF(X509_INFO)* info = PEM_X509_INFO_read_bio(bio, nullptr, nullptr, nullptr);
   if (!info) return nullptr;
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     sk_X509_INFO_pop_free(info, X509_INFO_free);
   });
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/env-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 63c0e20..c152b87 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -747,7 +747,7 @@ TEST_F(TestEnv, TestWalkBadPermissions) {
   struct stat stat_buf;
   PCHECK(stat(kTestPath.c_str(), &stat_buf) == 0);
   PCHECK(chmod(kTestPath.c_str(), 0000) == 0);
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     // Restore the old permissions so the path can be successfully deleted.
     PCHECK(chmod(kTestPath.c_str(), stat_buf.st_mode) == 0);
   });
@@ -812,7 +812,7 @@ TEST_F(TestEnv, TestGlobPermissionDenied) {
   string dir = GetTestPath("glob");
   ASSERT_OK(env_->CreateDir(dir));
   chmod(dir.c_str(), 0000);
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
       chmod(dir.c_str(), 0700);
     });
   vector<string> matches;

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/maintenance_manager.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/maintenance_manager.cc b/src/kudu/util/maintenance_manager.cc
index cf5bbf9..8984de8 100644
--- a/src/kudu/util/maintenance_manager.cc
+++ b/src/kudu/util/maintenance_manager.cc
@@ -462,7 +462,7 @@ void MaintenanceManager::LaunchOp(MaintenanceOp* op) {
     InsertOrDie(&running_instances_, thread_id, &op_instance);
   }
 
-  auto cleanup = MakeScopedCleanup([&] {
+  SCOPED_CLEANUP({
     op->RunningGauge()->Decrement();
 
     std::lock_guard<Mutex> l(lock_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/net/net_util.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/net/net_util.cc b/src/kudu/util/net/net_util.cc
index 3f9273b..520882f 100644
--- a/src/kudu/util/net/net_util.cc
+++ b/src/kudu/util/net/net_util.cc
@@ -290,7 +290,7 @@ Status GetLocalNetworks(std::vector<Network>* net) {
   struct ifaddrs *ifap = nullptr;
 
   int ret = getifaddrs(&ifap);
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     if (ifap) freeifaddrs(ifap);
   });
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/scoped_cleanup-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/scoped_cleanup-test.cc b/src/kudu/util/scoped_cleanup-test.cc
index fad50a0..2e77705 100644
--- a/src/kudu/util/scoped_cleanup-test.cc
+++ b/src/kudu/util/scoped_cleanup-test.cc
@@ -31,6 +31,17 @@ TEST(ScopedCleanup, TestCleanup) {
   ASSERT_EQ(0, var);
 }
 
+TEST(ScopedCleanup, TestCleanupMacro) {
+  int var = 0;
+  {
+    auto saved = var;
+    SCOPED_CLEANUP({ var = saved; });
+    var = 42;
+  }
+  ASSERT_EQ(0, var);
+}
+
+
 TEST(ScopedCleanup, TestCancelCleanup) {
   int var = 0;
   {

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/scoped_cleanup.h
----------------------------------------------------------------------
diff --git a/src/kudu/util/scoped_cleanup.h b/src/kudu/util/scoped_cleanup.h
index e989331..8ecfbcb 100644
--- a/src/kudu/util/scoped_cleanup.h
+++ b/src/kudu/util/scoped_cleanup.h
@@ -19,10 +19,26 @@
 
 #include <utility>
 
+#include "kudu/gutil/macros.h"
+
+// Run the given function body (which is typically a block of code surrounded by
+// curly-braces) when the current scope exits.
+//
+// Example:
+//   int fd = open(...);
+//   SCOPED_CLEANUP({ close(fd); });
+//
+// NOTE: in the case that you want to cancel the cleanup, use the more verbose
+// (non-macro) form below.
+#define SCOPED_CLEANUP(func_body) \
+  auto VARNAME_LINENUM(scoped_cleanup) = MakeScopedCleanup([&] { func_body });
+
 namespace kudu {
 
 // A scoped object which runs a cleanup function when going out of scope. Can
 // be used for scoped resource cleanup.
+//
+// Use 'MakeScopedCleanup()' below to instantiate.
 template<typename F>
 class ScopedCleanup {
  public:

http://git-wip-us.apache.org/repos/asf/kudu/blob/563f41b5/src/kudu/util/threadpool-test.cc
----------------------------------------------------------------------
diff --git a/src/kudu/util/threadpool-test.cc b/src/kudu/util/threadpool-test.cc
index c3d5c94..b92407b 100644
--- a/src/kudu/util/threadpool-test.cc
+++ b/src/kudu/util/threadpool-test.cc
@@ -192,7 +192,7 @@ TEST_F(ThreadPoolTest, TestThreadPoolWithNoMinimum) {
   ASSERT_TRUE(pool_->num_threads() == 0);
   // We get up to 3 threads when submitting work.
   CountDownLatch latch(1);
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     latch.CountDown();
   });
   ASSERT_OK(pool_->Submit(SlowTask::NewSlowTask(&latch)));
@@ -565,7 +565,7 @@ TEST_P(ThreadPoolTestTokenTypes, TestTokenSubmitsProcessedConcurrently)
{
   // A violation to the tested invariant would yield a deadlock, so let's set
   // up an alarm to bail us out.
   alarm(60);
-  auto cleanup = MakeScopedCleanup([&] {
+  SCOPED_CLEANUP({
       alarm(0); // Disable alarm on test exit.
   });
   shared_ptr<Barrier> b = std::make_shared<Barrier>(kNumTokens + 1);
@@ -588,7 +588,7 @@ TEST_F(ThreadPoolTest, TestTokenSubmitsNonSequential) {
   // A violation to the tested invariant would yield a deadlock, so let's set
   // up an alarm to bail us out.
   alarm(60);
-  auto cleanup = MakeScopedCleanup([&] {
+  SCOPED_CLEANUP({
       alarm(0); // Disable alarm on test exit.
   });
   shared_ptr<Barrier> b = std::make_shared<Barrier>(kNumSubmissions + 1);
@@ -615,7 +615,7 @@ TEST_P(ThreadPoolTestTokenTypes, TestTokenShutdown) {
   // A violation to the tested invariant would yield a deadlock, so let's set
   // up an alarm to bail us out.
   alarm(60);
-  auto cleanup = MakeScopedCleanup([&] {
+  SCOPED_CLEANUP({
       alarm(0); // Disable alarm on test exit.
   });
 
@@ -764,7 +764,7 @@ TEST_P(ThreadPoolTestTokenTypes, TestTokenSubmissionsAdhereToMaxQueueSize)
{
 
   CountDownLatch latch(1);
   unique_ptr<ThreadPoolToken> t = pool_->NewToken(GetParam());
-  auto cleanup = MakeScopedCleanup([&]() {
+  SCOPED_CLEANUP({
     latch.CountDown();
   });
   // We will be able to submit two tasks: one for max_threads == 1 and one for


Mime
View raw message