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 <[email protected]> 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 <[email protected]> Reviewed-by: Andrew Wong <[email protected]> --- 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() {
