Merge commit '7a65c6bc48c96fa640cbf13d45a692d0a64200e3' into HEAD
Change-Id: If41aa1299ee779171c376f3f4619538d7f380384
diff --git a/BUILD.gn b/BUILD.gn
index f30f945..8eefb58 100644
--- a/BUILD.gn
+++ b/BUILD.gn
@@ -8,17 +8,17 @@
group("all") {
deps = [
- ":libbrillo-${libbase_ver}",
- ":libbrillo-glib-${libbase_ver}",
- ":libbrillo-test-${libbase_ver}",
- ":libinstallattributes-${libbase_ver}",
- ":libpolicy-${libbase_ver}",
+ ":libbrillo",
+ ":libbrillo-glib",
+ ":libbrillo-test",
+ ":libinstallattributes",
+ ":libpolicy",
]
if (use.test) {
deps += [
- ":libbrillo-${libbase_ver}_tests",
- ":libinstallattributes-${libbase_ver}_tests",
- ":libpolicy-${libbase_ver}_tests",
+ ":libbrillo_tests",
+ ":libinstallattributes_tests",
+ ":libpolicy_tests",
]
}
if (use.fuzzer) {
@@ -44,7 +44,7 @@
]
}
-config("libbrillo-${libbase_ver}_configs") {
+config("libbrillo_configs") {
include_dirs = [ "../libbrillo" ]
}
@@ -56,7 +56,7 @@
# |library_name| is library file name without "lib" prefix. This is needed
# for composing -l*** flags in libbrillo-${libbasever}.so.
# (Current version of GN deployed to ChromeOS doesn't have string_replace.)
- library_name = "brillo-core-${libbase_ver}"
+ library_name = "brillo-core"
if (use.dbus) {
all_dependent_pkg_deps = [ "dbus-1" ]
}
@@ -81,11 +81,11 @@
"brillo/process.cc",
"brillo/process_information.cc",
"brillo/process_reaper.cc",
- "brillo/scoped_mount_namespace.cc",
"brillo/scoped_umask.cc",
"brillo/secure_blob.cc",
"brillo/strings/string_utils.cc",
"brillo/syslog_logging.cc",
+ "brillo/timezone/tzif_parser.cc",
"brillo/type_name_undecorate.cc",
"brillo/url_utils.cc",
"brillo/userdb_utils.cc",
@@ -112,13 +112,9 @@
},
{
- library_name = "brillo-blockdeviceutils-${libbase_ver}"
- deps = [
- ":libbrillo-core-${libbase_ver}",
- ]
- sources = [
- "brillo/blkdev_utils/loop_device.cc",
- ]
+ library_name = "brillo-blockdeviceutils"
+ deps = [ ":libbrillo-core" ]
+ sources = [ "brillo/blkdev_utils/loop_device.cc" ]
if (use.device_mapper) {
pkg_deps = [ "devmapper" ]
sources += [
@@ -129,10 +125,10 @@
},
{
- library_name = "brillo-http-${libbase_ver}"
+ library_name = "brillo-http"
deps = [
- ":libbrillo-core-${libbase_ver}",
- ":libbrillo-streams-${libbase_ver}",
+ ":libbrillo-core",
+ ":libbrillo-streams",
]
all_dependent_pkg_deps = [ "libcurl" ]
sources = [
@@ -150,10 +146,8 @@
},
{
- library_name = "brillo-streams-${libbase_ver}"
- deps = [
- ":libbrillo-core-${libbase_ver}",
- ]
+ library_name = "brillo-streams"
+ deps = [ ":libbrillo-core" ]
all_dependent_pkg_deps = [ "openssl" ]
sources = [
"brillo/streams/file_stream.cc",
@@ -169,34 +163,38 @@
},
{
- library_name = "brillo-cryptohome-${libbase_ver}"
+ library_name = "brillo-cryptohome"
all_dependent_pkg_deps = [ "openssl" ]
+ sources = [ "brillo/cryptohome.cc" ]
+ },
+
+ {
+ library_name = "brillo-namespaces"
+ deps = [ ":libbrillo-core" ]
sources = [
- "brillo/cryptohome.cc",
+ "brillo/namespaces/mount_namespace.cc",
+ "brillo/namespaces/platform.cc",
+ "brillo/scoped_mount_namespace.cc",
]
},
{
- library_name = "brillo-minijail-${libbase_ver}"
+ library_name = "brillo-minijail"
all_dependent_pkg_deps = [ "libminijail" ]
- sources = [
- "brillo/minijail/minijail.cc",
- ]
+ sources = [ "brillo/minijail/minijail.cc" ]
},
{
- library_name = "brillo-protobuf-${libbase_ver}"
+ library_name = "brillo-protobuf"
all_dependent_pkg_deps = [ "protobuf" ]
- sources = [
- "brillo/proto_file_io.cc",
- ]
+ sources = [ "brillo/proto_file_io.cc" ]
},
]
if (use.udev) {
libbrillo_sublibs += [
{
- library_name = "brillo-udev-${libbase_ver}"
+ library_name = "brillo-udev"
all_dependent_pkg_deps = [ "libudev" ]
sources = [
"brillo/udev/udev.cc",
@@ -238,7 +236,7 @@
}
}
-generate_pkg_config("libbrillo_pc") {
+generate_pkg_config("libbrillo-${libbase_ver}_pc") {
name = "libbrillo"
output_name = "libbrillo-${libbase_ver}"
description = "brillo base library"
@@ -256,20 +254,40 @@
}
}
defines = [ "USE_RTTI_FOR_TYPE_TAGS" ]
- libs = [ "-lbrillo-${libbase_ver}" ]
+ libs = [ "-lbrillo" ]
}
-action("libbrillo-${libbase_ver}") {
+generate_pkg_config("libbrillo_pc") {
+ name = "libbrillo"
+ output_name = "libbrillo"
+ description = "brillo base library"
+ version = libbase_ver
+ requires_private = default_pkg_deps
+ foreach(sublib, libbrillo_sublibs) {
+ if (defined(sublib.pkg_deps)) {
+ requires_private += sublib.pkg_deps
+ }
+ if (defined(sublib.public_pkg_deps)) {
+ requires_private += sublib.public_pkg_deps
+ }
+ if (defined(sublib.all_dependent_pkg_deps)) {
+ requires_private += sublib.all_dependent_pkg_deps
+ }
+ }
+ defines = [ "USE_RTTI_FOR_TYPE_TAGS" ]
+ libs = [ "-lbrillo" ]
+}
+
+action("libbrillo") {
deps = [
+ ":libbrillo-${libbase_ver}_pc",
":libbrillo_pc",
]
foreach(sublib, libbrillo_sublibs) {
deps += [ ":lib" + sublib.library_name ]
}
script = "//common-mk/write_args.py"
- outputs = [
- "${root_out_dir}/lib/libbrillo-${libbase_ver}.so",
- ]
+ outputs = [ "${root_out_dir}/lib/libbrillo.so" ]
args = [ "--output" ] + outputs + [ "--" ] + [
"GROUP",
"(",
@@ -285,16 +303,16 @@
]
}
-libbrillo_test_deps = [ "libbrillo-http-${libbase_ver}" ]
+libbrillo_test_deps = [ "libbrillo-http" ]
-generate_pkg_config("libbrillo-test_pc") {
+generate_pkg_config("libbrillo-test-${libbase_ver}_pc") {
name = "libbrillo-test"
output_name = "libbrillo-test-${libbase_ver}"
description = "brillo test library"
version = libbase_ver
# Because libbrillo-test is static, we have to depend directly on everything.
- requires = [ "libbrillo-${libbase_ver}" ] + default_pkg_deps
+ requires = [ "libbrillo" ] + default_pkg_deps
foreach(name, libbrillo_test_deps) {
foreach(t, libbrillo_sublibs) {
if ("lib" + t.library_name == name) {
@@ -310,17 +328,44 @@
}
}
}
- libs = [ "-lbrillo-test-${libbase_ver}" ]
+ libs = [ "-lbrillo-test" ]
}
-static_library("libbrillo-test-${libbase_ver}") {
+generate_pkg_config("libbrillo-test_pc") {
+ name = "libbrillo-test"
+ output_name = "libbrillo-test"
+ description = "brillo test library"
+ version = libbase_ver
+
+ # Because libbrillo-test is static, we have to depend directly on everything.
+ requires = [ "libbrillo" ] + default_pkg_deps
+ foreach(name, libbrillo_test_deps) {
+ foreach(t, libbrillo_sublibs) {
+ if ("lib" + t.library_name == name) {
+ if (defined(t.pkg_deps)) {
+ requires += t.pkg_deps
+ }
+ if (defined(t.public_pkg_deps)) {
+ requires += t.public_pkg_deps
+ }
+ if (defined(t.all_dependent_pkg_deps)) {
+ requires += t.all_dependent_pkg_deps
+ }
+ }
+ }
+ }
+ libs = [ "-lbrillo-test" ]
+}
+
+static_library("libbrillo-test") {
configs -= [ "//common-mk:use_thin_archive" ]
configs += [
"//common-mk:nouse_thin_archive",
":target_defaults",
]
deps = [
- ":libbrillo-http-${libbase_ver}",
+ ":libbrillo-http",
+ ":libbrillo-test-${libbase_ver}_pc",
":libbrillo-test_pc",
]
foreach(name, libbrillo_test_deps) {
@@ -339,22 +384,20 @@
}
}
-shared_library("libinstallattributes-${libbase_ver}") {
+shared_library("libinstallattributes") {
configs += [ ":target_defaults" ]
deps = [
":libinstallattributes-includes",
"../common-mk/external_dependencies:install_attributes-proto",
]
all_dependent_pkg_deps = [ "protobuf-lite" ]
- sources = [
- "install_attributes/libinstallattributes.cc",
- ]
+ sources = [ "install_attributes/libinstallattributes.cc" ]
}
-shared_library("libpolicy-${libbase_ver}") {
+shared_library("libpolicy") {
configs += [ ":target_defaults" ]
deps = [
- ":libinstallattributes-${libbase_ver}",
+ ":libinstallattributes",
":libpolicy-includes",
"../common-mk/external_dependencies:policy-protos",
]
@@ -383,19 +426,29 @@
]
}
-generate_pkg_config("libbrillo-glib_pc") {
+generate_pkg_config("libbrillo-glib-${libbase_ver}_pc") {
name = "libbrillo-glib"
output_name = "libbrillo-glib-${libbase_ver}"
description = "brillo glib wrapper library"
version = libbase_ver
requires_private = libbrillo_glib_pkg_deps
- libs = [ "-lbrillo-glib-${libbase_ver}" ]
+ libs = [ "-lbrillo-glib" ]
}
-shared_library("libbrillo-glib-${libbase_ver}") {
+generate_pkg_config("libbrillo-glib_pc") {
+ name = "libbrillo-glib"
+ output_name = "libbrillo-glib"
+ description = "brillo glib wrapper library"
+ version = libbase_ver
+ requires_private = libbrillo_glib_pkg_deps
+ libs = [ "-lbrillo-glib" ]
+}
+
+shared_library("libbrillo-glib") {
configs += [ ":target_defaults" ]
deps = [
- ":libbrillo-${libbase_ver}",
+ ":libbrillo",
+ ":libbrillo-glib-${libbase_ver}_pc",
":libbrillo-glib_pc",
]
all_dependent_pkg_deps = libbrillo_glib_pkg_deps
@@ -412,35 +465,34 @@
}
if (use.test) {
- static_library("libbrillo-${libbase_ver}_static") {
+ static_library("libbrillo_static") {
configs += [ ":target_defaults" ]
deps = [
+ ":libbrillo-${libbase_ver}_pc",
":libbrillo_pc",
- ":libinstallattributes-${libbase_ver}",
- ":libpolicy-${libbase_ver}",
+ ":libinstallattributes",
+ ":libpolicy",
]
foreach(sublib, libbrillo_sublibs) {
deps += [ ":lib" + sublib.library_name ]
}
- public_configs = [ ":libbrillo-${libbase_ver}_configs" ]
+ public_configs = [ ":libbrillo_configs" ]
}
- proto_library("libbrillo-${libbase_ver}_tests_proto") {
+ proto_library("libbrillo_tests_proto") {
proto_in_dir = "brillo/dbus"
proto_out_dir = "include/brillo/dbus"
- sources = [
- "${proto_in_dir}/test.proto",
- ]
+ sources = [ "${proto_in_dir}/test.proto" ]
}
- executable("libbrillo-${libbase_ver}_tests") {
+ executable("libbrillo_tests") {
configs += [
"//common-mk:test",
":target_defaults",
]
deps = [
- ":libbrillo-${libbase_ver}_static",
- ":libbrillo-${libbase_ver}_tests_proto",
- ":libbrillo-glib-${libbase_ver}",
- ":libbrillo-test-${libbase_ver}",
+ ":libbrillo-glib",
+ ":libbrillo-test",
+ ":libbrillo_static",
+ ":libbrillo_tests_proto",
]
pkg_deps = [ "libchrome-test-${libbase_ver}" ]
cflags = [ "-Wno-format-zero-length" ]
@@ -477,6 +529,7 @@
"brillo/message_loops/fake_message_loop_test.cc",
"brillo/message_loops/message_loop_test.cc",
"brillo/mime_utils_test.cc",
+ "brillo/namespaces/mount_namespace_test.cc",
"brillo/osrelease_reader_test.cc",
"brillo/process_reaper_test.cc",
"brillo/process_test.cc",
@@ -491,6 +544,7 @@
"brillo/streams/stream_test.cc",
"brillo/streams/stream_utils_test.cc",
"brillo/strings/string_utils_test.cc",
+ "brillo/timezone/tzif_parser_test.cc",
"brillo/unittest_utils.cc",
"brillo/url_utils_test.cc",
"brillo/value_conversion_test.cc",
@@ -519,29 +573,27 @@
}
}
- executable("libinstallattributes-${libbase_ver}_tests") {
+ executable("libinstallattributes_tests") {
configs += [
"//common-mk:test",
":target_defaults",
]
deps = [
- ":libinstallattributes-${libbase_ver}",
+ ":libinstallattributes",
"../common-mk/external_dependencies:install_attributes-proto",
"../common-mk/testrunner:testrunner",
]
- sources = [
- "install_attributes/tests/libinstallattributes_test.cc",
- ]
+ sources = [ "install_attributes/tests/libinstallattributes_test.cc" ]
}
- executable("libpolicy-${libbase_ver}_tests") {
+ executable("libpolicy_tests") {
configs += [
"//common-mk:test",
":target_defaults",
]
deps = [
- ":libinstallattributes-${libbase_ver}",
- ":libpolicy-${libbase_ver}",
+ ":libinstallattributes",
+ ":libpolicy",
"../common-mk/external_dependencies:install_attributes-proto",
"../common-mk/external_dependencies:policy-protos",
"../common-mk/testrunner:testrunner",
@@ -558,9 +610,7 @@
if (use.fuzzer) {
executable("libbrillo_data_encoding_fuzzer") {
- sources = [
- "brillo/data_encoding_fuzzer.cc",
- ]
+ sources = [ "brillo/data_encoding_fuzzer.cc" ]
configs += [ "//common-mk/common_fuzzer:common_fuzzer" ]
@@ -568,15 +618,11 @@
include_dirs = [ "../libbrillo" ]
- deps = [
- ":libbrillo-core-${libbase_ver}",
- ]
+ deps = [ ":libbrillo-core" ]
}
executable("libbrillo_dbus_data_serialization_fuzzer") {
- sources = [
- "brillo/dbus/data_serialization_fuzzer.cc",
- ]
+ sources = [ "brillo/dbus/data_serialization_fuzzer.cc" ]
configs += [ "//common-mk/common_fuzzer:common_fuzzer" ]
@@ -584,15 +630,11 @@
include_dirs = [ "../libbrillo" ]
- deps = [
- ":libbrillo-core-${libbase_ver}",
- ]
+ deps = [ ":libbrillo-core" ]
}
executable("libbrillo_http_form_data_fuzzer") {
- sources = [
- "brillo/http/http_form_data_fuzzer.cc",
- ]
+ sources = [ "brillo/http/http_form_data_fuzzer.cc" ]
configs += [ "//common-mk/common_fuzzer:common_fuzzer" ]
@@ -601,19 +643,16 @@
include_dirs = [ "../libbrillo" ]
deps = [
- ":libbrillo-http-${libbase_ver}",
- ":libbrillo-streams-${libbase_ver}",
+ ":libbrillo-http",
+ ":libbrillo-streams",
]
}
}
copy("libinstallattributes-includes") {
- sources = [
- "install_attributes/libinstallattributes.h",
- ]
- outputs = [
- "${root_gen_dir}/include/install_attributes/{{source_file_part}}",
- ]
+ sources = [ "install_attributes/libinstallattributes.h" ]
+ outputs =
+ [ "${root_gen_dir}/include/install_attributes/{{source_file_part}}" ]
}
copy("libpolicy-includes") {
@@ -626,7 +665,5 @@
"policy/policy_util.h",
"policy/resilient_policy_util.h",
]
- outputs = [
- "${root_gen_dir}/include/policy/{{source_file_part}}",
- ]
+ outputs = [ "${root_gen_dir}/include/policy/{{source_file_part}}" ]
}
diff --git a/brillo/asynchronous_signal_handler.cc b/brillo/asynchronous_signal_handler.cc
index d5fed50..30ac2a5 100644
--- a/brillo/asynchronous_signal_handler.cc
+++ b/brillo/asynchronous_signal_handler.cc
@@ -11,16 +11,10 @@
#include <base/bind.h>
#include <base/files/file_util.h>
#include <base/logging.h>
-#include <base/posix/eintr_wrapper.h>
-
-namespace {
-const int kInvalidDescriptor = -1;
-} // namespace
namespace brillo {
-AsynchronousSignalHandler::AsynchronousSignalHandler()
- : descriptor_(kInvalidDescriptor) {
+AsynchronousSignalHandler::AsynchronousSignalHandler() {
CHECK_EQ(sigemptyset(&signal_mask_), 0) << "Failed to initialize signal mask";
CHECK_EQ(sigemptyset(&saved_signal_mask_), 0)
<< "Failed to initialize signal mask";
@@ -29,26 +23,29 @@
AsynchronousSignalHandler::~AsynchronousSignalHandler() {
fd_watcher_ = nullptr;
- if (descriptor_ == kInvalidDescriptor)
+ if (!descriptor_.is_valid())
return;
- if (IGNORE_EINTR(close(descriptor_)) != 0)
- PLOG(WARNING) << "Failed to close file descriptor";
- descriptor_ = kInvalidDescriptor;
+ // Close FD before restoring sigprocmask.
+ descriptor_.reset();
CHECK_EQ(0, sigprocmask(SIG_SETMASK, &saved_signal_mask_, nullptr));
}
void AsynchronousSignalHandler::Init() {
- CHECK_EQ(kInvalidDescriptor, descriptor_);
+ // Making sure it is not yet initialized.
+ CHECK(!descriptor_.is_valid());
+
+ // Set sigprocmask before creating signalfd.
CHECK_EQ(0, sigprocmask(SIG_BLOCK, &signal_mask_, &saved_signal_mask_));
- descriptor_ =
- signalfd(descriptor_, &signal_mask_, SFD_CLOEXEC | SFD_NONBLOCK);
- CHECK_NE(kInvalidDescriptor, descriptor_);
+
+ // Creating signalfd, and start watching it.
+ descriptor_.reset(signalfd(-1, &signal_mask_, SFD_CLOEXEC | SFD_NONBLOCK));
+ CHECK(descriptor_.is_valid());
fd_watcher_ = base::FileDescriptorWatcher::WatchReadable(
- descriptor_,
+ descriptor_.get(),
base::BindRepeating(&AsynchronousSignalHandler::OnReadable,
base::Unretained(this)));
- CHECK(fd_watcher_) << "Watching shutdown pipe failed.";
+ CHECK(fd_watcher_) << "Watching signalfd failed.";
}
void AsynchronousSignalHandler::RegisterHandler(int signal,
@@ -60,15 +57,16 @@
void AsynchronousSignalHandler::UnregisterHandler(int signal) {
Callbacks::iterator callback_it = registered_callbacks_.find(signal);
- if (callback_it != registered_callbacks_.end()) {
- registered_callbacks_.erase(callback_it);
- ResetSignal(signal);
- }
+ if (callback_it == registered_callbacks_.end())
+ return;
+ registered_callbacks_.erase(callback_it);
+ CHECK_EQ(0, sigdelset(&signal_mask_, signal));
+ UpdateSignals();
}
void AsynchronousSignalHandler::OnReadable() {
struct signalfd_siginfo info;
- while (base::ReadFromFD(descriptor_,
+ while (base::ReadFromFD(descriptor_.get(),
reinterpret_cast<char*>(&info), sizeof(info))) {
int signal = info.ssi_signo;
Callbacks::iterator callback_it = registered_callbacks_.find(signal);
@@ -80,24 +78,20 @@
}
const SignalHandler& callback = callback_it->second;
bool must_unregister = callback.Run(info);
- if (must_unregister) {
+ if (must_unregister)
UnregisterHandler(signal);
- }
}
}
-void AsynchronousSignalHandler::ResetSignal(int signal) {
- CHECK_EQ(0, sigdelset(&signal_mask_, signal));
- UpdateSignals();
-}
-
void AsynchronousSignalHandler::UpdateSignals() {
- if (descriptor_ != kInvalidDescriptor) {
- CHECK_EQ(0, sigprocmask(SIG_SETMASK, &saved_signal_mask_, nullptr));
- CHECK_EQ(0, sigprocmask(SIG_BLOCK, &signal_mask_, nullptr));
- CHECK_EQ(descriptor_,
- signalfd(descriptor_, &signal_mask_, SFD_CLOEXEC | SFD_NONBLOCK));
- }
+ if (!descriptor_.is_valid())
+ return;
+ sigset_t mask;
+ CHECK_EQ(0, sigorset(&mask, &signal_mask_, &saved_signal_mask_));
+ CHECK_EQ(0, sigprocmask(SIG_SETMASK, &mask, nullptr));
+ CHECK_EQ(
+ descriptor_.get(),
+ signalfd(descriptor_.get(), &signal_mask_, SFD_CLOEXEC | SFD_NONBLOCK));
}
} // namespace brillo
diff --git a/brillo/asynchronous_signal_handler.h b/brillo/asynchronous_signal_handler.h
index 903e2ef..4b0edce 100644
--- a/brillo/asynchronous_signal_handler.h
+++ b/brillo/asynchronous_signal_handler.h
@@ -5,16 +5,14 @@
#ifndef LIBBRILLO_BRILLO_ASYNCHRONOUS_SIGNAL_HANDLER_H_
#define LIBBRILLO_BRILLO_ASYNCHRONOUS_SIGNAL_HANDLER_H_
-#include <signal.h>
#include <sys/signalfd.h>
#include <map>
#include <memory>
#include <base/callback.h>
-#include <base/compiler_specific.h>
#include <base/files/file_descriptor_watcher_posix.h>
-#include <base/macros.h>
+#include <base/files/scoped_file.h>
#include <brillo/asynchronous_signal_handler_interface.h>
#include <brillo/brillo_export.h>
@@ -25,10 +23,14 @@
class BRILLO_EXPORT AsynchronousSignalHandler final :
public AsynchronousSignalHandlerInterface {
public:
+ using AsynchronousSignalHandlerInterface::SignalHandler;
+
AsynchronousSignalHandler();
~AsynchronousSignalHandler() override;
- using AsynchronousSignalHandlerInterface::SignalHandler;
+ AsynchronousSignalHandler(const AsynchronousSignalHandler&) = delete;
+ AsynchronousSignalHandler&
+ operator=(const AsynchronousSignalHandler&) = delete;
// Initialize the handler.
void Init();
@@ -42,15 +44,18 @@
// that a signal was processed.
void OnReadable();
- // Controller used to manage watching of signalling pipe.
- std::unique_ptr<base::FileDescriptorWatcher::Controller> fd_watcher_;
+ // Updates the set of signals that this handler listens to.
+ BRILLO_PRIVATE void UpdateSignals();
- // The registered callbacks.
- typedef std::map<int, SignalHandler> Callbacks;
+ // Map from signal to its registered callback.
+ using Callbacks = std::map<int, SignalHandler>;
Callbacks registered_callbacks_;
// File descriptor for accepting signals indicated by |signal_mask_|.
- int descriptor_;
+ base::ScopedFD descriptor_;
+
+ // Controller used to manage watching of signalling pipe.
+ std::unique_ptr<base::FileDescriptorWatcher::Controller> fd_watcher_;
// A set of signals to be handled after the dispatcher is running.
sigset_t signal_mask_;
@@ -58,15 +63,6 @@
// A copy of the signal mask before the dispatcher starts, which will be
// used to restore to the original state when the dispatcher stops.
sigset_t saved_signal_mask_;
-
- // Resets the given signal to its default behavior. Doesn't touch
- // |registered_callbacks_|.
- BRILLO_PRIVATE void ResetSignal(int signal);
-
- // Updates the set of signals that this handler listens to.
- BRILLO_PRIVATE void UpdateSignals();
-
- DISALLOW_COPY_AND_ASSIGN(AsynchronousSignalHandler);
};
} // namespace brillo
diff --git a/brillo/asynchronous_signal_handler_interface.h b/brillo/asynchronous_signal_handler_interface.h
index ef0012d..7bae444 100644
--- a/brillo/asynchronous_signal_handler_interface.h
+++ b/brillo/asynchronous_signal_handler_interface.h
@@ -20,7 +20,8 @@
virtual ~AsynchronousSignalHandlerInterface() = default;
// The callback called when a signal is received.
- using SignalHandler = base::Callback<bool(const struct signalfd_siginfo&)>;
+ using SignalHandler =
+ base::RepeatingCallback<bool(const struct signalfd_siginfo&)>;
// Register a new handler for the given |signal|, replacing any previously
// registered handler. |callback| will be called on the thread the
diff --git a/brillo/blkdev_utils/loop_device.cc b/brillo/blkdev_utils/loop_device.cc
index bd1b67c..2b2219d 100644
--- a/brillo/blkdev_utils/loop_device.cc
+++ b/brillo/blkdev_utils/loop_device.cc
@@ -72,7 +72,8 @@
std::vector<std::string> device_ids = base::SplitString(
device_string, ":", base::TRIM_WHITESPACE, base::SPLIT_WANT_NONEMPTY);
- if (device_ids.size() != 2 || device_ids[0] != base::IntToString(LOOP_MAJOR))
+ if (device_ids.size() != 2 ||
+ device_ids[0] != base::NumberToString(LOOP_MAJOR))
return -1;
base::StringToInt(device_ids[1], &device_number);
diff --git a/brillo/cryptohome.cc b/brillo/cryptohome.cc
index 88e4739..a82356e 100644
--- a/brillo/cryptohome.cc
+++ b/brillo/cryptohome.cc
@@ -41,7 +41,7 @@
static std::string* salt = nullptr;
-static bool EnsureSystemSaltIsLoaded() {
+bool EnsureSystemSaltIsLoaded() {
if (salt && !salt->empty())
return true;
FilePath salt_path(g_system_salt_path);
diff --git a/brillo/cryptohome.h b/brillo/cryptohome.h
index 798d3a0..a9d5927 100644
--- a/brillo/cryptohome.h
+++ b/brillo/cryptohome.h
@@ -74,6 +74,9 @@
// Returns the system salt.
BRILLO_EXPORT std::string* GetSystemSalt();
+// Ensures the system salt is loaded in the memory.
+BRILLO_EXPORT bool EnsureSystemSaltIsLoaded();
+
} // namespace home
} // namespace cryptohome
} // namespace brillo
diff --git a/brillo/daemons/daemon.cc b/brillo/daemons/daemon.cc
index b706017..82de826 100644
--- a/brillo/daemons/daemon.cc
+++ b/brillo/daemons/daemon.cc
@@ -4,6 +4,7 @@
#include <brillo/daemons/daemon.h>
+#include <signal.h>
#include <sysexits.h>
#include <time.h>
@@ -27,7 +28,7 @@
return exit_code;
message_loop_.PostTask(
- base::Bind(&Daemon::OnEventLoopStartedTask, base::Unretained(this)));
+ base::BindOnce(&Daemon::OnEventLoopStartedTask, base::Unretained(this)));
message_loop_.Run();
OnShutdown(&exit_code_);
diff --git a/brillo/dbus/data_serialization.cc b/brillo/dbus/data_serialization.cc
index fa348a0..5f47d67 100644
--- a/brillo/dbus/data_serialization.cc
+++ b/brillo/dbus/data_serialization.cc
@@ -320,7 +320,6 @@
LOG(FATAL) << "Unknown D-Bus data type: " << variant_reader.GetDataType();
return false;
}
- return true;
}
} // namespace dbus_utils
diff --git a/brillo/dbus/dbus_method_invoker_test.cc b/brillo/dbus/dbus_method_invoker_test.cc
index 9e6600a..c0c681b 100644
--- a/brillo/dbus/dbus_method_invoker_test.cc
+++ b/brillo/dbus/dbus_method_invoker_test.cc
@@ -83,18 +83,16 @@
GetObjectProxy(kTestServiceName, dbus::ObjectPath(kTestPath)))
.WillRepeatedly(Return(mock_object_proxy_.get()));
int def_timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
- EXPECT_CALL(
- *mock_object_proxy_,
- MIGRATE_MockCallMethodAndBlockWithErrorDetails(_, def_timeout_ms, _))
+ EXPECT_CALL(*mock_object_proxy_,
+ CallMethodAndBlockWithErrorDetails(_, def_timeout_ms, _))
.WillRepeatedly(Invoke(this, &DBusMethodInvokerTest::CreateResponse));
}
void TearDown() override { bus_ = nullptr; }
- MIGRATE_WrapObjectProxyResponseType(Response)
- CreateResponse(dbus::MethodCall* method_call,
- int /* timeout_ms */,
- dbus::ScopedDBusError* dbus_error) {
+ std::unique_ptr<Response> CreateResponse(dbus::MethodCall* method_call,
+ int /* timeout_ms */,
+ dbus::ScopedDBusError* dbus_error) {
if (method_call->GetInterface() == kTestInterface) {
if (method_call->GetMember() == kTestMethod1) {
MessageReader reader(method_call);
@@ -105,12 +103,12 @@
auto response = Response::CreateEmpty();
MessageWriter writer(response.get());
writer.AppendString(std::to_string(v1 + v2));
- return MIGRATE_WrapObjectProxyResponseConversion(response);
+ return response;
}
} else if (method_call->GetMember() == kTestMethod2) {
method_call->SetSerial(123);
dbus_set_error(dbus_error->get(), "org.MyError", "My error message");
- return MIGRATE_WrapObjectProxyResponseEmpty;
+ return std::unique_ptr<dbus::Response>();
} else if (method_call->GetMember() == kTestMethod3) {
MessageReader reader(method_call);
dbus_utils_test::TestMessage msg;
@@ -118,7 +116,7 @@
auto response = Response::CreateEmpty();
MessageWriter writer(response.get());
AppendValueToWriter(&writer, msg);
- return MIGRATE_WrapObjectProxyResponseConversion(response);
+ return response;
}
} else if (method_call->GetMember() == kTestMethod4) {
method_call->SetSerial(123);
@@ -128,13 +126,13 @@
auto response = Response::CreateEmpty();
MessageWriter writer(response.get());
writer.AppendFileDescriptor(fd.get());
- return MIGRATE_WrapObjectProxyResponseConversion(response);
+ return response;
}
}
}
LOG(ERROR) << "Unexpected method call: " << method_call->ToString();
- return MIGRATE_WrapObjectProxyResponseEmpty;
+ return std::unique_ptr<dbus::Response>();
}
std::string CallTestMethod(int v1, int v2) {
@@ -245,7 +243,7 @@
.WillRepeatedly(Return(mock_object_proxy_.get()));
int def_timeout_ms = dbus::ObjectProxy::TIMEOUT_USE_DEFAULT;
EXPECT_CALL(*mock_object_proxy_,
- MIGRATE_CallMethodWithErrorCallback(_, def_timeout_ms, _, _))
+ DoCallMethodWithErrorCallback(_, def_timeout_ms, _, _))
.WillRepeatedly(Invoke(this, &AsyncDBusMethodInvokerTest::HandleCall));
}
@@ -253,10 +251,8 @@
void HandleCall(dbus::MethodCall* method_call,
int /* timeout_ms */,
- dbus::ObjectProxy::ResponseCallback
- MIGRATE_WrapObjectProxyCallback(success_callback),
- dbus::ObjectProxy::ErrorCallback
- MIGRATE_WrapObjectProxyCallback(error_callback)) {
+ dbus::ObjectProxy::ResponseCallback* success_callback,
+ dbus::ObjectProxy::ErrorCallback* error_callback) {
if (method_call->GetInterface() == kTestInterface) {
if (method_call->GetMember() == kTestMethod1) {
MessageReader reader(method_call);
@@ -267,16 +263,14 @@
auto response = Response::CreateEmpty();
MessageWriter writer(response.get());
writer.AppendString(std::to_string(v1 + v2));
- std::move(MIGRATE_WrapObjectProxyCallback(success_callback))
- .Run(response.get());
+ std::move(*success_callback).Run(response.get());
}
return;
} else if (method_call->GetMember() == kTestMethod2) {
method_call->SetSerial(123);
auto error_response = dbus::ErrorResponse::FromMethodCall(
method_call, "org.MyError", "My error message");
- std::move(MIGRATE_WrapObjectProxyCallback(error_callback))
- .Run(error_response.get());
+ std::move(*error_callback).Run(error_response.get());
return;
}
}
diff --git a/brillo/dbus/dbus_object.cc b/brillo/dbus/dbus_object.cc
index 12eb353..502af3e 100644
--- a/brillo/dbus/dbus_object.cc
+++ b/brillo/dbus/dbus_object.cc
@@ -9,6 +9,7 @@
#include <vector>
#include <base/bind.h>
+#include <base/bind_helpers.h>
#include <base/logging.h>
#include <brillo/dbus/async_event_sequencer.h>
#include <brillo/dbus/exported_object_manager.h>
@@ -41,8 +42,7 @@
const std::string& interface_name)
: dbus_object_(dbus_object),
interface_name_(interface_name),
- // TODO(crbug.com/909719): Use base::DoNothing()
- release_interface_cb_(base::Bind([]() {})) {}
+ release_interface_cb_(base::DoNothing()) {}
void DBusInterface::AddProperty(const std::string& property_name,
ExportedPropertyBase* prop_base) {
diff --git a/brillo/dbus/dbus_property.h b/brillo/dbus/dbus_property.h
index 77b7328..f82759e 100644
--- a/brillo/dbus/dbus_property.h
+++ b/brillo/dbus/dbus_property.h
@@ -5,6 +5,8 @@
#ifndef LIBBRILLO_BRILLO_DBUS_DBUS_PROPERTY_H_
#define LIBBRILLO_BRILLO_DBUS_DBUS_PROPERTY_H_
+#include <utility>
+
#include <brillo/dbus/data_serialization.h>
#include <dbus/property.h>
@@ -42,7 +44,7 @@
// remote object.
void Set(const T& value, ::dbus::PropertySet::SetCallback callback) {
set_value_ = value;
- property_set()->Set(this, callback);
+ property_set()->Set(this, std::move(callback));
}
// Synchronous version of Set().
diff --git a/brillo/dbus/dbus_signal_handler_test.cc b/brillo/dbus/dbus_signal_handler_test.cc
index edd0eca..35bad65 100644
--- a/brillo/dbus/dbus_signal_handler_test.cc
+++ b/brillo/dbus/dbus_signal_handler_test.cc
@@ -50,7 +50,7 @@
void CallSignal(SignalHandlerSink* sink, Args... args) {
dbus::ObjectProxy::SignalCallback signal_callback;
EXPECT_CALL(*mock_object_proxy_,
- MIGRATE_ConnectToSignal(kInterface, kSignal, _, _))
+ DoConnectToSignal(kInterface, kSignal, _, _))
.WillOnce(SaveArg<2>(&signal_callback));
brillo::dbus_utils::ConnectToSignal(
@@ -71,8 +71,7 @@
};
TEST_F(DBusSignalHandlerTest, ConnectToSignal) {
- EXPECT_CALL(*mock_object_proxy_,
- MIGRATE_ConnectToSignal(kInterface, kSignal, _, _))
+ EXPECT_CALL(*mock_object_proxy_, DoConnectToSignal(kInterface, kSignal, _, _))
.Times(1);
brillo::dbus_utils::ConnectToSignal(
diff --git a/brillo/dbus/exported_object_manager.cc b/brillo/dbus/exported_object_manager.cc
index 61dae68..a2ae1fe 100644
--- a/brillo/dbus/exported_object_manager.cc
+++ b/brillo/dbus/exported_object_manager.cc
@@ -89,11 +89,11 @@
// DICT<STRING,VARIANT>>> )
bus_->AssertOnOriginThread();
ExportedObjectManager::ObjectMap objects;
- for (const auto path_pair : registered_objects_) {
+ for (const auto& path_pair : registered_objects_) {
std::map<std::string, VariantDictionary>& interfaces =
objects[path_pair.first];
const InterfaceProperties& interface2properties = path_pair.second;
- for (const auto interface : interface2properties) {
+ for (const auto& interface : interface2properties) {
interface.second.Run(&interfaces[interface.first]);
}
}
diff --git a/brillo/file_utils.cc b/brillo/file_utils.cc
index b4370d1..3661551 100644
--- a/brillo/file_utils.cc
+++ b/brillo/file_utils.cc
@@ -11,11 +11,13 @@
#include <utility>
#include <vector>
+#include <base/files/file_enumerator.h>
#include <base/files/file_path.h>
#include <base/files/file_util.h>
#include <base/logging.h>
#include <base/posix/eintr_wrapper.h>
#include <base/rand_util.h>
+#include <base/stl_util.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/stringprintf.h>
#include <base/time/time.h>
@@ -140,7 +142,7 @@
std::string GetRandomSuffix() {
const int kBufferSize = 6;
unsigned char buffer[kBufferSize];
- base::RandBytes(buffer, arraysize(buffer));
+ base::RandBytes(buffer, base::size(buffer));
std::string suffix;
for (int i = 0; i < kBufferSize; ++i) {
int random_value = buffer[i] % (2 * 26 + 10);
@@ -505,4 +507,20 @@
return true;
}
+int64_t ComputeDirectoryDiskUsage(const base::FilePath& root_path) {
+ constexpr size_t S_BLKSIZE = 512;
+ int64_t running_blocks = 0;
+ base::FileEnumerator file_iter(root_path, true,
+ base::FileEnumerator::FILES |
+ base::FileEnumerator::DIRECTORIES |
+ base::FileEnumerator::SHOW_SYM_LINKS);
+ while (!file_iter.Next().empty()) {
+ // st_blocks in struct stat is the number of S_BLKSIZE (512) bytes sized
+ // blocks occupied by this file.
+ running_blocks += file_iter.GetInfo().stat().st_blocks;
+ }
+ // Each block is S_BLKSIZE (512) bytes so *S_BLKSIZE.
+ return running_blocks * S_BLKSIZE;
+}
+
} // namespace brillo
diff --git a/brillo/file_utils.h b/brillo/file_utils.h
index 3862a43..f328165 100644
--- a/brillo/file_utils.h
+++ b/brillo/file_utils.h
@@ -144,6 +144,48 @@
blob.size(), mode);
}
+// ComputeDirectoryDiskUsage() is similar to base::ComputeDirectorySize() in
+// libbase, but it returns the actual disk usage instead of the apparent size.
+// In another word, ComputeDirectoryDiskUsage() behaves like "du -s
+// --apparent-size", and ComputeDirectorySize() behaves like "du -s". The
+// primary difference is that sparse file and files on filesystem with
+// transparent compression will report smaller file size than
+// ComputeDirectorySize(). Returns the total used bytes.
+// The following behaviours of this function is guaranteed and is verified by
+// unit tests:
+// - This function recursively processes directory down the tree, so disk space
+// used by files in all the subdirectories are counted.
+// - Symbolic links will not be followed (the size of link itself is counted,
+// the target is not)
+// - Hidden files are counted as well.
+// The following behaviours are not guaranteed, and it is recommended to avoid
+// them in the field. Their current behaviour is provided for reference only:
+// - This function doesn't care about filesystem boundaries, so it'll cross
+// filesystem boundary to count file size if there's one in the specified
+// directory.
+// - Hard links will be treated like normal files, so they could be
+// over-reported.
+// - Directories that the current user doesn't have permission to list/stat will
+// be ignored, and an error will be logged but the returned result could be
+// under-reported without error in the returned value.
+// - Deduplication (should the filesystem support it) is ignored, and the result
+// could be over-reported.
+// - Doesn't check if |root_path| exists, a non-existent directory will results
+// in 0 bytes without any error.
+// - There are no limit on the depth of file system tree, the program will crash
+// if it run out of memory to hold the entire depth of file system tree.
+// - If the directory is modified during this function call, there's no
+// guarantee on if the function will count the updated or original file system
+// state. The function could choose to count the updated state for one file and
+// original state for another file.
+// - Non-POSIX system is not supported.
+// - Disk space used by directory (and its subdirectories) itself is counted.
+//
+// Parameters
+// root_path - The directory to compute the size for
+BRILLO_EXPORT int64_t
+ComputeDirectoryDiskUsage(const base::FilePath& root_path);
+
} // namespace brillo
#endif // LIBBRILLO_BRILLO_FILE_UTILS_H_
diff --git a/brillo/file_utils_test.cc b/brillo/file_utils_test.cc
index 9a1f646..3407cd1 100644
--- a/brillo/file_utils_test.cc
+++ b/brillo/file_utils_test.cc
@@ -13,6 +13,7 @@
#include <base/files/file_util.h>
#include <base/files/scoped_temp_dir.h>
#include <base/rand_util.h>
+#include <base/stl_util.h>
#include <base/strings/string_number_conversions.h>
#include <gtest/gtest.h>
@@ -29,8 +30,8 @@
std::string GetRandomSuffix() {
const int kBufferSize = 6;
unsigned char buffer[kBufferSize];
- base::RandBytes(buffer, arraysize(buffer));
- return base::HexEncode(buffer, arraysize(buffer));
+ base::RandBytes(buffer, base::size(buffer));
+ return base::HexEncode(buffer, base::size(buffer));
}
bool IsNonBlockingFD(int fd) {
@@ -343,4 +344,165 @@
umask(old_mask);
}
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageNormalRandomFile) {
+ // 2MB test file.
+ constexpr size_t kFileSize = 2 * 1024 * 1024;
+
+ const base::FilePath dirname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ const base::FilePath filename = dirname.Append("test.temp");
+
+ std::string file_content = base::RandBytesAsString(kFileSize);
+ EXPECT_TRUE(WriteStringToFile(filename, file_content));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+ int64_t result_size = base::ComputeDirectorySize(dirname);
+
+ // result_usage (what we are testing here) should be within +/-10% of ground
+ // truth. The variation is to account for filesystem overhead variations.
+ EXPECT_GT(result_usage, kFileSize / 10 * 9);
+ EXPECT_LT(result_usage, kFileSize / 10 * 11);
+
+ // result_usage should be close to result_size, because the test file is
+ // random so it's disk usage should be similar to apparent size.
+ EXPECT_GT(result_usage, result_size / 10 * 9);
+ EXPECT_LT(result_usage, result_size / 10 * 11);
+}
+
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageDeepRandomFile) {
+ // 2MB test file.
+ constexpr size_t kFileSize = 2 * 1024 * 1024;
+
+ const base::FilePath dirname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ base::FilePath currentlevel = dirname;
+ for (int i = 0; i < 10; i++) {
+ base::FilePath nextlevel = currentlevel.Append("test.dir");
+ EXPECT_TRUE(base::CreateDirectory(nextlevel));
+ currentlevel = nextlevel;
+ }
+ const base::FilePath filename = currentlevel.Append("test.temp");
+
+ std::string file_content = base::RandBytesAsString(kFileSize);
+ EXPECT_TRUE(WriteStringToFile(filename, file_content));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+ int64_t result_size = base::ComputeDirectorySize(dirname);
+
+ // result_usage (what we are testing here) should be within +/-10% of ground
+ // truth. The variation is to account for filesystem overhead variations.
+ EXPECT_GT(result_usage, kFileSize / 10 * 9);
+ EXPECT_LT(result_usage, kFileSize / 10 * 11);
+
+ // result_usage should be close to result_size, because the test file is
+ // random so it's disk usage should be similar to apparent size.
+ EXPECT_GT(result_usage, result_size / 10 * 9);
+ EXPECT_LT(result_usage, result_size / 10 * 11);
+}
+
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageHiddenRandomFile) {
+ // 2MB test file.
+ constexpr size_t kFileSize = 2 * 1024 * 1024;
+
+ const base::FilePath dirname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ // File name starts with a dot, so it's a hidden file.
+ const base::FilePath filename = dirname.Append(".test.temp");
+
+ std::string file_content = base::RandBytesAsString(kFileSize);
+ EXPECT_TRUE(WriteStringToFile(filename, file_content));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+ int64_t result_size = base::ComputeDirectorySize(dirname);
+
+ // result_usage (what we are testing here) should be within +/-10% of ground
+ // truth. The variation is to account for filesystem overhead variations.
+ EXPECT_GT(result_usage, kFileSize / 10 * 9);
+ EXPECT_LT(result_usage, kFileSize / 10 * 11);
+
+ // result_usage should be close to result_size, because the test file is
+ // random so it's disk usage should be similar to apparent size.
+ EXPECT_GT(result_usage, result_size / 10 * 9);
+ EXPECT_LT(result_usage, result_size / 10 * 11);
+}
+
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageSparseFile) {
+ // 128MB sparse test file.
+ constexpr size_t kFileSize = 128 * 1024 * 1024;
+ constexpr size_t kFileSizeThreshold = 64 * 1024;
+
+ const base::FilePath dirname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ const base::FilePath filename = dirname.Append("test.temp");
+
+ int fd =
+ open(filename.value().c_str(), O_CREAT | O_WRONLY, S_IRUSR | S_IWUSR);
+ EXPECT_NE(fd, -1);
+ // Calling ftruncate on an empty file will create a sparse file.
+ EXPECT_EQ(0, ftruncate(fd, kFileSize));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+ int64_t result_size = base::ComputeDirectorySize(dirname);
+
+ // result_usage (what we are testing here) should be less than
+ // kFileSizeThreshold, the threshold is to account for filesystem overhead
+ // variations.
+ EXPECT_LT(result_usage, kFileSizeThreshold);
+
+ // Since we are dealing with sparse files here, the apparent size should be
+ // much much larger than the actual disk usage.
+ EXPECT_LT(result_usage, result_size / 1000);
+}
+
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageSymlinkFile) {
+ // 2MB test file.
+ constexpr size_t kFileSize = 2 * 1024 * 1024;
+
+ const base::FilePath dirname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ const base::FilePath filename = dirname.Append("test.temp");
+ const base::FilePath linkname = dirname.Append("test.link");
+
+ std::string file_content = base::RandBytesAsString(kFileSize);
+ EXPECT_TRUE(WriteStringToFile(filename, file_content));
+
+ // Create a symlink.
+ EXPECT_TRUE(base::CreateSymbolicLink(filename, linkname));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+
+ // result_usage (what we are testing here) should be within +/-10% of ground
+ // truth. The variation is to account for filesystem overhead variations.
+ // Note that it's not 2x kFileSize because symblink is not counted twice.
+ EXPECT_GT(result_usage, kFileSize / 10 * 9);
+ EXPECT_LT(result_usage, kFileSize / 10 * 11);
+}
+
+TEST_F(FileUtilsTest, ComputeDirectoryDiskUsageSymlinkDir) {
+ // 2MB test file.
+ constexpr size_t kFileSize = 2 * 1024 * 1024;
+
+ const base::FilePath parentname(GetTempName());
+ EXPECT_TRUE(base::CreateDirectory(parentname));
+ const base::FilePath dirname = parentname.Append("target.dir");
+ EXPECT_TRUE(base::CreateDirectory(dirname));
+ const base::FilePath linkname = parentname.Append("link.dir");
+
+ const base::FilePath filename = dirname.Append("test.temp");
+
+ std::string file_content = base::RandBytesAsString(kFileSize);
+ EXPECT_TRUE(WriteStringToFile(filename, file_content));
+
+ // Create a symlink.
+ EXPECT_TRUE(base::CreateSymbolicLink(dirname, linkname));
+
+ int64_t result_usage = ComputeDirectoryDiskUsage(dirname);
+
+ // result_usage (what we are testing here) should be within +/-10% of ground
+ // truth. The variation is to account for filesystem overhead variations.
+ // Note that it's not 2x kFileSize because symblink is not counted twice.
+ EXPECT_GT(result_usage, kFileSize / 10 * 9);
+ EXPECT_LT(result_usage, kFileSize / 10 * 11);
+}
+
} // namespace brillo
diff --git a/brillo/files/file_util_test.cc b/brillo/files/file_util_test.cc
index 98d28fc..f1ba527 100644
--- a/brillo/files/file_util_test.cc
+++ b/brillo/files/file_util_test.cc
@@ -6,6 +6,7 @@
#include <base/files/file_util.h>
#include <base/rand_util.h>
+#include <base/stl_util.h>
#include <base/strings/string_number_conversions.h>
#include <brillo/files/file_util.h>
#include <brillo/files/safe_fd.h>
@@ -42,8 +43,8 @@
std::string GetRandomSuffix() {
const int kBufferSize = 6;
unsigned char buffer[kBufferSize];
- base::RandBytes(buffer, arraysize(buffer));
- return base::HexEncode(buffer, arraysize(buffer));
+ base::RandBytes(buffer, base::size(buffer));
+ return base::HexEncode(buffer, base::size(buffer));
}
void FileTest::SetUpTestCase() {
diff --git a/brillo/files/safe_fd.cc b/brillo/files/safe_fd.cc
index 855207d..ac19dc3 100644
--- a/brillo/files/safe_fd.cc
+++ b/brillo/files/safe_fd.cc
@@ -446,7 +446,8 @@
SafeFD::Error SafeFD::Rmdir(const std::string& name,
bool recursive,
- size_t max_depth) {
+ size_t max_depth,
+ bool keep_going) {
if (!fd_.is_valid()) {
return SafeFD::Error::kNotInitialized;
}
@@ -460,6 +461,8 @@
return err;
}
+ SafeFD::Error last_err = SafeFD::Error::kNoError;
+
if (recursive) {
SafeFD dir_fd;
std::tie(dir_fd, err) =
@@ -490,32 +493,36 @@
errno = 0;
const dirent* entry = HANDLE_EINTR_IF_EQ(readdir(dir.get()), nullptr);
while (entry != nullptr) {
- if (strcmp(entry->d_name, ".") == 0 || strcmp(entry->d_name, "..") == 0) {
- goto continue_;
- }
+ SafeFD::Error err = [&]() {
+ if (strcmp(entry->d_name, ".") == 0 ||
+ strcmp(entry->d_name, "..") == 0) {
+ return SafeFD::Error::kNoError;
+ }
- struct stat child_info;
- if (fstatat(dir_fd.get(), entry->d_name, &child_info,
- AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW) != 0) {
- return SafeFD::Error::kIOError;
- }
+ struct stat child_info;
+ if (fstatat(dir_fd.get(), entry->d_name, &child_info,
+ AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW) != 0) {
+ return SafeFD::Error::kIOError;
+ }
- if (child_info.st_dev != dir_info.st_dev) {
- return SafeFD::Error::kBoundaryDetected;
- }
+ if (child_info.st_dev != dir_info.st_dev) {
+ return SafeFD::Error::kBoundaryDetected;
+ }
- SafeFD::Error err;
- if (entry->d_type == DT_DIR) {
- err = dir_fd.Rmdir(entry->d_name, true, max_depth - 1);
- } else {
- err = dir_fd.Unlink(entry->d_name);
- }
+ if (entry->d_type != DT_DIR) {
+ return dir_fd.Unlink(entry->d_name);
+ }
+
+ return dir_fd.Rmdir(entry->d_name, true, max_depth - 1, keep_going);
+ }();
if (IsError(err)) {
- return err;
+ if (!keep_going) {
+ return err;
+ }
+ last_err = err;
}
- continue_:
errno = 0;
entry = HANDLE_EINTR_IF_EQ(readdir(dir.get()), nullptr);
}
@@ -530,9 +537,16 @@
if (errno == ENOTDIR) {
return SafeFD::Error::kWrongType;
}
+ // If there was an error during the recursive delete, we expect unlink
+ // to fail with ENOTEMPTY and we bubble the error from recursion
+ // instead.
+ if (IsError(last_err) && errno == ENOTEMPTY) {
+ return last_err;
+ }
return SafeFD::Error::kIOError;
}
- return SafeFD::Error::kNoError;
+
+ return last_err;
}
} // namespace brillo
diff --git a/brillo/files/safe_fd.h b/brillo/files/safe_fd.h
index 5e126b4..3c77362 100644
--- a/brillo/files/safe_fd.h
+++ b/brillo/files/safe_fd.h
@@ -183,10 +183,13 @@
// recursive - if true also unlink child paths.
// max_depth - limit on recursion depth to prevent fd exhaustion and stack
// overflows.
+ // keep_going - in recursive case continue deleting even in the face of
+ // errors. If all entries cannot be deleted, the last error encountered
+ // during recursion is returned.
BRILLO_EXPORT Error Rmdir(const std::string& name,
bool recursive = false,
- size_t max_depth = kDefaultMaxPathDepth)
- WARN_UNUSED_RESULT;
+ size_t max_depth = kDefaultMaxPathDepth,
+ bool keep_going = true) WARN_UNUSED_RESULT;
private:
BRILLO_EXPORT static const char* RootPath;
diff --git a/brillo/files/safe_fd_test.cc b/brillo/files/safe_fd_test.cc
index fff3a6c..6401f3d 100644
--- a/brillo/files/safe_fd_test.cc
+++ b/brillo/files/safe_fd_test.cc
@@ -624,4 +624,75 @@
EXPECT_EQ(subdir.first.Rmdir(kFileName), SafeFD::Error::kWrongType);
}
+TEST_F(SafeFDTest, Rmdir_Recursive_KeepGoing) {
+ ASSERT_TRUE(SetupSubdir());
+
+ ASSERT_TRUE(base::CreateDirectory(sub_dir_path_.Append(kSubdirName)));
+
+ // Give us something to iterate over.
+ constexpr int kNumSentinel = 25;
+ for (int i = 0; i < kNumSentinel; i++) {
+ SafeFD::SafeFDResult file =
+ root_.MakeFile(sub_dir_path_.Append(GetRandomSuffix()));
+ EXPECT_EQ(file.second, SafeFD::Error::kNoError);
+ ASSERT_TRUE(file.first.is_valid());
+ }
+
+ // Recursively delete with a max level that is too small. Capture errno.
+ SafeFD::Error result = root_.Rmdir(kSubdirName, true /*recursive*/,
+ 1 /*max_depth*/, true /*keep_going*/);
+ int rmdir_errno = errno;
+
+ EXPECT_EQ(result, SafeFD::Error::kExceededMaximum);
+
+ // If we keep going, the last operation will be the post-order unlink of
+ // the top-level directory. This has to fail with ENOTEMPTY since we did
+ // not delete the too-deep sub-directories. This particular behavior
+ // should not be part of the API contract and this can be relaxed if the
+ // implementation is changed.
+ EXPECT_EQ(rmdir_errno, ENOTEMPTY);
+
+ // The deep directory must still exist.
+ ASSERT_TRUE(
+ base::DeleteFile(sub_dir_path_.Append(kSubdirName), false /*recursive*/));
+
+ // We cannot control the iteration order so even if we incorrectly
+ // stopped early the directory might still be empty if the deep
+ // directories were last in the iteration order. But a non-empty
+ // directory is always incorrect.
+ ASSERT_TRUE(base::IsDirectoryEmpty(sub_dir_path_));
+}
+
+TEST_F(SafeFDTest, Rmdir_Recursive_StopOnError) {
+ ASSERT_TRUE(SetupSubdir());
+
+ ASSERT_TRUE(base::CreateDirectory(sub_dir_path_.Append(kSubdirName)));
+
+ // Give us something to iterate over.
+ constexpr int kNumSentinel = 25;
+ for (int i = 0; i < kNumSentinel; i++) {
+ SafeFD::SafeFDResult file =
+ root_.MakeFile(sub_dir_path_.Append(GetRandomSuffix()));
+ EXPECT_EQ(file.second, SafeFD::Error::kNoError);
+ ASSERT_TRUE(file.first.is_valid());
+ }
+
+ // Recursively delete with a max level that is too small. Capture errno.
+ SafeFD::Error result = root_.Rmdir(kSubdirName, true /*recursive*/,
+ 1 /*max_depth*/, false /*keep_going*/);
+ int rmdir_errno = errno;
+
+ EXPECT_EQ(result, SafeFD::Error::kExceededMaximum);
+
+ // If we stop on encountering a too-deep directory, we never actually
+ // make any libc calls that encounter errors. This particular behavior
+ // should not be part of the API contract and this can be relaxed if the
+ // implementation is changed.
+ EXPECT_EQ(rmdir_errno, 0);
+
+ // The deep directory must still exist.
+ ASSERT_TRUE(
+ base::DeleteFile(sub_dir_path_.Append(kSubdirName), false /*recursive*/));
+}
+
} // namespace brillo
diff --git a/brillo/flag_helper.cc b/brillo/flag_helper.cc
index 065a1c7..1c332d3 100644
--- a/brillo/flag_helper.cc
+++ b/brillo/flag_helper.cc
@@ -86,6 +86,22 @@
return "int";
}
+UInt32Flag::UInt32Flag(const char* name,
+ uint32_t* value,
+ const char* default_value,
+ const char* help,
+ bool visible)
+ : Flag(name, default_value, help, visible), value_(value) {
+}
+
+bool UInt32Flag::SetValue(const std::string& value) {
+ return base::StringToUint(value, value_);
+}
+
+const char* UInt32Flag::GetType() const {
+ return "uint32";
+}
+
Int64Flag::Int64Flag(const char* name,
int64_t* value,
const char* default_value,
diff --git a/brillo/flag_helper.h b/brillo/flag_helper.h
index 810a00c..c6d63cd 100644
--- a/brillo/flag_helper.h
+++ b/brillo/flag_helper.h
@@ -20,6 +20,7 @@
//
// DEFINE_bool(name, default_value, help)
// DEFINE_int32(name, default_value, help)
+// DEFINE_uint32(name, default_value, help)
// DEFINE_int64(name, default_value, help)
// DEFINE_uint64(name, default_value, help)
// DEFINE_double(name, default_value, help)
@@ -118,6 +119,21 @@
int* value_;
};
+class BRILLO_EXPORT UInt32Flag final : public Flag {
+ public:
+ UInt32Flag(const char* name,
+ uint32_t* value,
+ const char* default_value,
+ const char* help,
+ bool visible);
+ bool SetValue(const std::string& value) override;
+
+ const char* GetType() const override;
+
+ private:
+ uint32_t* value_;
+};
+
class BRILLO_EXPORT Int64Flag final : public Flag {
public:
Int64Flag(const char* name,
@@ -191,6 +207,8 @@
#define DEFINE_int32(name, value, help) \
DEFINE_type(int, Int32Flag, name, value, help)
+#define DEFINE_uint32(name, value, help) \
+ DEFINE_type(uint32_t, UInt32Flag, name, value, help)
#define DEFINE_int64(name, value, help) \
DEFINE_type(int64_t, Int64Flag, name, value, help)
#define DEFINE_uint64(name, value, help) \
diff --git a/brillo/flag_helper_test.cc b/brillo/flag_helper_test.cc
index 29c6429..7c7164d 100644
--- a/brillo/flag_helper_test.cc
+++ b/brillo/flag_helper_test.cc
@@ -8,6 +8,7 @@
#include <base/command_line.h>
#include <base/macros.h>
+#include <base/stl_util.h>
#include <brillo/flag_helper.h>
#include <gtest/gtest.h>
@@ -29,6 +30,8 @@
DEFINE_int32(int32_1, INT32_MIN, "Test int32 flag");
DEFINE_int32(int32_2, 0, "Test int32 flag");
DEFINE_int32(int32_3, INT32_MAX, "Test int32 flag");
+ DEFINE_uint32(uint32_1, 0, "Test uint32 flag");
+ DEFINE_uint32(uint32_2, UINT32_MAX, "Test uint32 flag");
DEFINE_int64(int64_1, INT64_MIN, "Test int64 flag");
DEFINE_int64(int64_2, 0, "Test int64 flag");
DEFINE_int64(int64_3, INT64_MAX, "Test int64 flag");
@@ -41,17 +44,19 @@
DEFINE_string(string_2, "value", "Test string flag");
const char* argv[] = {"test_program"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestDefaultTrue");
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestDefaultTrue");
EXPECT_TRUE(FLAGS_bool1);
EXPECT_FALSE(FLAGS_bool2);
EXPECT_EQ(FLAGS_int32_1, INT32_MIN);
EXPECT_EQ(FLAGS_int32_2, 0);
EXPECT_EQ(FLAGS_int32_3, INT32_MAX);
+ EXPECT_EQ(FLAGS_uint32_1, 0);
+ EXPECT_EQ(FLAGS_uint32_2, UINT32_MAX);
EXPECT_EQ(FLAGS_int64_1, INT64_MIN);
EXPECT_EQ(FLAGS_int64_2, 0);
EXPECT_EQ(FLAGS_int64_3, INT64_MAX);
@@ -74,6 +79,8 @@
DEFINE_int32(int32_1, 1, "Test int32 flag");
DEFINE_int32(int32_2, 1, "Test int32 flag");
DEFINE_int32(int32_3, 1, "Test int32 flag");
+ DEFINE_uint32(uint32_1, 1, "Test uint32 flag");
+ DEFINE_uint32(uint32_2, 1, "Test uint32 flag");
DEFINE_int64(int64_1, 1, "Test int64 flag");
DEFINE_int64(int64_2, 1, "Test int64 flag");
DEFINE_int64(int64_3, 1, "Test int64 flag");
@@ -93,6 +100,8 @@
"--int32_1=-2147483648",
"--int32_2=0",
"--int32_3=2147483647",
+ "--uint32_1=0",
+ "--uint32_2=4294967295",
"--int64_1=-9223372036854775808",
"--int64_2=0",
"--int64_3=9223372036854775807",
@@ -103,11 +112,11 @@
"--double_3=100.5",
"--string_1=",
"--string_2=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestDefaultTrue");
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestDefaultTrue");
EXPECT_TRUE(FLAGS_bool1);
EXPECT_FALSE(FLAGS_bool2);
@@ -116,6 +125,8 @@
EXPECT_EQ(FLAGS_int32_1, INT32_MIN);
EXPECT_EQ(FLAGS_int32_2, 0);
EXPECT_EQ(FLAGS_int32_3, INT32_MAX);
+ EXPECT_EQ(FLAGS_uint32_1, 0);
+ EXPECT_EQ(FLAGS_uint32_2, UINT32_MAX);
EXPECT_EQ(FLAGS_int64_1, INT64_MIN);
EXPECT_EQ(FLAGS_int64_2, 0);
EXPECT_EQ(FLAGS_int64_3, INT64_MAX);
@@ -136,6 +147,8 @@
DEFINE_int32(int32_1, 1, "Test int32 flag");
DEFINE_int32(int32_2, 1, "Test int32 flag");
DEFINE_int32(int32_3, 1, "Test int32 flag");
+ DEFINE_uint64(uint32_1, 1, "Test uint32 flag");
+ DEFINE_uint64(uint32_2, 1, "Test uint32 flag");
DEFINE_int64(int64_1, 1, "Test int64 flag");
DEFINE_int64(int64_2, 1, "Test int64 flag");
DEFINE_int64(int64_3, 1, "Test int64 flag");
@@ -153,6 +166,8 @@
"-int32_1=-2147483648",
"-int32_2=0",
"-int32_3=2147483647",
+ "-uint32_1=0",
+ "-uint32_2=4294967295",
"-int64_1=-9223372036854775808",
"-int64_2=0",
"-int64_3=9223372036854775807",
@@ -163,17 +178,19 @@
"-double_3=100.5",
"-string_1=",
"-string_2=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestDefaultTrue");
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestDefaultTrue");
EXPECT_TRUE(FLAGS_bool1);
EXPECT_FALSE(FLAGS_bool2);
EXPECT_EQ(FLAGS_int32_1, INT32_MIN);
EXPECT_EQ(FLAGS_int32_2, 0);
EXPECT_EQ(FLAGS_int32_3, INT32_MAX);
+ EXPECT_EQ(FLAGS_uint32_1, 0);
+ EXPECT_EQ(FLAGS_uint32_2, UINT32_MAX);
EXPECT_EQ(FLAGS_int64_1, INT64_MIN);
EXPECT_EQ(FLAGS_int64_2, 0);
EXPECT_EQ(FLAGS_int64_3, INT64_MAX);
@@ -192,11 +209,11 @@
DEFINE_int32(int32_1, 0, "Test in32 flag");
const char* argv[] = {"test_program", "--int32_1=5", "--int32_1=10"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestDuplicateSetvalue");
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestDuplicateSetvalue");
EXPECT_EQ(FLAGS_int32_1, 10);
}
@@ -206,11 +223,11 @@
DEFINE_int32(int32_1, 0, "Test int32 flag");
const char* argv[] = {"test_program", "--int32_1=5", "--", "--int32_1=10"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestFlagTerminator");
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestFlagTerminator");
EXPECT_EQ(FLAGS_int32_1, 5);
}
@@ -220,13 +237,14 @@
TEST_F(FlagHelperTest, HelpMessage) {
DEFINE_bool(bool_1, true, "Test bool flag");
DEFINE_int32(int_1, 0, "Test int flag");
+ DEFINE_uint32(uint32_1, 0, "Test uint32 flag");
DEFINE_int64(int64_1, 0, "Test int64 flag");
DEFINE_uint64(uint64_1, 0, "Test uint64 flag");
DEFINE_double(double_1, 0, "Test double flag");
DEFINE_string(string_1, "", "Test string flag");
const char* argv[] = {"test_program", "--int_1=value", "--help"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -235,7 +253,7 @@
stdout = stderr;
ASSERT_EXIT(
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestHelpMessage"),
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestHelpMessage"),
::testing::ExitedWithCode(EX_OK),
"TestHelpMessage\n\n"
" --bool_1 \\(Test bool flag\\) type: bool default: true\n"
@@ -244,6 +262,7 @@
" --int64_1 \\(Test int64 flag\\) type: int64 default: 0\n"
" --int_1 \\(Test int flag\\) type: int default: 0\n"
" --string_1 \\(Test string flag\\) type: string default: \"\"\n"
+ " --uint32_1 \\(Test uint32 flag\\) type: uint32 default: 0\n"
" --uint64_1 \\(Test uint64 flag\\) type: uint64 default: 0\n");
stdout = orig;
@@ -253,7 +272,7 @@
// to exit with EX_USAGE error code and corresponding error message.
TEST_F(FlagHelperTest, UnknownFlag) {
const char* argv[] = {"test_program", "--flag=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -261,7 +280,7 @@
FILE* orig = stdout;
stdout = stderr;
- ASSERT_EXIT(brillo::FlagHelper::Init(arraysize(argv), argv, "TestIntExit"),
+ ASSERT_EXIT(brillo::FlagHelper::Init(base::size(argv), argv, "TestIntExit"),
::testing::ExitedWithCode(EX_USAGE),
"ERROR: unknown command line flag 'flag'");
@@ -274,7 +293,7 @@
DEFINE_bool(bool_1, 0, "Test bool flag");
const char* argv[] = {"test_program", "--bool_1=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -283,7 +302,7 @@
stdout = stderr;
ASSERT_EXIT(
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestBoolParseError"),
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestBoolParseError"),
::testing::ExitedWithCode(EX_DATAERR),
"ERROR: illegal value 'value' specified for bool flag 'bool_1'");
@@ -296,7 +315,7 @@
DEFINE_int32(int_1, 0, "Test int flag");
const char* argv[] = {"test_program", "--int_1=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -304,11 +323,57 @@
FILE* orig = stdout;
stdout = stderr;
- ASSERT_EXIT(brillo::FlagHelper::Init(arraysize(argv),
- argv,
- "TestInt32ParseError"),
- ::testing::ExitedWithCode(EX_DATAERR),
- "ERROR: illegal value 'value' specified for int flag 'int_1'");
+ ASSERT_EXIT(
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestInt32ParseError"),
+ ::testing::ExitedWithCode(EX_DATAERR),
+ "ERROR: illegal value 'value' specified for int flag 'int_1'");
+
+ stdout = orig;
+}
+
+// Test that when passing an incorrect/unparsable type to a command line flag,
+// the program exits with code EX_DATAERR and outputs a corresponding message.
+TEST_F(FlagHelperTest, Uint32ParseErrorUppperBound) {
+ DEFINE_uint32(uint32_1, 0, "Test uint32 flag");
+
+ // test with UINT32_MAX + 1
+ const char* argv[] = {"test_program", "--uint32_1=4294967296"};
+ base::CommandLine command_line(base::size(argv), argv);
+
+ brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
+ &command_line);
+
+ FILE* orig = stdout;
+ stdout = stderr;
+
+ ASSERT_EXIT(
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestUint32ParseError"),
+ ::testing::ExitedWithCode(EX_DATAERR),
+ "ERROR: illegal value '4294967296' specified for uint32 flag "
+ "'uint32_1'");
+
+ stdout = orig;
+}
+
+// Test that when passing an incorrect/unparsable type to a command line flag,
+// the program exits with code EX_DATAERR and outputs a corresponding message.
+TEST_F(FlagHelperTest, Uint32ParseErrorNegativeValue) {
+ DEFINE_uint32(uint32_1, 0, "Test uint32 flag");
+
+ const char* argv[] = {"test_program", "--uint32_1=-1"};
+ base::CommandLine command_line(base::size(argv), argv);
+
+ brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
+ &command_line);
+
+ FILE* orig = stdout;
+ stdout = stderr;
+
+ ASSERT_EXIT(
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestUint32ParseError"),
+ ::testing::ExitedWithCode(EX_DATAERR),
+ "ERROR: illegal value '-1' specified for uint32 flag "
+ "'uint32_1'");
stdout = orig;
}
@@ -319,7 +384,7 @@
DEFINE_int64(int64_1, 0, "Test int64 flag");
const char* argv[] = {"test_program", "--int64_1=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -328,7 +393,7 @@
stdout = stderr;
ASSERT_EXIT(
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestInt64ParseError"),
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestInt64ParseError"),
::testing::ExitedWithCode(EX_DATAERR),
"ERROR: illegal value 'value' specified for int64 flag "
"'int64_1'");
@@ -342,7 +407,7 @@
DEFINE_uint64(uint64_1, 0, "Test uint64 flag");
const char* argv[] = {"test_program", "--uint64_1=value"};
- base::CommandLine command_line(arraysize(argv), argv);
+ base::CommandLine command_line(base::size(argv), argv);
brillo::FlagHelper::GetInstance()->set_command_line_for_testing(
&command_line);
@@ -351,7 +416,7 @@
stdout = stderr;
ASSERT_EXIT(
- brillo::FlagHelper::Init(arraysize(argv), argv, "TestUInt64ParseError"),
+ brillo::FlagHelper::Init(base::size(argv), argv, "TestUInt64ParseError"),
::testing::ExitedWithCode(EX_DATAERR),
"ERROR: illegal value 'value' specified for uint64 flag "
"'uint64_1'");
diff --git a/brillo/http/http_proxy_test.cc b/brillo/http/http_proxy_test.cc
index eb44263..a0d1bfa 100644
--- a/brillo/http/http_proxy_test.cc
+++ b/brillo/http/http_proxy_test.cc
@@ -32,23 +32,20 @@
public:
void ResolveProxyHandlerAsync(dbus::MethodCall* method_call,
int timeout_msec,
- dbus::ObjectProxy::ResponseCallback
- MIGRATE_WrapObjectProxyCallback(callback)) {
+ dbus::ObjectProxy::ResponseCallback* callback) {
if (null_dbus_response_) {
- std::move(MIGRATE_WrapObjectProxyCallback(callback)).Run(nullptr);
+ std::move(*callback).Run(nullptr);
return;
}
- std::move(MIGRATE_WrapObjectProxyCallback(callback))
- .Run(CreateDBusResponse(method_call).get());
+ std::move(*callback).Run(CreateDBusResponse(method_call).get());
}
- MIGRATE_WrapObjectProxyResponseType(dbus::Response)
- ResolveProxyHandler(dbus::MethodCall* method_call, int timeout_msec) {
+ std::unique_ptr<dbus::Response> ResolveProxyHandler(
+ dbus::MethodCall* method_call, int timeout_msec) {
if (null_dbus_response_) {
- return MIGRATE_WrapObjectProxyResponseEmpty;
+ return std::unique_ptr<dbus::Response>();
}
- return MIGRATE_WrapObjectProxyResponseConversion(
- CreateDBusResponse(method_call));
+ return CreateDBusResponse(method_call);
}
MOCK_METHOD(void,
@@ -102,7 +99,7 @@
TEST_F(HttpProxyTest, DBusNullResponseFails) {
std::vector<std::string> proxies;
null_dbus_response_ = true;
- EXPECT_CALL(*object_proxy_, MIGRATE_MockCallMethodAndBlock(_, _))
+ EXPECT_CALL(*object_proxy_, CallMethodAndBlock(_, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler));
EXPECT_FALSE(GetChromeProxyServers(bus_, kTestUrl, &proxies));
}
@@ -110,14 +107,14 @@
TEST_F(HttpProxyTest, DBusInvalidResponseFails) {
std::vector<std::string> proxies;
invalid_dbus_response_ = true;
- EXPECT_CALL(*object_proxy_, MIGRATE_MockCallMethodAndBlock(_, _))
+ EXPECT_CALL(*object_proxy_, CallMethodAndBlock(_, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler));
EXPECT_FALSE(GetChromeProxyServers(bus_, kTestUrl, &proxies));
}
TEST_F(HttpProxyTest, NoProxies) {
std::vector<std::string> proxies;
- EXPECT_CALL(*object_proxy_, MIGRATE_MockCallMethodAndBlock(_, _))
+ EXPECT_CALL(*object_proxy_, CallMethodAndBlock(_, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler));
EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies));
EXPECT_THAT(proxies, ElementsAre(kDirectProxy));
@@ -126,7 +123,7 @@
TEST_F(HttpProxyTest, MultipleProxiesWithoutDirect) {
proxy_info_ = "proxy example.com; socks5 foo.com;";
std::vector<std::string> proxies;
- EXPECT_CALL(*object_proxy_, MIGRATE_MockCallMethodAndBlock(_, _))
+ EXPECT_CALL(*object_proxy_, CallMethodAndBlock(_, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler));
EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies));
EXPECT_THAT(proxies, ElementsAre("http://example.com", "socks5://foo.com",
@@ -137,7 +134,7 @@
proxy_info_ = "socks foo.com; Https example.com ; badproxy example2.com ; "
"socks5 test.com ; proxy foobar.com; DIRECT ";
std::vector<std::string> proxies;
- EXPECT_CALL(*object_proxy_, MIGRATE_MockCallMethodAndBlock(_, _))
+ EXPECT_CALL(*object_proxy_, CallMethodAndBlock(_, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandler));
EXPECT_TRUE(GetChromeProxyServers(bus_, kTestUrl, &proxies));
EXPECT_THAT(proxies, ElementsAre("socks4://foo.com", "https://example.com",
@@ -147,7 +144,7 @@
TEST_F(HttpProxyTest, DBusNullResponseFailsAsync) {
null_dbus_response_ = true;
- EXPECT_CALL(*object_proxy_, MIGRATE_CallMethod(_, _, _))
+ EXPECT_CALL(*object_proxy_, DoCallMethod(_, _, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync));
EXPECT_CALL(*this, GetProxiesCallback(false, _));
GetChromeProxyServersAsync(
@@ -157,7 +154,7 @@
TEST_F(HttpProxyTest, DBusInvalidResponseFailsAsync) {
invalid_dbus_response_ = true;
- EXPECT_CALL(*object_proxy_, MIGRATE_CallMethod(_, _, _))
+ EXPECT_CALL(*object_proxy_, DoCallMethod(_, _, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync));
EXPECT_CALL(*this, GetProxiesCallback(false, _));
GetChromeProxyServersAsync(
@@ -173,7 +170,7 @@
std::vector<std::string> expected = {
"socks4://foo.com", "https://example.com", "socks5://test.com",
"http://foobar.com", kDirectProxy};
- EXPECT_CALL(*object_proxy_, MIGRATE_CallMethod(_, _, _))
+ EXPECT_CALL(*object_proxy_, DoCallMethod(_, _, _))
.WillOnce(Invoke(this, &HttpProxyTest::ResolveProxyHandlerAsync));
EXPECT_CALL(*this, GetProxiesCallback(true, expected));
GetChromeProxyServersAsync(
diff --git a/brillo/http/http_transport_curl.cc b/brillo/http/http_transport_curl.cc
index 741cb3c..de6899a 100644
--- a/brillo/http/http_transport_curl.cc
+++ b/brillo/http/http_transport_curl.cc
@@ -10,8 +10,8 @@
#include <base/files/file_descriptor_watcher_posix.h>
#include <base/files/file_util.h>
#include <base/logging.h>
-#include <base/message_loop/message_loop.h>
#include <base/strings/stringprintf.h>
+#include <base/threading/thread_task_runner_handle.h>
#include <brillo/http/http_connection_curl.h>
#include <brillo/http/http_request.h>
#include <brillo/strings/string_utils.h>
@@ -222,8 +222,7 @@
void Transport::RunCallbackAsync(const base::Location& from_here,
const base::Closure& callback) {
- base::MessageLoopForIO::current()->task_runner()->PostTask(
- from_here, callback);
+ base::ThreadTaskRunnerHandle::Get()->PostTask(from_here, callback);
}
RequestID Transport::StartAsyncTransfer(http::Connection* connection,
@@ -399,8 +398,7 @@
poll_data->StopWatcher();
// This method can be called indirectly from SocketPollData::OnSocketReady,
// so delay destruction of SocketPollData object till the next loop cycle.
- base::MessageLoopForIO::current()->task_runner()->DeleteSoon(FROM_HERE,
- poll_data);
+ base::ThreadTaskRunnerHandle::Get()->DeleteSoon(FROM_HERE, poll_data);
return 0;
}
@@ -424,11 +422,11 @@
// Cancel any previous timer callbacks.
transport->weak_ptr_factory_for_timer_.InvalidateWeakPtrs();
if (timeout_ms >= 0) {
- base::MessageLoopForIO::current()->task_runner()->PostDelayedTask(
- FROM_HERE,
- base::Bind(&Transport::OnTimer,
- transport->weak_ptr_factory_for_timer_.GetWeakPtr()),
- base::TimeDelta::FromMilliseconds(timeout_ms));
+ base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+ FROM_HERE,
+ base::Bind(&Transport::OnTimer,
+ transport->weak_ptr_factory_for_timer_.GetWeakPtr()),
+ base::TimeDelta::FromMilliseconds(timeout_ms));
}
return 0;
}
diff --git a/brillo/http/http_transport_curl_test.cc b/brillo/http/http_transport_curl_test.cc
index 40ef23e..6e94978 100644
--- a/brillo/http/http_transport_curl_test.cc
+++ b/brillo/http/http_transport_curl_test.cc
@@ -8,6 +8,7 @@
#include <base/bind.h>
#include <base/message_loop/message_loop.h>
#include <base/run_loop.h>
+#include <base/threading/thread_task_runner_handle.h>
#include <brillo/http/http_connection_curl.h>
#include <brillo/http/http_request.h>
#include <brillo/http/mock_curl_api.h>
@@ -242,7 +243,7 @@
auto success_callback = base::Bind([](
int* success_call_count, const base::Closure& quit_closure,
RequestID /* request_id */, std::unique_ptr<http::Response> /* resp */) {
- base::MessageLoop::current()->task_runner()->PostTask(
+ base::ThreadTaskRunnerHandle::Get()->PostTask(
FROM_HERE, quit_closure);
(*success_call_count)++;
}, &success_call_count, run_loop.QuitClosure());
diff --git a/brillo/http/http_transport_fake.cc b/brillo/http/http_transport_fake.cc
index c4757f9..8fd9d2c 100644
--- a/brillo/http/http_transport_fake.cc
+++ b/brillo/http/http_transport_fake.cc
@@ -180,7 +180,9 @@
if (brillo::mime::RemoveParameters(
GetHeader(request_header::kContentType)) ==
brillo::mime::application::kJson) {
- auto value = base::JSONReader::Read(GetDataAsString());
+ // TODO(crbug.com/1054279): use base::JSONReader::Read after uprev to
+ // r680000.
+ auto value = base::JSONReader::ReadDeprecated(GetDataAsString());
result = base::DictionaryValue::From(std::move(value));
}
return result;
diff --git a/brillo/http/http_utils.cc b/brillo/http/http_utils.cc
index 6132d11..55e0bf3 100644
--- a/brillo/http/http_utils.cc
+++ b/brillo/http/http_utils.cc
@@ -396,8 +396,10 @@
std::string json = response->ExtractDataAsString();
std::string error_message;
- auto value = base::JSONReader::ReadAndReturnError(json, base::JSON_PARSE_RFC,
- nullptr, &error_message);
+ // TODO(crbug.com/1054279): use base::JSONReader::ReadAndReturnValueWithError
+ // after uprev to r680000.
+ auto value = base::JSONReader::ReadAndReturnErrorDeprecated(
+ json, base::JSON_PARSE_RFC, nullptr, &error_message);
if (!value) {
brillo::Error::AddToPrintf(error, FROM_HERE, brillo::errors::json::kDomain,
brillo::errors::json::kParseError,
diff --git a/brillo/message_loops/base_message_loop.cc b/brillo/message_loops/base_message_loop.cc
index 0c2b9db..c3499ab 100644
--- a/brillo/message_loops/base_message_loop.cc
+++ b/brillo/message_loops/base_message_loop.cc
@@ -30,12 +30,11 @@
#include <base/run_loop.h>
#include <base/strings/string_number_conversions.h>
#include <base/strings/string_split.h>
+#include <base/threading/thread_task_runner_handle.h>
#include <brillo/location_logging.h>
#include <brillo/strings/string_utils.h>
-using base::Closure;
-
namespace {
const char kMiscMinorPath[] = "/proc/misc";
@@ -49,7 +48,7 @@
const int BaseMessageLoop::kUninitializedMinor = -2;
BaseMessageLoop::BaseMessageLoop() {
- CHECK(!base::MessageLoop::current())
+ CHECK(!base::ThreadTaskRunnerHandle::IsSet())
<< "You can't create a base::MessageLoopForIO when another "
"base::MessageLoop is already created for this thread.";
owned_base_loop_.reset(new base::MessageLoopForIO());
@@ -80,21 +79,21 @@
MessageLoop::TaskId BaseMessageLoop::PostDelayedTask(
const base::Location& from_here,
- const Closure &task,
+ base::OnceClosure task,
base::TimeDelta delay) {
TaskId task_id = NextTaskId();
bool base_scheduled = base_loop_->task_runner()->PostDelayedTask(
from_here,
- base::Bind(&BaseMessageLoop::OnRanPostedTask,
- weak_ptr_factory_.GetWeakPtr(),
- task_id),
+ base::BindOnce(&BaseMessageLoop::OnRanPostedTask,
+ weak_ptr_factory_.GetWeakPtr(), task_id),
delay);
DVLOG_LOC(from_here, 1) << "Scheduling delayed task_id " << task_id
<< " to run in " << delay << ".";
if (!base_scheduled)
return MessageLoop::kTaskIdNull;
- delayed_tasks_.emplace(task_id, DelayedTask{from_here, task_id, task});
+ delayed_tasks_.emplace(task_id,
+ DelayedTask{from_here, task_id, std::move(task)});
return task_id;
}
@@ -114,10 +113,10 @@
DVLOG_LOC(delayed_task_it->second.location, 1)
<< "Removing task_id " << task_id << " scheduled from this location.";
- // We reset to closure to a null Closure to release all the resources
+ // We reset to closure to a null OnceClosure to release all the resources
// used by this closure at this point, but we don't remove the task_id from
// delayed_tasks_ since we can't tell base::MessageLoopForIO to not run it.
- delayed_task_it->second.closure = Closure();
+ delayed_task_it->second.closure.Reset();
return true;
}
@@ -154,7 +153,7 @@
base_run_loop_->Quit();
}
-Closure BaseMessageLoop::QuitClosure() const {
+base::RepeatingClosure BaseMessageLoop::QuitClosure() const {
if (base_run_loop_ == nullptr)
return base::DoNothing();
return base_run_loop_->QuitClosure();
@@ -179,9 +178,7 @@
<< " scheduled from this location.";
// Mark the task as canceled while we are running it so CancelTask returns
// false.
- Closure closure = std::move(task_it->second.closure);
- task_it->second.closure = Closure();
- closure.Run();
+ std::move(task_it->second.closure).Run();
// If the |run_once_| flag is set, it is because we are instructed to run
// only once callback.
diff --git a/brillo/message_loops/base_message_loop.h b/brillo/message_loops/base_message_loop.h
index 92c5bda..75e4361 100644
--- a/brillo/message_loops/base_message_loop.h
+++ b/brillo/message_loops/base_message_loop.h
@@ -42,7 +42,7 @@
// MessageLoop overrides.
TaskId PostDelayedTask(const base::Location& from_here,
- const base::Closure& task,
+ base::OnceClosure task,
base::TimeDelta delay) override;
using MessageLoop::PostDelayedTask;
bool CancelTask(TaskId task_id) override;
@@ -52,7 +52,7 @@
// Returns a callback that will quit the current message loop. If the message
// loop is not running, an empty (null) callback is returned.
- base::Closure QuitClosure() const;
+ base::RepeatingClosure QuitClosure() const;
private:
FRIEND_TEST(BaseMessageLoopTest, ParseBinderMinor);
@@ -79,7 +79,7 @@
base::Location location;
MessageLoop::TaskId task_id;
- base::Closure closure;
+ base::OnceClosure closure;
};
// The base::MessageLoopForIO instance owned by this class, if any. This
diff --git a/brillo/message_loops/fake_message_loop.cc b/brillo/message_loops/fake_message_loop.cc
index 9ab2aa9..185b20c 100644
--- a/brillo/message_loops/fake_message_loop.cc
+++ b/brillo/message_loops/fake_message_loop.cc
@@ -15,7 +15,7 @@
MessageLoop::TaskId FakeMessageLoop::PostDelayedTask(
const base::Location& from_here,
- const base::Closure& task,
+ base::OnceClosure task,
base::TimeDelta delay) {
// If no SimpleTestClock was provided, we use the last time we fired a
// callback. In this way, tasks scheduled from a Closure will have the right
@@ -25,7 +25,7 @@
MessageLoop::TaskId current_id = ++last_id_;
// FakeMessageLoop is limited to only 2^64 tasks. That should be enough.
CHECK(current_id);
- tasks_.emplace(current_id, ScheduledTask{from_here, false, task});
+ tasks_.emplace(current_id, ScheduledTask{from_here, std::move(task)});
fire_order_.push(std::make_pair(current_time_ + delay, current_id));
VLOG_LOC(from_here, 1) << "Scheduling delayed task_id " << current_id
<< " to run at " << current_time_ + delay
@@ -64,13 +64,13 @@
// Move the Closure out of the map before delete it. We need to delete the
// entry from the map before we call the callback, since calling CancelTask
// for the task you are running now should fail and return false.
- base::Closure callback = std::move(scheduled_task_ref->second.callback);
+ base::OnceClosure callback = std::move(scheduled_task_ref->second.callback);
VLOG_LOC(scheduled_task_ref->second.location, 1)
<< "Running task_id " << task_ref.second
<< " at time " << current_time_ << " from this location.";
tasks_.erase(scheduled_task_ref);
- callback.Run();
+ std::move(callback).Run();
return true;
}
return false;
@@ -79,8 +79,7 @@
bool FakeMessageLoop::PendingTasks() {
for (const auto& task : tasks_) {
VLOG_LOC(task.second.location, 1)
- << "Pending " << (task.second.persistent ? "persistent " : "")
- << "task_id " << task.first << " scheduled from here.";
+ << "Pending task_id " << task.first << " scheduled from here.";
}
return !tasks_.empty();
}
diff --git a/brillo/message_loops/fake_message_loop.h b/brillo/message_loops/fake_message_loop.h
index 45bab81..783af1b 100644
--- a/brillo/message_loops/fake_message_loop.h
+++ b/brillo/message_loops/fake_message_loop.h
@@ -37,7 +37,7 @@
~FakeMessageLoop() override = default;
TaskId PostDelayedTask(const base::Location& from_here,
- const base::Closure& task,
+ base::OnceClosure task,
base::TimeDelta delay) override;
using MessageLoop::PostDelayedTask;
bool CancelTask(TaskId task_id) override;
@@ -52,8 +52,7 @@
private:
struct ScheduledTask {
base::Location location;
- bool persistent;
- base::Closure callback;
+ base::OnceClosure callback;
};
// The sparse list of scheduled pending callbacks.
diff --git a/brillo/message_loops/fake_message_loop_test.cc b/brillo/message_loops/fake_message_loop_test.cc
index 07af569..a5d0607 100644
--- a/brillo/message_loops/fake_message_loop_test.cc
+++ b/brillo/message_loops/fake_message_loop_test.cc
@@ -15,7 +15,7 @@
#include <brillo/message_loops/message_loop.h>
-using base::Bind;
+using base::BindOnce;
using base::Time;
using base::TimeDelta;
using std::vector;
@@ -45,17 +45,18 @@
TEST_F(FakeMessageLoopTest, PostDelayedTaskRunsInOrder) {
vector<int> order;
- auto callback = [](std::vector<int>* order, int value) {
- order->push_back(value);
- };
- loop_->PostDelayedTask(Bind(callback, base::Unretained(&order), 1),
- TimeDelta::FromSeconds(1));
- loop_->PostDelayedTask(Bind(callback, base::Unretained(&order), 4),
- TimeDelta::FromSeconds(4));
- loop_->PostDelayedTask(Bind(callback, base::Unretained(&order), 3),
- TimeDelta::FromSeconds(3));
- loop_->PostDelayedTask(Bind(callback, base::Unretained(&order), 2),
- TimeDelta::FromSeconds(2));
+ loop_->PostDelayedTask(
+ BindOnce([](vector<int>* order) { order->push_back(1); }, &order),
+ TimeDelta::FromSeconds(1));
+ loop_->PostDelayedTask(
+ BindOnce([](vector<int>* order) { order->push_back(4); }, &order),
+ TimeDelta::FromSeconds(4));
+ loop_->PostDelayedTask(
+ BindOnce([](vector<int>* order) { order->push_back(3); }, &order),
+ TimeDelta::FromSeconds(3));
+ loop_->PostDelayedTask(
+ BindOnce([](vector<int>* order) { order->push_back(2); }, &order),
+ TimeDelta::FromSeconds(2));
// Run until all the tasks are run.
loop_->Run();
EXPECT_EQ((vector<int>{1, 2, 3, 4}), order);
diff --git a/brillo/message_loops/message_loop.h b/brillo/message_loops/message_loop.h
index 0f9740d..13c4dc2 100644
--- a/brillo/message_loops/message_loop.h
+++ b/brillo/message_loops/message_loop.h
@@ -6,6 +6,7 @@
#define LIBBRILLO_BRILLO_MESSAGE_LOOPS_MESSAGE_LOOP_H_
#include <string>
+#include <utility>
#include <base/callback.h>
#include <base/location.h>
@@ -50,21 +51,20 @@
// at a later point.
// This methond can only be called from the same thread running the main loop.
virtual TaskId PostDelayedTask(const base::Location& from_here,
- const base::Closure& task,
+ base::OnceClosure task,
base::TimeDelta delay) = 0;
// Variant without the Location for easier usage.
- TaskId PostDelayedTask(const base::Closure& task, base::TimeDelta delay) {
- return PostDelayedTask(base::Location(), task, delay);
+ TaskId PostDelayedTask(base::OnceClosure task, base::TimeDelta delay) {
+ return PostDelayedTask(base::Location(), std::move(task), delay);
}
// A convenience method to schedule a call with no delay.
// This methond can only be called from the same thread running the main loop.
- TaskId PostTask(const base::Closure& task) {
- return PostDelayedTask(task, base::TimeDelta());
+ TaskId PostTask(base::OnceClosure task) {
+ return PostDelayedTask(std::move(task), base::TimeDelta());
}
- TaskId PostTask(const base::Location& from_here,
- const base::Closure& task) {
- return PostDelayedTask(from_here, task, base::TimeDelta());
+ TaskId PostTask(const base::Location& from_here, base::OnceClosure task) {
+ return PostDelayedTask(from_here, std::move(task), base::TimeDelta());
}
// Cancel a scheduled task. Returns whether the task was canceled. For
diff --git a/brillo/message_loops/message_loop_test.cc b/brillo/message_loops/message_loop_test.cc
index 45a3b95..86c41a6 100644
--- a/brillo/message_loops/message_loop_test.cc
+++ b/brillo/message_loops/message_loop_test.cc
@@ -22,12 +22,13 @@
#include <brillo/message_loops/message_loop_utils.h>
#include <brillo/unittest_utils.h>
-using base::Bind;
+using base::BindOnce;
+using base::BindRepeating;
using base::TimeDelta;
namespace {
-// Convenience functions for passing to base::Bind.
+// Convenience functions for passing to base::Bind{Once,Repeating}.
void SetToTrue(bool* b) {
*b = true;
}
@@ -80,7 +81,8 @@
TYPED_TEST(MessageLoopTest, PostTaskTest) {
bool called = false;
- TaskId task_id = this->loop_->PostTask(FROM_HERE, Bind(&SetToTrue, &called));
+ TaskId task_id =
+ this->loop_->PostTask(FROM_HERE, BindOnce(&SetToTrue, &called));
EXPECT_NE(MessageLoop::kTaskIdNull, task_id);
MessageLoopRunMaxIterations(this->loop_.get(), 100);
EXPECT_TRUE(called);
@@ -89,7 +91,8 @@
// Tests that we can cancel tasks right after we schedule them.
TYPED_TEST(MessageLoopTest, PostTaskCancelledTest) {
bool called = false;
- TaskId task_id = this->loop_->PostTask(FROM_HERE, Bind(&SetToTrue, &called));
+ TaskId task_id =
+ this->loop_->PostTask(FROM_HERE, BindOnce(&SetToTrue, &called));
EXPECT_TRUE(this->loop_->CancelTask(task_id));
MessageLoopRunMaxIterations(this->loop_.get(), 100);
EXPECT_FALSE(called);
@@ -99,12 +102,12 @@
TYPED_TEST(MessageLoopTest, PostDelayedTaskRunsEventuallyTest) {
bool called = false;
- TaskId task_id = this->loop_->PostDelayedTask(
- FROM_HERE, Bind(&SetToTrue, &called), TimeDelta::FromMilliseconds(50));
+ TaskId task_id =
+ this->loop_->PostDelayedTask(FROM_HERE, BindOnce(&SetToTrue, &called),
+ TimeDelta::FromMilliseconds(50));
EXPECT_NE(MessageLoop::kTaskIdNull, task_id);
- MessageLoopRunUntil(this->loop_.get(),
- TimeDelta::FromSeconds(10),
- Bind(&ReturnBool, &called));
+ MessageLoopRunUntil(this->loop_.get(), TimeDelta::FromSeconds(10),
+ BindRepeating(&ReturnBool, &called));
// Check that the main loop finished before the 10 seconds timeout, so it
// finished due to the callback being called and not due to the timeout.
EXPECT_TRUE(called);
@@ -124,9 +127,11 @@
TaskId task_id;
task_id = this->loop_->PostTask(
FROM_HERE,
- Bind([](bool* cancel_result, MessageLoop* loop, TaskId* task_id) {
- *cancel_result = loop->CancelTask(*task_id);
- }, &cancel_result, this->loop_.get(), &task_id));
+ BindOnce(
+ [](bool* cancel_result, MessageLoop* loop, TaskId* task_id) {
+ *cancel_result = loop->CancelTask(*task_id);
+ },
+ &cancel_result, this->loop_.get(), &task_id));
EXPECT_EQ(1, MessageLoopRunMaxIterations(this->loop_.get(), 100));
EXPECT_FALSE(cancel_result);
}
diff --git a/brillo/message_loops/message_loop_utils.cc b/brillo/message_loops/message_loop_utils.cc
index c16f268..0f3214b 100644
--- a/brillo/message_loops/message_loop_utils.cc
+++ b/brillo/message_loops/message_loop_utils.cc
@@ -9,15 +9,14 @@
namespace brillo {
-void MessageLoopRunUntil(
- MessageLoop* loop,
- base::TimeDelta timeout,
- base::Callback<bool()> terminate) {
+void MessageLoopRunUntil(MessageLoop* loop,
+ base::TimeDelta timeout,
+ base::RepeatingCallback<bool()> terminate) {
bool timeout_called = false;
MessageLoop::TaskId task_id = loop->PostDelayedTask(
FROM_HERE,
- base::Bind([](bool* timeout_called) { *timeout_called = true; },
- base::Unretained(&timeout_called)),
+ base::BindOnce([](bool* timeout_called) { *timeout_called = true; },
+ &timeout_called),
timeout);
while (!timeout_called && (terminate.is_null() || !terminate.Run()))
loop->RunOnce(true);
diff --git a/brillo/message_loops/message_loop_utils.h b/brillo/message_loops/message_loop_utils.h
index d49ebdf..7384ddb 100644
--- a/brillo/message_loops/message_loop_utils.h
+++ b/brillo/message_loops/message_loop_utils.h
@@ -18,7 +18,7 @@
BRILLO_EXPORT void MessageLoopRunUntil(
MessageLoop* loop,
base::TimeDelta timeout,
- base::Callback<bool()> terminate);
+ base::RepeatingCallback<bool()> terminate);
// Run the MessageLoop |loop| for up to |iterations| times without blocking.
// Return the number of tasks run.
diff --git a/brillo/message_loops/mock_message_loop.h b/brillo/message_loops/mock_message_loop.h
index 30a19b8..357ec24 100644
--- a/brillo/message_loops/mock_message_loop.h
+++ b/brillo/message_loops/mock_message_loop.h
@@ -37,7 +37,7 @@
&fake_loop_,
static_cast<TaskId(FakeMessageLoop::*)(
const base::Location&,
- const base::Closure&,
+ base::OnceClosure,
base::TimeDelta)>(
&FakeMessageLoop::PostDelayedTask)));
ON_CALL(*this, CancelTask(::testing::_))
@@ -51,7 +51,7 @@
MOCK_METHOD(TaskId,
PostDelayedTask,
- (const base::Location&, const base::Closure&, base::TimeDelta),
+ (const base::Location&, base::OnceClosure, base::TimeDelta),
(override));
using MessageLoop::PostDelayedTask;
MOCK_METHOD(bool, CancelTask, (TaskId), (override));
diff --git a/brillo/minijail/minijail.cc b/brillo/minijail/minijail.cc
index a08233d..9f88585 100644
--- a/brillo/minijail/minijail.cc
+++ b/brillo/minijail/minijail.cc
@@ -121,6 +121,23 @@
#endif // __ANDROID__
}
+bool Minijail::RunEnvPipes(struct minijail* jail,
+ vector<char*> args,
+ vector<char*> env,
+ pid_t* pid,
+ int* stdin,
+ int* stdout,
+ int* stderr) {
+#if defined(__ANDROID__)
+ return minijail_run_env_pid_pipes_no_preload(jail, args[0], args.data(),
+ env.data(), pid, stdin, stdout,
+ stderr) == 0;
+#else
+ return minijail_run_env_pid_pipes(jail, args[0], args.data(), env.data(), pid,
+ stdin, stdout, stderr) == 0;
+#endif // __ANDROID__
+}
+
bool Minijail::RunAndDestroy(struct minijail* jail,
vector<char*> args,
pid_t* pid) {
@@ -157,4 +174,16 @@
return res;
}
+bool Minijail::RunEnvPipesAndDestroy(struct minijail* jail,
+ vector<char*> args,
+ vector<char*> env,
+ pid_t* pid,
+ int* stdin,
+ int* stdout,
+ int* stderr) {
+ bool res = RunEnvPipes(jail, args, env, pid, stdin, stdout, stderr);
+ Destroy(jail);
+ return res;
+}
+
} // namespace brillo
diff --git a/brillo/minijail/minijail.h b/brillo/minijail/minijail.h
index c71211d..6cdc7ad 100644
--- a/brillo/minijail/minijail.h
+++ b/brillo/minijail/minijail.h
@@ -89,6 +89,14 @@
int* stdout,
int* stderr);
+ // minijail_run_env_pid_pipes
+ virtual bool RunEnvPipes(struct minijail* jail,
+ std::vector<char*> args,
+ std::vector<char*> env,
+ pid_t* pid,
+ int* stdin,
+ int* stdout,
+ int* stderr);
// Run() and Destroy()
virtual bool RunAndDestroy(struct minijail* jail,
std::vector<char*> args,
@@ -113,6 +121,15 @@
int* stdout,
int* stderr);
+ // RunEnvPipes() and Destroy()
+ virtual bool RunEnvPipesAndDestroy(struct minijail* jail,
+ std::vector<char*> args,
+ std::vector<char*> env,
+ pid_t* pid,
+ int* stdin,
+ int* stdout,
+ int* stderr);
+
protected:
Minijail();
diff --git a/brillo/minijail/mock_minijail.h b/brillo/minijail/mock_minijail.h
index 6c95405..21e7ad7 100644
--- a/brillo/minijail/mock_minijail.h
+++ b/brillo/minijail/mock_minijail.h
@@ -44,6 +44,20 @@
(struct minijail*, std::vector<char*>, int*),
(override));
MOCK_METHOD(bool,
+ RunPipes,
+ (struct minijail*, std::vector<char*>, pid_t*, int*, int*, int*),
+ (override));
+ MOCK_METHOD(bool,
+ RunEnvPipes,
+ (struct minijail*,
+ std::vector<char*>,
+ std::vector<char*>,
+ pid_t*,
+ int*,
+ int*,
+ int*),
+ (override));
+ MOCK_METHOD(bool,
RunAndDestroy,
(struct minijail*, std::vector<char*>, pid_t*),
(override));
@@ -59,6 +73,16 @@
RunPipesAndDestroy,
(struct minijail*, std::vector<char*>, pid_t*, int*, int*, int*),
(override));
+ MOCK_METHOD(bool,
+ RunEnvPipesAndDestroy,
+ (struct minijail*,
+ std::vector<char*>,
+ std::vector<char*>,
+ pid_t*,
+ int*,
+ int*,
+ int*),
+ (override));
private:
DISALLOW_COPY_AND_ASSIGN(MockMinijail);
diff --git a/brillo/namespaces/OWNERS b/brillo/namespaces/OWNERS
new file mode 100644
index 0000000..e96f211
--- /dev/null
+++ b/brillo/namespaces/OWNERS
@@ -0,0 +1,2 @@
+betuls@chromium.org
+jorgelo@chromium.org
diff --git a/brillo/namespaces/mock_platform.h b/brillo/namespaces/mock_platform.h
new file mode 100644
index 0000000..1b96b46
--- /dev/null
+++ b/brillo/namespaces/mock_platform.h
@@ -0,0 +1,37 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_BRILLO_NAMESPACES_MOCK_PLATFORM_H_
+#define LIBBRILLO_BRILLO_NAMESPACES_MOCK_PLATFORM_H_
+
+#include "brillo/namespaces/platform.h"
+
+#include <string>
+
+#include <base/files/file_path.h>
+#include <gmock/gmock.h>
+
+namespace brillo {
+
+class MockPlatform : public Platform {
+ public:
+ MockPlatform() {}
+ virtual ~MockPlatform() {}
+
+ MOCK_METHOD(bool, Unmount, (const base::FilePath&, bool, bool*), (override));
+ MOCK_METHOD(pid_t, Fork, (), (override));
+ MOCK_METHOD(pid_t, Waitpid, (pid_t, int*), (override));
+ MOCK_METHOD(int,
+ Mount,
+ (const std::string&,
+ const std::string&,
+ const std::string&,
+ uint64_t,
+ const void*),
+ (override));
+};
+
+} // namespace brillo
+
+#endif // LIBBRILLO_BRILLO_NAMESPACES_MOCK_PLATFORM_H_
diff --git a/brillo/namespaces/mount_namespace.cc b/brillo/namespaces/mount_namespace.cc
new file mode 100644
index 0000000..1944983
--- /dev/null
+++ b/brillo/namespaces/mount_namespace.cc
@@ -0,0 +1,114 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Contains the implementation of class MountNamespace for libbrillo.
+
+#include "brillo/namespaces/mount_namespace.h"
+
+#include <sched.h>
+#include <sys/mount.h>
+#include <sys/types.h>
+
+#include <string>
+
+#include <base/files/file_path.h>
+#include <base/files/file_util.h>
+#include <base/logging.h>
+#include <base/strings/stringprintf.h>
+#include <brillo/namespaces/platform.h>
+
+namespace brillo {
+MountNamespace::MountNamespace(const base::FilePath& ns_path,
+ Platform* platform)
+ : ns_path_(ns_path), platform_(platform), exists_(false) {}
+
+MountNamespace::~MountNamespace() {
+ if (exists_)
+ Destroy();
+}
+
+bool MountNamespace::Create() {
+ if (platform_->FileSystemIsNsfs(ns_path_)) {
+ LOG(ERROR) << "Mount namespace at " << ns_path_.value()
+ << " already exists.";
+ return false;
+ }
+ int fd_mounted[2];
+ int fd_unshared[2];
+ char byte = '\0';
+ if (pipe(fd_mounted) != 0) {
+ PLOG(ERROR) << "Cannot create mount signalling pipe";
+ return false;
+ }
+ if (pipe(fd_unshared) != 0) {
+ PLOG(ERROR) << "Cannot create unshare signalling pipe";
+ return false;
+ }
+ pid_t pid = platform_->Fork();
+ if (pid < 0) {
+ PLOG(ERROR) << "Fork failed";
+ } else if (pid == 0) {
+ // Child.
+ close(fd_mounted[1]);
+ close(fd_unshared[0]);
+ if (unshare(CLONE_NEWNS) != 0) {
+ PLOG(ERROR) << "unshare(CLONE_NEWNS) failed";
+ exit(1);
+ }
+ base::WriteFileDescriptor(fd_unshared[1], &byte, 1);
+ base::ReadFromFD(fd_mounted[0], &byte, 1);
+ exit(0);
+ } else {
+ // Parent.
+ close(fd_mounted[0]);
+ close(fd_unshared[1]);
+ std::string proc_ns_path = base::StringPrintf("/proc/%d/ns/mnt", pid);
+ bool mount_success = true;
+ base::ReadFromFD(fd_unshared[0], &byte, 1);
+ if (platform_->Mount(proc_ns_path, ns_path_.value(), "", MS_BIND) != 0) {
+ PLOG(ERROR) << "Mount(" << proc_ns_path << ", " << ns_path_.value()
+ << ", MS_BIND) failed";
+ mount_success = false;
+ }
+ base::WriteFileDescriptor(fd_mounted[1], &byte, 1);
+
+ int status;
+ if (platform_->Waitpid(pid, &status) < 0) {
+ PLOG(ERROR) << "waitpid(" << pid << ") failed";
+ return false;
+ }
+ if (!WIFEXITED(status)) {
+ LOG(ERROR) << "Child process did not exit normally.";
+ } else if (WEXITSTATUS(status) != 0) {
+ LOG(ERROR) << "Child process failed.";
+ } else {
+ exists_ = mount_success;
+ }
+ }
+ return exists_;
+}
+
+bool MountNamespace::Destroy() {
+ if (!exists_) {
+ LOG(ERROR) << "Mount namespace at " << ns_path_.value()
+ << "does not exist, cannot destroy";
+ return false;
+ }
+ bool was_busy;
+ if (!platform_->Unmount(ns_path_, false /*lazy*/, &was_busy)) {
+ PLOG(ERROR) << "Failed to unmount " << ns_path_.value();
+ if (was_busy) {
+ LOG(ERROR) << ns_path_.value().c_str() << " was busy";
+ }
+ // If Unmount() fails, keep the object valid by keeping |exists_|
+ // set to true.
+ return false;
+ } else {
+ VLOG(1) << "Unmounted namespace at " << ns_path_.value();
+ }
+ exists_ = false;
+ return true;
+}
+
+} // namespace brillo
diff --git a/brillo/namespaces/mount_namespace.h b/brillo/namespaces/mount_namespace.h
new file mode 100644
index 0000000..bfadff0
--- /dev/null
+++ b/brillo/namespaces/mount_namespace.h
@@ -0,0 +1,70 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_BRILLO_NAMESPACES_MOUNT_NAMESPACE_H_
+#define LIBBRILLO_BRILLO_NAMESPACES_MOUNT_NAMESPACE_H_
+
+#include "brillo/namespaces/platform.h"
+
+#include <base/files/file_path.h>
+#include <base/macros.h>
+#include <brillo/brillo_export.h>
+
+namespace brillo {
+
+class BRILLO_EXPORT MountNamespaceInterface {
+ // An interface declaring the basic functionality of a mount namespace bound
+ // to a specific path. This basic functionality consists of reporting the
+ // namespace path.
+ public:
+ virtual ~MountNamespaceInterface() = default;
+
+ virtual const base::FilePath& path() const = 0;
+};
+
+class BRILLO_EXPORT UnownedMountNamespace : public MountNamespaceInterface {
+ // A class to store and retrieve the path of a persistent namespace. This
+ // class doesn't create nor destroy the namespace.
+ public:
+ explicit UnownedMountNamespace(const base::FilePath& ns_path)
+ : ns_path_(ns_path) {}
+
+ ~UnownedMountNamespace() override;
+
+ const base::FilePath& path() const override { return ns_path_; }
+
+ private:
+ base::FilePath ns_path_;
+
+ DISALLOW_COPY_AND_ASSIGN(UnownedMountNamespace);
+};
+
+class BRILLO_EXPORT MountNamespace : public MountNamespaceInterface {
+ // A class to create a persistent mount namespace bound to a specific path.
+ // A new mount namespace is unshared from the mount namespace of the calling
+ // process when Create() is called; the namespace of the calling process
+ // remains unchanged. Recurring creation on a path is not allowed.
+ //
+ // Given that we cannot ensure that creation always succeeds this class is not
+ // fully RAII, but once the namespace is created (with Create()), it will be
+ // destroyed when the object goes out of scope.
+ public:
+ MountNamespace(const base::FilePath& ns_path, Platform* platform);
+ ~MountNamespace() override;
+
+ bool Create();
+ bool Destroy();
+ const base::FilePath& path() const override { return ns_path_; }
+
+ private:
+ base::FilePath ns_path_;
+ Platform* platform_;
+ bool exists_;
+
+ DISALLOW_COPY_AND_ASSIGN(MountNamespace);
+};
+
+} // namespace brillo
+
+#endif // LIBBRILLO_BRILLO_NAMESPACES_MOUNT_NAMESPACE_H_
diff --git a/brillo/namespaces/mount_namespace_test.cc b/brillo/namespaces/mount_namespace_test.cc
new file mode 100644
index 0000000..1bfa038
--- /dev/null
+++ b/brillo/namespaces/mount_namespace_test.cc
@@ -0,0 +1,92 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "brillo/namespaces/mock_platform.h"
+#include "brillo/namespaces/mount_namespace.h"
+#include "brillo/namespaces/platform.h"
+
+#include <unistd.h>
+
+#include <memory>
+
+#include <base/files/file_path.h>
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+using ::testing::_;
+using ::testing::DoAll;
+using ::testing::NiceMock;
+using ::testing::Return;
+using ::testing::SetArgPointee;
+
+namespace brillo {
+
+class MountNamespaceTest : public ::testing::Test {
+ public:
+ MountNamespaceTest() {}
+ ~MountNamespaceTest() {}
+ void SetUp() {}
+
+ void TearDown() {}
+
+ protected:
+ NiceMock<MockPlatform> platform_;
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MountNamespaceTest);
+};
+
+TEST_F(MountNamespaceTest, CreateNamespace) {
+ std::unique_ptr<MountNamespace> ns =
+ std::make_unique<MountNamespace>(base::FilePath(), &platform_);
+ EXPECT_CALL(platform_, Fork()).WillOnce(Return(1));
+ EXPECT_CALL(platform_, Mount(_, _, _, _, _)).WillOnce(Return(0));
+ EXPECT_CALL(platform_, Waitpid(_, _))
+ .WillOnce(DoAll(SetArgPointee<1>(0x00000000), Return(0)));
+ EXPECT_TRUE(ns->Create());
+ EXPECT_CALL(platform_, Unmount(ns->path(), _, _)).WillOnce(Return(true));
+}
+
+TEST_F(MountNamespaceTest, CreateNamespaceFailedOnWaitpid) {
+ std::unique_ptr<MountNamespace> ns =
+ std::make_unique<MountNamespace>(base::FilePath(), &platform_);
+ EXPECT_CALL(platform_, Fork()).WillOnce(Return(1));
+ EXPECT_CALL(platform_, Mount(_, _, _, _, _)).WillOnce(Return(0));
+ EXPECT_CALL(platform_, Waitpid(_, _)).WillOnce(Return(-1));
+ EXPECT_FALSE(ns->Create());
+}
+
+TEST_F(MountNamespaceTest, CreateNamespaceFailedOnMount) {
+ std::unique_ptr<MountNamespace> ns =
+ std::make_unique<MountNamespace>(base::FilePath(), &platform_);
+ EXPECT_CALL(platform_, Fork()).WillOnce(Return(1));
+ EXPECT_CALL(platform_, Mount(_, _, _, _, _)).WillOnce(Return(-1));
+ EXPECT_FALSE(ns->Create());
+}
+
+TEST_F(MountNamespaceTest, CreateNamespaceFailedOnStatus) {
+ std::unique_ptr<MountNamespace> ns =
+ std::make_unique<MountNamespace>(base::FilePath(), &platform_);
+ EXPECT_CALL(platform_, Fork()).WillOnce(Return(1));
+ EXPECT_CALL(platform_, Mount(_, _, _, _, _)).WillOnce(Return(0));
+ EXPECT_CALL(platform_, Waitpid(_, _))
+ .WillOnce(DoAll(SetArgPointee<1>(0xFFFFFFFF), Return(0)));
+ EXPECT_FALSE(ns->Create());
+}
+
+TEST_F(MountNamespaceTest, DestroyAfterUnmountFailsAndUnmountSucceeds) {
+ std::unique_ptr<MountNamespace> ns =
+ std::make_unique<MountNamespace>(base::FilePath(), &platform_);
+ EXPECT_CALL(platform_, Fork()).WillOnce(Return(1));
+ EXPECT_CALL(platform_, Mount(_, _, _, _, _)).WillOnce(Return(0));
+ EXPECT_CALL(platform_, Waitpid(_, _))
+ .WillOnce(DoAll(SetArgPointee<1>(0x00000000), Return(0)));
+ EXPECT_TRUE(ns->Create());
+ EXPECT_CALL(platform_, Unmount(ns->path(), _, _)).WillOnce(Return(false));
+ EXPECT_FALSE(ns->Destroy());
+ EXPECT_CALL(platform_, Unmount(ns->path(), _, _)).WillOnce(Return(true));
+ EXPECT_TRUE(ns->Destroy());
+}
+
+} // namespace brillo
diff --git a/brillo/namespaces/platform.cc b/brillo/namespaces/platform.cc
new file mode 100644
index 0000000..5fe9140
--- /dev/null
+++ b/brillo/namespaces/platform.cc
@@ -0,0 +1,75 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Contains the implementation of class Platform for libbrillo.
+
+#include "brillo/namespaces/platform.h"
+
+#include <errno.h>
+#include <linux/magic.h>
+#include <stdio.h>
+#include <sys/mount.h>
+#include <sys/stat.h>
+#include <sys/vfs.h>
+#include <sys/wait.h>
+
+#include <memory>
+
+#include <base/files/file_path.h>
+
+using base::FilePath;
+
+namespace brillo {
+
+Platform::Platform() {}
+
+Platform::~Platform() {}
+
+bool Platform::FileSystemIsNsfs(const FilePath& ns_path) {
+ struct statfs buff;
+ if (statfs(ns_path.value().c_str(), &buff) < 0) {
+ PLOG(ERROR) << "Statfs() error for " << ns_path.value();
+ return false;
+ }
+ if ((uint64_t)buff.f_type == NSFS_MAGIC) {
+ return true;
+ }
+ return false;
+}
+
+bool Platform::Unmount(const FilePath& path, bool lazy, bool* was_busy) {
+ int flags = 0;
+ if (lazy) {
+ flags = MNT_DETACH;
+ }
+ if (umount2(path.value().c_str(), flags) != 0) {
+ if (was_busy) {
+ *was_busy = (errno == EBUSY);
+ }
+ return false;
+ }
+ if (was_busy) {
+ *was_busy = false;
+ }
+ return true;
+}
+
+int Platform::Mount(const std::string& source,
+ const std::string& target,
+ const std::string& fs_type,
+ uint64_t mount_flags,
+ const void* data) {
+ return mount(source.c_str(), target.c_str(), fs_type.c_str(), mount_flags,
+ data);
+}
+
+pid_t Platform::Fork() {
+ return fork();
+}
+
+pid_t Platform::Waitpid(pid_t pid, int* status) {
+ return waitpid(pid, status, 0);
+}
+
+} // namespace brillo
diff --git a/brillo/namespaces/platform.h b/brillo/namespaces/platform.h
new file mode 100644
index 0000000..6ef6a73
--- /dev/null
+++ b/brillo/namespaces/platform.h
@@ -0,0 +1,71 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_BRILLO_NAMESPACES_PLATFORM_H_
+#define LIBBRILLO_BRILLO_NAMESPACES_PLATFORM_H_
+
+#include <sys/types.h>
+
+#include <memory>
+#include <string>
+
+#include <base/files/file_path.h>
+#include <base/macros.h>
+#include <brillo/brillo_export.h>
+
+namespace brillo {
+// Platform specific routines abstraction layer.
+// Also helps us to be able to mock them in tests.
+class BRILLO_EXPORT Platform {
+ public:
+ Platform();
+
+ virtual ~Platform();
+ // Calls the platform fork() function and returns the pid returned
+ // by fork().
+ virtual pid_t Fork();
+
+ // Calls the platform unmount.
+ //
+ // Parameters
+ // path - The path to unmount
+ // lazy - Whether to call a lazy unmount
+ // was_busy (OUT) - Set to true on return if the mount point was busy
+ virtual bool Unmount(const base::FilePath& path, bool lazy, bool* was_busy);
+
+ // Calls the platform mount.
+ //
+ // Parameters
+ // source - The path to mount from
+ // target - The path to mount to
+ // fs_type - File system type of the mount
+ // mount_flags - Flags spesifying the type of the mount operation
+ // data - Mount options
+ virtual int Mount(const std::string& source,
+ const std::string& target,
+ const std::string& fs_type,
+ uint64_t mount_flags,
+ const void* = nullptr);
+
+ // Checks the file system type of the |path| and returns true if the
+ // filesystem type is nsfs.
+ //
+ // Parameters
+ // path - The path to check the file system type
+ virtual bool FileSystemIsNsfs(const base::FilePath& path);
+
+ // Calls the platform waitpid() function and returns the value returned by
+ // waitpid().
+ //
+ // Parameters
+ // pid - The child pid to be waited on
+ // status (OUT)- Termination status of a child process.
+ virtual pid_t Waitpid(pid_t pid, int* status);
+
+ DISALLOW_COPY_AND_ASSIGN(Platform);
+};
+
+} // namespace brillo
+
+#endif // LIBBRILLO_BRILLO_NAMESPACES_PLATFORM_H_
diff --git a/brillo/scoped_mount_namespace.cc b/brillo/scoped_mount_namespace.cc
index 0f35e82..09f3c75 100644
--- a/brillo/scoped_mount_namespace.cc
+++ b/brillo/scoped_mount_namespace.cc
@@ -38,7 +38,7 @@
// static
std::unique_ptr<ScopedMountNamespace> ScopedMountNamespace::CreateFromPath(
- base::FilePath ns_path) {
+ const base::FilePath& ns_path) {
base::ScopedFD original_mount_namespace_fd(
HANDLE_EINTR(open(kCurrentMountNamespacePath, O_RDONLY)));
if (!original_mount_namespace_fd.is_valid()) {
diff --git a/brillo/scoped_mount_namespace.h b/brillo/scoped_mount_namespace.h
index f360221..e029d78 100644
--- a/brillo/scoped_mount_namespace.h
+++ b/brillo/scoped_mount_namespace.h
@@ -28,7 +28,7 @@
// Enters the mount namespace identified by |path| and returns a unique_ptr
// that restores the original mount namespace when it goes out of scope.
static std::unique_ptr<ScopedMountNamespace> CreateFromPath(
- base::FilePath ns_path);
+ const base::FilePath& ns_path);
explicit ScopedMountNamespace(base::ScopedFD mount_namespace_fd);
~ScopedMountNamespace();
diff --git a/brillo/secure_allocator.h b/brillo/secure_allocator.h
index 0cbb8d9..de0b348 100644
--- a/brillo/secure_allocator.h
+++ b/brillo/secure_allocator.h
@@ -5,13 +5,51 @@
#ifndef LIBBRILLO_BRILLO_SECURE_ALLOCATOR_H_
#define LIBBRILLO_BRILLO_SECURE_ALLOCATOR_H_
+#include <errno.h>
+#include <sys/mman.h>
+#include <unistd.h>
+
+#include <limits>
#include <memory>
+#include <base/callback_helpers.h>
+#include <base/logging.h>
#include <brillo/brillo_export.h>
namespace brillo {
// SecureAllocator is a stateless derivation of std::allocator that clears
-// the contents of the object on deallocation.
+// the contents of the object on deallocation. Additionally, to prevent the
+// memory from being leaked, we use the following defensive mechanisms:
+//
+// 1. Use page-aligned memory so that it can be locked (therefore, use mmap()
+// instead of malloc()). Note that mlock()s are not inherited over fork(),
+//
+// 2. Always allocate memory in multiples of pages: this adds a memory overhead
+// of ~1 page for each object. Moreover, the extra memory is not available
+// for the allocated object to expand into: the container expects that the
+// memory allocated to it matches the size set in reserve().
+// TODO(sarthakkukreti): Figure out if it is possible to propagate the real
+// capacity to the container without an intrusive change to the STL.
+// [Example: allow __recommend() override in allocators for containers.]
+//
+// 3. Mark the memory segments as undumpable, unmergeable.
+//
+// 4. Use MADV_WIPEONFORK:
+// this results in a new anonymous vma instead of copying over the contents
+// of the secure object after a fork(). By default [MADV_DOFORK], the vma is
+// marked as copy-on-write, and the first process which writes to the secure
+// object after fork get a new copy. This may break the security guarantees
+// setup above. Another alternative is to use MADV_DONTFORK which results in
+// the memory mapping not getting copied over to child process at all: this
+// may result in cases where if the child process gets segmentation faults
+// on attempts to access virtual addresses in the secure object's address
+// range,
+//
+// With MADV_WIPEONFORK, the child processes can access the secure object
+// memory safely, but the contents of the secure object appear as zero to
+// the child process. Note that threads share the virtual address space and
+// secure objects would be transparent across threads.
+// TODO(sarthakkukreti): Figure out patterns to pass secure data over fork().
template <typename T>
class BRILLO_PRIVATE SecureAllocator : public std::allocator<T> {
public:
@@ -19,21 +57,118 @@
using typename std::allocator<T>::size_type;
using typename std::allocator<T>::value_type;
- // Implicit std::allocator constructors.
+ // Constructors that wrap over std::allocator.
+ // Make sure that the allocator's static members are only allocated once.
+ SecureAllocator() noexcept : std::allocator<T>() {}
+ SecureAllocator(const SecureAllocator& other) noexcept
+ : std::allocator<T>(other) {}
+
+ template <class U>
+ SecureAllocator(const SecureAllocator<U>& other) noexcept
+ : std::allocator<T>(other) {}
template <typename U> struct rebind {
typedef SecureAllocator<U> other;
};
- // Allocation/deallocation: use the std::allocation functions but make sure
- // that on deallocation, the contents of the element are cleared out.
- pointer allocate(size_type n, pointer = {}) {
- return std::allocator<T>::allocate(n);
+ // Return cached max_size. Deprecated in C++17, removed in C++20.
+ size_type max_size() const { return max_size_; }
+
+ // Allocation: allocate ceil(size/pagesize) for holding the data.
+ pointer allocate(size_type n, pointer hint = nullptr) {
+ pointer buffer = nullptr;
+ // Check if n can be theoretically allocated.
+ CHECK_LT(n, max_size());
+
+ // std::allocator is expected to throw a std::bad_alloc on failing to
+ // allocate the memory correctly. Instead of returning a nullptr, which
+ // confuses the standard template library, use CHECK(false) to crash on
+ // the failure path.
+ base::ScopedClosureRunner fail_on_allocation_error(base::Bind([]() {
+ PLOG(ERROR) << "Failed to allocate secure memory";
+ CHECK(false);
+ }));
+
+ // Check if n = 0: there's nothing to allocate;
+ if (n == 0)
+ return nullptr;
+
+ // Calculate the page-aligned buffer size.
+ size_type buffer_size = CalculatePageAlignedBufferSize(n);
+
+ // Memory locking granularity is per-page: mmap ceil(size/page size) pages.
+ buffer = reinterpret_cast<pointer>(
+ mmap(nullptr, buffer_size, PROT_READ | PROT_WRITE,
+ MAP_PRIVATE | MAP_ANONYMOUS, -1, 0));
+ if (buffer == MAP_FAILED)
+ return nullptr;
+
+ // Lock buffer into physical memory.
+ if (mlock(buffer, buffer_size)) {
+ CHECK_NE(errno, ENOMEM) << "It is likely that SecureAllocator have "
+ "exceeded the RLIMIT_MEMLOCK limit";
+ return nullptr;
+ }
+
+ // Mark memory as non dumpable in a core dump.
+ if (madvise(buffer, buffer_size, MADV_DONTDUMP))
+ return nullptr;
+
+ // Mark memory as non mergeable with another page, even if the contents
+ // are the same.
+ if (madvise(buffer, buffer_size, MADV_UNMERGEABLE)) {
+ // MADV_UNMERGEABLE is only available if the kernel has been configured
+ // with CONFIG_KSM set. If the CONFIG_KSM flag has not been set, then
+ // pages are not mergeable so this madvise option is not necessary.
+ //
+ // In the case where CONFIG_KSM is not set, EINVAL is the error set.
+ // Since this error value is expected in some cases, we don't return a
+ // nullptr.
+ if (errno != EINVAL)
+ return nullptr;
+ }
+
+ // Make this mapping available to child processes but don't copy data from
+ // the secure object's pages during fork. With MADV_DONTFORK, the
+ // vma is not mapped in the child process which leads to segmentation
+ // faults if the child process tries to access this address. For example,
+ // if the parent process creates a SecureObject, forks() and the child
+ // process tries to call the destructor at the virtual address.
+ if (madvise(buffer, buffer_size, MADV_WIPEONFORK))
+ return nullptr;
+
+ ignore_result(fail_on_allocation_error.Release());
+
+ // Allocation was successful.
+ return buffer;
+ }
+
+ // Destroy object before deallocation. Deprecated in C++17, removed in C++20.
+ // After destroying the object, clear the contents of where the object was
+ // stored.
+ template <class U>
+ void destroy(U* p) {
+ // Return if the pointer is invalid.
+ if (!p)
+ return;
+ std::allocator<U>::destroy(p);
+ clear_contents(p, sizeof(U));
}
virtual void deallocate(pointer p, size_type n) {
- clear_contents(p, n * sizeof(value_type));
- std::allocator<T>::deallocate(p, n);
+ // Check if n can be theoretically deallocated.
+ CHECK_LT(n, max_size());
+
+ // Check if n = 0 or p is a nullptr: there's nothing to deallocate;
+ if (n == 0 || !p)
+ return;
+
+ // Calculate the page-aligned buffer size.
+ size_type buffer_size = CalculatePageAlignedBufferSize(n);
+
+ clear_contents(p, buffer_size);
+ munlock(p, buffer_size);
+ munmap(p, buffer_size);
}
protected:
@@ -54,8 +189,53 @@
}
#undef __attribute_no_opt
+
+ private:
+ // Calculate the page-aligned buffer size.
+ size_t CalculatePageAlignedBufferSize(size_type n) {
+ size_type real_size = n * sizeof(value_type);
+ size_type page_aligned_remainder = real_size % page_size_;
+ size_type padding =
+ page_aligned_remainder != 0 ? page_size_ - page_aligned_remainder : 0;
+ return real_size + padding;
+ }
+
+ static size_t CalculatePageSize() {
+ long ret = sysconf(_SC_PAGESIZE); // NOLINT [runtime/int]
+
+ // Initialize page size.
+ CHECK_GT(ret, 0L);
+ return ret;
+ }
+
+ // Since the allocator reuses page size and max size consistently,
+ // cache these values initially and reuse.
+ static size_t GetMaxSizeForType(size_t page_size) {
+ // Initialize max size that can be theoretically allocated.
+ // Calculate the max size that is page-aligned.
+ size_t max_theoretical_size = std::numeric_limits<size_type>::max();
+ size_t max_page_aligned_size =
+ max_theoretical_size - (max_theoretical_size % page_size);
+
+ return max_page_aligned_size / sizeof(value_type);
+ }
+
+ // Page size on system.
+ static const size_type page_size_;
+ // Max theoretical count for type on system.
+ static const size_type max_size_;
};
+// Inline definitions are only allowed for static const members with integral
+// constexpr initializers, define static members of SecureAllocator types here.
+template <typename T>
+const typename SecureAllocator<T>::size_type SecureAllocator<T>::page_size_ =
+ SecureAllocator<T>::CalculatePageSize();
+
+template <typename T>
+const typename SecureAllocator<T>::size_type SecureAllocator<T>::max_size_ =
+ SecureAllocator<T>::GetMaxSizeForType(SecureAllocator<T>::page_size_);
+
} // namespace brillo
#endif // LIBBRILLO_BRILLO_SECURE_ALLOCATOR_H_
diff --git a/brillo/secure_blob_test.cc b/brillo/secure_blob_test.cc
index 75f3cfb..7242e86 100644
--- a/brillo/secure_blob_test.cc
+++ b/brillo/secure_blob_test.cc
@@ -234,16 +234,18 @@
public:
using typename SecureAllocator<T>::pointer;
using typename SecureAllocator<T>::size_type;
+ using typename SecureAllocator<T>::value_type;
int GetErasedCount() { return erased_count; }
protected:
void clear_contents(pointer p, size_type n) override {
SecureAllocator<T>::clear_contents(p, n);
+ unsigned char *v = reinterpret_cast<unsigned char*>(p);
for (int i = 0; i < n; i++) {
- EXPECT_EQ(p[i], 0);
+ EXPECT_EQ(v[i], 0);
+ erased_count++;
}
- erased_count++;
}
private:
@@ -259,7 +261,54 @@
// Deallocate memory; the mock class should check for cleared data.
e.deallocate(test_string_addr, 15);
- EXPECT_EQ(e.GetErasedCount(), 1);
+ // The deallocation should have traversed the complete page.
+ EXPECT_EQ(e.GetErasedCount(), 4096);
}
+TEST(SecureAllocator, MultiPageCorrectness) {
+ // Make sure that the contents are cleared on deallocation.
+ TestSecureAllocator<uint64_t> e;
+
+ // Allocate 4100*8 bytes.
+ uint64_t *test_array = e.allocate(4100);
+
+ // Check if the space was correctly allocated for long long.
+ for (int i = 0; i < 4100; i++)
+ test_array[i] = 0xF0F0F0F0F0F0F0F0;
+
+ // Deallocate memory; the mock class should check for cleared data.
+ e.deallocate(test_array, 4100);
+ // 36864 bytes is the next highest size that is a multiple of the page size.
+ EXPECT_EQ(e.GetErasedCount(), 36864);
+}
+
+// DeathTests fork a new process and check how it proceeds. Take advantage
+// of this and check if the value of SecureString is passed on to
+// forked children.
+#if GTEST_IS_THREADSAFE
+// Check if the contents of the container are zeroed out.
+void CheckPropagationOnFork(const brillo::SecureBlob& forked_blob,
+ const Blob& reference) {
+ LOG(INFO) << forked_blob.to_string();
+ for (int i = 0; i < forked_blob.size(); i++) {
+ CHECK_NE(reference[i], forked_blob[i]);
+ CHECK_EQ(forked_blob[i], 0);
+ }
+ exit(0);
+}
+
+TEST(SecureAllocatorDeathTest, ErasureOnFork) {
+ Blob reference = BlobFromString("Test String");
+ SecureBlob erasable_blob(reference.begin(), reference.end());
+
+ EXPECT_EXIT(CheckPropagationOnFork(erasable_blob, reference),
+ ::testing::ExitedWithCode(0), "");
+
+ // In the original process, check the SecureBlob to see if it has not
+ // changed.
+ for (int i = 0; i < erasable_blob.size(); i++)
+ EXPECT_EQ(erasable_blob[i], reference[i]);
+}
+#endif // GTEST_IS_THREADSAFE
+
} // namespace brillo
diff --git a/brillo/streams/input_stream_set.cc b/brillo/streams/input_stream_set.cc
index 0ceb5b4..847bf05 100644
--- a/brillo/streams/input_stream_set.cc
+++ b/brillo/streams/input_stream_set.cc
@@ -172,7 +172,7 @@
return stream->WaitForData(mode, callback, error);
}
- MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, mode));
+ MessageLoop::current()->PostTask(FROM_HERE, base::BindOnce(callback, mode));
return true;
}
diff --git a/brillo/streams/memory_stream.cc b/brillo/streams/memory_stream.cc
index 54f127a..f4f9cca 100644
--- a/brillo/streams/memory_stream.cc
+++ b/brillo/streams/memory_stream.cc
@@ -185,7 +185,7 @@
bool MemoryStream::WaitForData(AccessMode mode,
const base::Callback<void(AccessMode)>& callback,
ErrorPtr* /* error */) {
- MessageLoop::current()->PostTask(FROM_HERE, base::Bind(callback, mode));
+ MessageLoop::current()->PostTask(FROM_HERE, base::BindOnce(callback, mode));
return true;
}
diff --git a/brillo/streams/stream.cc b/brillo/streams/stream.cc
index 6a40c00..80f2df4 100644
--- a/brillo/streams/stream.cc
+++ b/brillo/streams/stream.cc
@@ -213,8 +213,9 @@
if (force_async_callback) {
MessageLoop::current()->PostTask(
FROM_HERE,
- base::Bind(&Stream::OnReadAsyncDone, weak_ptr_factory_.GetWeakPtr(),
- success_callback, read, eos));
+ base::BindOnce(&Stream::OnReadAsyncDone,
+ weak_ptr_factory_.GetWeakPtr(),
+ success_callback, read, eos));
} else {
is_async_read_pending_ = false;
success_callback.Run(read, eos);
@@ -277,8 +278,9 @@
if (force_async_callback) {
MessageLoop::current()->PostTask(
FROM_HERE,
- base::Bind(&Stream::OnWriteAsyncDone, weak_ptr_factory_.GetWeakPtr(),
- success_callback, written));
+ base::BindOnce(&Stream::OnWriteAsyncDone,
+ weak_ptr_factory_.GetWeakPtr(),
+ success_callback, written));
} else {
is_async_write_pending_ = false;
success_callback.Run(written);
diff --git a/brillo/streams/stream_utils.cc b/brillo/streams/stream_utils.cc
index 6f8a1d0..5029e3a 100644
--- a/brillo/streams/stream_utils.cc
+++ b/brillo/streams/stream_utils.cc
@@ -213,7 +213,7 @@
state->success_callback = success_callback;
state->error_callback = error_callback;
brillo::MessageLoop::current()->PostTask(FROM_HERE,
- base::Bind(&PerformRead, state));
+ base::BindOnce(&PerformRead, state));
}
} // namespace stream_utils
diff --git a/brillo/streams/stream_utils_test.cc b/brillo/streams/stream_utils_test.cc
index e0b327d..50fb67e 100644
--- a/brillo/streams/stream_utils_test.cc
+++ b/brillo/streams/stream_utils_test.cc
@@ -25,7 +25,7 @@
HAS_1_TEMPLATE_PARAMS(int, k),
AND_1_VALUE_PARAMS(size)) {
brillo::MessageLoop::current()->PostTask(
- FROM_HERE, base::Bind(std::get<k>(args), size));
+ FROM_HERE, base::BindOnce(std::get<k>(args), size));
return true;
}
@@ -42,7 +42,8 @@
brillo::ErrorPtr error;
brillo::Error::AddTo(&error, FROM_HERE, "test", code, "message");
brillo::MessageLoop::current()->PostTask(
- FROM_HERE, base::Bind(std::get<k>(args), base::Owned(error.release())));
+ FROM_HERE, base::BindOnce(std::get<k>(args),
+ base::Owned(error.release())));
return true;
}
diff --git a/brillo/streams/tls_stream.cc b/brillo/streams/tls_stream.cc
index cc63258..4b8a227 100644
--- a/brillo/streams/tls_stream.cc
+++ b/brillo/streams/tls_stream.cc
@@ -393,10 +393,10 @@
if (MessageLoop::ThreadHasCurrent()) {
MessageLoop::current()->PostTask(
FROM_HERE,
- base::Bind(&TlsStreamImpl::DoHandshake,
- weak_ptr_factory_.GetWeakPtr(),
- success_callback,
- error_callback));
+ base::BindOnce(&TlsStreamImpl::DoHandshake,
+ weak_ptr_factory_.GetWeakPtr(),
+ success_callback,
+ error_callback));
} else {
DoHandshake(success_callback, error_callback);
}
diff --git a/brillo/timezone/EST_test.tzif b/brillo/timezone/EST_test.tzif
new file mode 100644
index 0000000..ae34663
--- /dev/null
+++ b/brillo/timezone/EST_test.tzif
Binary files differ
diff --git a/brillo/timezone/Indian_Christmas_test.tzif b/brillo/timezone/Indian_Christmas_test.tzif
new file mode 100644
index 0000000..066c1e9
--- /dev/null
+++ b/brillo/timezone/Indian_Christmas_test.tzif
Binary files differ
diff --git a/brillo/timezone/Pacific_Fiji_test.tzif b/brillo/timezone/Pacific_Fiji_test.tzif
new file mode 100644
index 0000000..76ae63e
--- /dev/null
+++ b/brillo/timezone/Pacific_Fiji_test.tzif
Binary files differ
diff --git a/brillo/timezone/tzif_parser.cc b/brillo/timezone/tzif_parser.cc
new file mode 100644
index 0000000..5756196
--- /dev/null
+++ b/brillo/timezone/tzif_parser.cc
@@ -0,0 +1,164 @@
+// Copyright 2018 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "brillo/timezone/tzif_parser.h"
+
+#include <arpa/inet.h>
+#include <stdint.h>
+#include <string.h>
+#include <utility>
+#include <vector>
+
+#include <base/files/file.h>
+#include <base/files/file_path.h>
+#include <base/logging.h>
+#include <base/stl_util.h>
+#include <base/strings/string_util.h>
+
+namespace {
+
+struct tzif_header {
+ char magic[4];
+ char version;
+ char reserved[15];
+ int32_t ttisgmtcnt;
+ int32_t ttisstdcnt;
+ int32_t leapcnt;
+ int32_t timecnt;
+ int32_t typecnt;
+ int32_t charcnt;
+};
+
+bool ReadInt(base::File* file, int32_t* out_int) {
+ DCHECK(out_int);
+ int32_t buf;
+ int read = file->ReadAtCurrentPos(reinterpret_cast<char*>(&buf), sizeof(buf));
+ if (read != sizeof(buf)) {
+ return false;
+ }
+ // Values are stored in network byte order (highest-order byte first).
+ // We probably need to convert them to match the endianness of our system.
+ *out_int = ntohl(buf);
+ return true;
+}
+
+bool ParseTzifHeader(base::File* tzfile, struct tzif_header* header) {
+ DCHECK(header);
+ int read = tzfile->ReadAtCurrentPos(header->magic, sizeof(header->magic));
+ if (read != sizeof(header->magic)) {
+ return false;
+ }
+ if (memcmp(header->magic, "TZif", 4) != 0) {
+ return false;
+ }
+
+ read = tzfile->ReadAtCurrentPos(&header->version, sizeof(header->version));
+ if (read != sizeof(header->version)) {
+ return false;
+ }
+ if (header->version != '\0' && header->version != '2' &&
+ header->version != '3') {
+ return false;
+ }
+
+ read = tzfile->ReadAtCurrentPos(header->reserved, sizeof(header->reserved));
+ if (read != sizeof(header->reserved)) {
+ return false;
+ }
+ for (size_t i = 0; i < sizeof(header->reserved); i++) {
+ if (header->reserved[i] != 0) {
+ return false;
+ }
+ }
+
+ if (!ReadInt(tzfile, &header->ttisgmtcnt) || header->ttisgmtcnt < 0) {
+ return false;
+ }
+ if (!ReadInt(tzfile, &header->ttisstdcnt) || header->ttisstdcnt < 0) {
+ return false;
+ }
+ if (!ReadInt(tzfile, &header->leapcnt) || header->leapcnt < 0) {
+ return false;
+ }
+ if (!ReadInt(tzfile, &header->timecnt) || header->timecnt < 0) {
+ return false;
+ }
+ if (!ReadInt(tzfile, &header->typecnt) || header->typecnt < 0) {
+ return false;
+ }
+ if (!ReadInt(tzfile, &header->charcnt) || header->charcnt < 0) {
+ return false;
+ }
+ return true;
+}
+
+} // namespace
+
+namespace brillo {
+
+namespace timezone {
+
+base::Optional<std::string> GetPosixTimezone(const base::FilePath& tzif_path) {
+ base::FilePath to_parse;
+ if (tzif_path.IsAbsolute()) {
+ to_parse = tzif_path;
+ } else {
+ to_parse = base::FilePath("/usr/share/zoneinfo").Append(tzif_path);
+ }
+ base::File tzfile(to_parse, base::File::FLAG_OPEN | base::File::FLAG_READ);
+ struct tzif_header first_header;
+ if (!tzfile.IsValid() || !ParseTzifHeader(&tzfile, &first_header)) {
+ return base::nullopt;
+ }
+
+ if (first_header.version == '\0') {
+ // Generating a POSIX-style TZ string from a TZif version 1 file is hard;
+ // TZ strings need relative dates (1st Sunday in March, 1st Sunday in Nov,
+ // etc.), but the version 1 files only give absolute dates for each
+ // year up to 2037. Fortunately version 1 files are no longer created by
+ // the newer versions of the timezone-data package.
+ //
+ // Because of this, we're not going to try and handle this, and instead just
+ // return an error.
+ return base::nullopt;
+ }
+
+ // TZif versions 2 and 3 embed a POSIX-style TZ string after their
+ // contents. We read that out and return it.
+
+ // Skip past the first body section and all of the second section. See
+ // 'man tzfile' for an explanation of these offset values.
+ int64_t first_body_size =
+ 4 * first_header.timecnt + 1 * first_header.timecnt +
+ (4 + 1 + 1) * first_header.typecnt + 1 * first_header.charcnt +
+ (4 + 4) * first_header.leapcnt + 1 * first_header.ttisstdcnt +
+ 1 * first_header.ttisgmtcnt;
+ tzfile.Seek(base::File::FROM_CURRENT, first_body_size);
+
+ struct tzif_header second_header;
+ if (!ParseTzifHeader(&tzfile, &second_header)) {
+ return base::nullopt;
+ }
+
+ int64_t second_body_size =
+ 8 * second_header.timecnt + 1 * second_header.timecnt +
+ (4 + 1 + 1) * second_header.typecnt + 1 * second_header.charcnt +
+ (8 + 4) * second_header.leapcnt + 1 * second_header.ttisstdcnt +
+ 1 * second_header.ttisgmtcnt;
+ int64_t offset = tzfile.Seek(base::File::FROM_CURRENT, second_body_size);
+
+ std::string time_string(tzfile.GetLength() - offset, '\0');
+ if (tzfile.ReadAtCurrentPos(base::data(time_string), time_string.size()) !=
+ time_string.size()) {
+ return base::nullopt;
+ }
+
+ // According to the spec, the embedded string is enclosed by '\n' characters.
+ base::TrimWhitespaceASCII(time_string, base::TRIM_ALL, &time_string);
+ return std::move(time_string);
+}
+
+} // namespace timezone
+
+} // namespace brillo
diff --git a/brillo/timezone/tzif_parser.h b/brillo/timezone/tzif_parser.h
new file mode 100644
index 0000000..058079c
--- /dev/null
+++ b/brillo/timezone/tzif_parser.h
@@ -0,0 +1,29 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef LIBBRILLO_BRILLO_TIMEZONE_TZIF_PARSER_H_
+#define LIBBRILLO_BRILLO_TIMEZONE_TZIF_PARSER_H_
+
+#include <string>
+
+#include <base/files/file_path.h>
+#include <base/optional.h>
+#include <brillo/brillo_export.h>
+
+namespace brillo {
+
+namespace timezone {
+
+// GetPosixTimezone takes a path to a tzfile, and returns the POSIX timezone in
+// a string. See 'man tzfile' for more info on the format. If |tzif_path| is a
+// relative path, it will be appended to /usr/share/zoneinfo/, otherwise
+// |tzif_path| as an absolute path will be used directly.
+base::Optional<std::string> BRILLO_EXPORT GetPosixTimezone(
+ const base::FilePath& tzif_path);
+
+} // namespace timezone
+
+} // namespace brillo
+
+#endif // LIBBRILLO_BRILLO_TIMEZONE_TZIF_PARSER_H_
diff --git a/brillo/timezone/tzif_parser_test.cc b/brillo/timezone/tzif_parser_test.cc
new file mode 100644
index 0000000..305da4d
--- /dev/null
+++ b/brillo/timezone/tzif_parser_test.cc
@@ -0,0 +1,46 @@
+// Copyright 2018 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include <stdlib.h>
+
+#include <base/files/file_path.h>
+#include <gtest/gtest.h>
+
+#include "brillo/timezone/tzif_parser.h"
+
+namespace brillo {
+
+namespace timezone {
+
+class TzifParserTest : public ::testing::Test {
+ public:
+ TzifParserTest() {
+ source_dir_ =
+ base::FilePath(getenv("SRC")).Append("brillo").Append("timezone");
+ }
+
+ protected:
+ base::FilePath source_dir_;
+};
+
+TEST_F(TzifParserTest, EST) {
+ auto posix_result = GetPosixTimezone(source_dir_.Append("EST_test.tzif"));
+ EXPECT_EQ(posix_result, "EST5");
+}
+
+TEST_F(TzifParserTest, TzifVersionTwo) {
+ auto posix_result =
+ GetPosixTimezone(source_dir_.Append("Indian_Christmas_test.tzif"));
+ EXPECT_EQ(posix_result, "<+07>-7");
+}
+
+TEST_F(TzifParserTest, TzifVersionThree) {
+ auto posix_result =
+ GetPosixTimezone(source_dir_.Append("Pacific_Fiji_test.tzif"));
+ EXPECT_EQ(posix_result, "<+12>-12<+13>,M11.1.0,M1.2.2/123");
+}
+
+} // namespace timezone
+
+} // namespace brillo
diff --git a/brillo/type_list.h b/brillo/type_list.h
index c5ccc5e..b14ef1e 100644
--- a/brillo/type_list.h
+++ b/brillo/type_list.h
@@ -48,6 +48,22 @@
using EnableIfIsOneOf =
std::enable_if_t<type_list::is_one_of<T, Types>::value>;
+// Enables a template if the type T is in the typelist Types and T is an
+// arithmetic type (some sort of int or floating-point number).
+template <typename T, typename Types>
+using EnableIfIsOneOfArithmetic =
+ std::enable_if_t<std::is_arithmetic<T>::value &&
+ type_list::is_one_of<T, Types>::value,
+ int>;
+
+// Enables a template if the type T is in the typelist Types and T is not an
+// arithmetic type (is void, nullptr_t, or a non-fundamental type).
+template <typename T, typename Types>
+using EnableIfIsOneOfNonArithmetic =
+ std::enable_if_t<!std::is_arithmetic<T>::value &&
+ type_list::is_one_of<T, Types>::value,
+ int>;
+
} // namespace brillo
#endif // LIBBRILLO_BRILLO_TYPE_LIST_H_
diff --git a/brillo/value_conversion_test.cc b/brillo/value_conversion_test.cc
index fec4052..2cf6d00 100644
--- a/brillo/value_conversion_test.cc
+++ b/brillo/value_conversion_test.cc
@@ -31,8 +31,10 @@
std::unique_ptr<base::Value> ParseValue(std::string json) {
std::replace(json.begin(), json.end(), '\'', '"');
std::string message;
- auto value = base::JSONReader::ReadAndReturnError(json, base::JSON_PARSE_RFC,
- nullptr, &message);
+ // TODO(crbug.com/1054279): use base::JSONReader::ReadAndReturnValueWithError
+ // after uprev to r680000.
+ auto value = base::JSONReader::ReadAndReturnErrorDeprecated(
+ json, base::JSON_PARSE_RFC, nullptr, &message);
CHECK(value) << "Failed to load JSON: " << message << ", " << json;
return value;
}
diff --git a/policy/OWNERS b/policy/OWNERS
index 0208469..74cea9e 100644
--- a/policy/OWNERS
+++ b/policy/OWNERS
@@ -1,5 +1,4 @@
emaxx@chromium.org
-ljusten@chromium.org
pmarko@chromium.org
poromov@chromium.org
rsorokin@chromium.org
diff --git a/policy/device_policy_impl.cc b/policy/device_policy_impl.cc
index 72d30b1..de082bf 100644
--- a/policy/device_policy_impl.cc
+++ b/policy/device_policy_impl.cc
@@ -16,6 +16,7 @@
#include <base/logging.h>
#include <base/macros.h>
#include <base/memory/ptr_util.h>
+#include <base/stl_util.h>
#include <base/time/time.h>
#include <base/values.h>
#include <openssl/evp.h>
@@ -100,7 +101,7 @@
"ethernet", "wifi", "wimax", "bluetooth", "cellular",
};
- if (type < 0 || type >= static_cast<int>(arraysize(kConnectionTypes)))
+ if (type < 0 || type >= static_cast<int>(base::size(kConnectionTypes)))
return std::string();
return kConnectionTypes[type];
@@ -153,10 +154,11 @@
std::unique_ptr<base::ListValue> DecodeListValueFromJSON(
const std::string& json_string) {
std::string error;
+ // TODO(crbug.com/1054279): use base::JSONReader::ReadAndReturnValueWithError
+ // after uprev to r680000.
std::unique_ptr<base::Value> decoded_json =
- base::JSONReader::ReadAndReturnError(json_string,
- base::JSON_ALLOW_TRAILING_COMMAS,
- nullptr, &error);
+ base::JSONReader::ReadAndReturnErrorDeprecated(
+ json_string, base::JSON_ALLOW_TRAILING_COMMAS, nullptr, &error);
if (!decoded_json) {
LOG(ERROR) << "Invalid JSON string: " << error;
return nullptr;