You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kudu.apache.org by ad...@apache.org on 2020/03/30 18:56:35 UTC
[kudu] 01/07: env: remove kudu::Bind usage from Walk
This is an automated email from the ASF dual-hosted git repository.
adar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
commit 08464561d95651470957ddeceb94a1ad3170df3d
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Fri Mar 27 23:49:07 2020 -0700
env: remove kudu::Bind usage from Walk
This isn't as elegant as the other patches in this series due to the number
of parameters in WalkCallback. C++14 allows the use of 'auto' for parameter
types which can shorten them somewhat.
Change-Id: I080809aef7e2f0ac3c240d4ca6909951b9615bb0
Reviewed-on: http://gerrit.cloudera.org:8080/15575
Tested-by: Kudu Jenkins
Reviewed-by: Alexey Serbin <as...@cloudera.com>
Reviewed-by: Andrew Wong <aw...@cloudera.com>
---
src/kudu/fs/block_manager-test.cc | 9 +++++----
src/kudu/fs/file_block_manager.cc | 14 +++++++++-----
src/kudu/util/env-test.cc | 18 ++++++++++++------
src/kudu/util/env.h | 10 +++-------
src/kudu/util/env_posix.cc | 22 +++++++++++++---------
src/kudu/util/env_util.cc | 8 ++++++--
src/kudu/util/env_util.h | 5 +----
src/kudu/util/metrics.h | 2 +-
8 files changed, 50 insertions(+), 38 deletions(-)
diff --git a/src/kudu/fs/block_manager-test.cc b/src/kudu/fs/block_manager-test.cc
index 88143d3..1aca697 100644
--- a/src/kudu/fs/block_manager-test.cc
+++ b/src/kudu/fs/block_manager-test.cc
@@ -43,7 +43,6 @@
#include "kudu/fs/fs_report.h"
#include "kudu/fs/log_block_manager.h"
#include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/casts.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/ref_counted.h"
@@ -224,9 +223,11 @@ class BlockManagerTest : public KuduTest {
// hierarchy, ignoring '.', '..', and file 'kInstanceMetadataFileName'.
Status CountFiles(const string& root, int* num_files) {
*num_files = 0;
- RETURN_NOT_OK(env_->Walk(root, Env::PRE_ORDER,
- Bind(BlockManagerTest::CountFilesCb, num_files)));
- return Status::OK();
+ return env_->Walk(
+ root, Env::PRE_ORDER,
+ [num_files](Env::FileType type, const string& dirname, const string& basename) {
+ return CountFilesCb(num_files, type, dirname, basename);
+ });
}
// Keep an internal copy of the data dir group to act as metadata.
diff --git a/src/kudu/fs/file_block_manager.cc b/src/kudu/fs/file_block_manager.cc
index 09b2a99..ae8f1bc 100644
--- a/src/kudu/fs/file_block_manager.cc
+++ b/src/kudu/fs/file_block_manager.cc
@@ -18,6 +18,7 @@
#include "kudu/fs/file_block_manager.h"
#include <cstddef>
+#include <functional>
#include <memory>
#include <mutex>
#include <numeric>
@@ -921,11 +922,14 @@ Status GetAllBlockIdsForDataDirCb(Dir* dd,
}
void GetAllBlockIdsForDir(Env* env,
- Dir* dd,
- vector<BlockId>* block_ids,
- Status* status) {
- *status = env->Walk(dd->dir(), Env::PRE_ORDER,
- Bind(&GetAllBlockIdsForDataDirCb, dd, block_ids));
+ Dir* dd,
+ vector<BlockId>* block_ids,
+ Status* status) {
+ *status = env->Walk(
+ dd->dir(), Env::PRE_ORDER,
+ [dd, block_ids](Env::FileType type, const string& dirname, const string& basename) {
+ return GetAllBlockIdsForDataDirCb(dd, block_ids, type, dirname, basename);
+ });
}
} // anonymous namespace
diff --git a/src/kudu/util/env-test.cc b/src/kudu/util/env-test.cc
index 6dda618..3eb4253 100644
--- a/src/kudu/util/env-test.cc
+++ b/src/kudu/util/env-test.cc
@@ -52,7 +52,6 @@
#include <glog/stl_logging.h> // IWYU pragma: keep
#include <gtest/gtest.h>
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/port.h"
@@ -752,13 +751,17 @@ TEST_F(TestEnv, TestWalk) {
// Do the walk.
unordered_set<string> actual;
- ASSERT_OK(env_->Walk(root, Env::PRE_ORDER, Bind(&TestWalkCb, &actual)));
+ ASSERT_OK(env_->Walk(
+ root, Env::PRE_ORDER,
+ [&actual](Env::FileType type, const string& dirname, const string& basename) {
+ return TestWalkCb(&actual, type, dirname, basename);
+ }));
ASSERT_EQ(expected, actual);
}
TEST_F(TestEnv, TestWalkNonExistentPath) {
// A walk on a non-existent path should fail.
- Status s = env_->Walk("/not/a/real/path", Env::PRE_ORDER, Bind(&NoopTestWalkCb));
+ Status s = env_->Walk("/not/a/real/path", Env::PRE_ORDER, &NoopTestWalkCb);
ASSERT_TRUE(s.IsIOError());
ASSERT_STR_CONTAINS(s.ToString(), "One or more errors occurred");
}
@@ -777,7 +780,7 @@ TEST_F(TestEnv, TestWalkBadPermissions) {
// A walk on a directory without execute permission should fail,
// unless the calling process has super-user's effective ID.
- Status s = env_->Walk(kTestPath, Env::PRE_ORDER, Bind(&NoopTestWalkCb));
+ Status s = env_->Walk(kTestPath, Env::PRE_ORDER, &NoopTestWalkCb);
if (geteuid() == 0) {
ASSERT_TRUE(s.ok()) << s.ToString();
} else {
@@ -801,8 +804,11 @@ TEST_F(TestEnv, TestWalkCbReturnsError) {
unique_ptr<WritableFile> writer;
ASSERT_OK(env_->NewWritableFile(JoinPathSegments(new_dir, new_file), &writer));
int num_calls = 0;
- ASSERT_TRUE(env_->Walk(new_dir, Env::PRE_ORDER,
- Bind(&TestWalkErrorCb, &num_calls)).IsIOError());
+ ASSERT_TRUE(env_->Walk(
+ new_dir, Env::PRE_ORDER,
+ [&num_calls](Env::FileType type, const string& dirname, const string& basename) {
+ return TestWalkErrorCb(&num_calls, type, dirname, basename);
+ }).IsIOError());
// Once for the directory and once for the file inside it.
ASSERT_EQ(2, num_calls);
diff --git a/src/kudu/util/env.h b/src/kudu/util/env.h
index 9a2d7bc..7c57bb3 100644
--- a/src/kudu/util/env.h
+++ b/src/kudu/util/env.h
@@ -9,19 +9,17 @@
//
// All Env implementations are safe for concurrent access from
// multiple threads without any external synchronization.
-
-#ifndef STORAGE_LEVELDB_INCLUDE_ENV_H_
-#define STORAGE_LEVELDB_INCLUDE_ENV_H_
+#pragma once
#include <cstddef>
#include <cstdint>
+#include <functional>
#include <iosfwd>
#include <map>
#include <memory>
#include <string>
#include <vector>
-#include "kudu/gutil/callback_forward.h"
#include "kudu/gutil/macros.h"
#include "kudu/util/status.h"
@@ -278,7 +276,7 @@ class Env {
//
// Returning an error won't halt the walk, but it will cause it to return
// with an error status when it's done.
- typedef Callback<Status(FileType, const std::string&, const std::string&)> WalkCallback;
+ typedef std::function<Status(FileType, const std::string&, const std::string&)> WalkCallback;
// Whether to walk directories in pre-order or post-order.
enum DirectoryOrder {
@@ -702,5 +700,3 @@ extern Status ReadFileToString(Env* env, const std::string& fname,
std::ostream& operator<<(std::ostream& o, Env::ResourceLimitType t);
} // namespace kudu
-
-#endif // STORAGE_LEVELDB_INCLUDE_ENV_H_
diff --git a/src/kudu/util/env_posix.cc b/src/kudu/util/env_posix.cc
index 3590fd1..586b333 100644
--- a/src/kudu/util/env_posix.cc
+++ b/src/kudu/util/env_posix.cc
@@ -26,6 +26,7 @@
#include <cstdlib>
#include <cstring>
#include <ctime>
+#include <functional>
#include <map>
#include <memory>
#include <numeric>
@@ -39,8 +40,6 @@
#include "kudu/gutil/atomicops.h"
#include "kudu/gutil/basictypes.h"
-#include "kudu/gutil/bind.h"
-#include "kudu/gutil/bind_helpers.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/map-util.h"
#include "kudu/gutil/once.h"
@@ -1332,8 +1331,11 @@ class PosixEnv : public Env {
}
virtual Status DeleteRecursively(const string &name) OVERRIDE {
- return Walk(name, POST_ORDER, Bind(&PosixEnv::DeleteRecursivelyCb,
- Unretained(this)));
+ return Walk(
+ name, POST_ORDER,
+ [this](FileType type, const string& dirname, const string& basename) -> Status {
+ return this->DeleteRecursivelyCb(type, dirname, basename);
+ });
}
virtual Status GetFileSize(const string& fname, uint64_t* size) OVERRIDE {
@@ -1373,9 +1375,11 @@ class PosixEnv : public Env {
uint64_t* bytes_used) OVERRIDE {
TRACE_EVENT1("io", "PosixEnv::GetFileSizeOnDiskRecursively", "path", root);
uint64_t total = 0;
- RETURN_NOT_OK(Walk(root, Env::PRE_ORDER,
- Bind(&PosixEnv::GetFileSizeOnDiskRecursivelyCb,
- Unretained(this), &total)));
+ RETURN_NOT_OK(Walk(
+ root, PRE_ORDER,
+ [this, &total](FileType type, const string& dirname, const string& basename) -> Status {
+ return this->GetFileSizeOnDiskRecursivelyCb(&total, type, dirname, basename);
+ }));
*bytes_used = total;
return Status::OK();
}
@@ -1625,7 +1629,7 @@ class PosixEnv : public Env {
break;
}
if (doCb) {
- if (!cb.Run(type, DirName(ent->fts_path), ent->fts_name).ok()) {
+ if (!cb(type, DirName(ent->fts_path), ent->fts_name).ok()) {
had_errors = true;
}
}
@@ -1891,7 +1895,7 @@ class PosixEnv : public Env {
}
Status GetFileSizeOnDiskRecursivelyCb(uint64_t* bytes_used,
- Env::FileType type,
+ FileType type,
const string& dirname,
const string& basename) {
uint64_t file_bytes_used = 0;
diff --git a/src/kudu/util/env_util.cc b/src/kudu/util/env_util.cc
index 6fb8b8a..41f0ad1 100644
--- a/src/kudu/util/env_util.cc
+++ b/src/kudu/util/env_util.cc
@@ -23,6 +23,7 @@
#include <cerrno>
#include <cstdint>
#include <ctime>
+#include <functional>
#include <memory>
#include <string>
#include <unordered_set>
@@ -32,7 +33,6 @@
#include <gflags/gflags.h>
#include <glog/logging.h>
-#include "kudu/gutil/bind.h"
#include "kudu/gutil/macros.h"
#include "kudu/gutil/port.h"
#include "kudu/gutil/strings/split.h"
@@ -309,7 +309,11 @@ static Status DeleteTmpFilesRecursivelyCb(Env* env,
}
Status DeleteTmpFilesRecursively(Env* env, const string& path) {
- return env->Walk(path, Env::PRE_ORDER, Bind(&DeleteTmpFilesRecursivelyCb, env));
+ return env->Walk(
+ path, Env::PRE_ORDER,
+ [env](Env::FileType type, const string& dirname, const string& basename) {
+ return DeleteTmpFilesRecursivelyCb(env, type, dirname, basename);
+ });
}
Status IsDirectoryEmpty(Env* env, const string& path, bool* is_empty) {
diff --git a/src/kudu/util/env_util.h b/src/kudu/util/env_util.h
index bba1e92..00007d6 100644
--- a/src/kudu/util/env_util.h
+++ b/src/kudu/util/env_util.h
@@ -14,8 +14,7 @@
// KIND, either express or implied. See the License for the
// specific language governing permissions and limitations
// under the License.
-#ifndef KUDU_UTIL_ENV_UTIL_H
-#define KUDU_UTIL_ENV_UTIL_H
+#pragma once
#include <cstddef>
#include <cstdint>
@@ -111,5 +110,3 @@ Status ListFilesInDir(Env* env,
} // namespace env_util
} // namespace kudu
-
-#endif
diff --git a/src/kudu/util/metrics.h b/src/kudu/util/metrics.h
index 7a1ed07..ab31113 100644
--- a/src/kudu/util/metrics.h
+++ b/src/kudu/util/metrics.h
@@ -1169,7 +1169,7 @@ class AtomicGauge : public Gauge {
// public:
// MyClassWithMetrics(const scoped_refptr<MetricEntity>& entity) {
// METRIC_my_metric.InstantiateFunctionGauge(entity,
-// Bind(&MyClassWithMetrics::ComputeMyMetric, Unretained(this)))
+// [this]() { return this->ComputeMyMetric(); })
// ->AutoDetach(&metric_detacher_);
// }
// ~MyClassWithMetrics() {