Merge "Revert "Be more aggressive about obtaining vold service.""
diff --git a/Android.mk b/Android.mk
index 5957df7..4d812b7 100644
--- a/Android.mk
+++ b/Android.mk
@@ -82,11 +82,10 @@
libbatteryservice \
libavb \
-# TODO: include "cert-err34-c" once we move to Binder
# TODO: include "cert-err58-cpp" once 36656327 is fixed
common_local_tidy_enabled := true
common_local_tidy_flags := -warnings-as-errors=clang-analyzer-security*,cert-*
-common_local_tidy_checks := -*,clang-analyzer-security*,cert-*,-cert-err34-c,-cert-err58-cpp
+common_local_tidy_checks := -*,clang-analyzer-security*,cert-*,-cert-err58-cpp
vold_conlyflags := -std=c11
vold_cflags := -Werror -Wall -Wno-missing-field-initializers -Wno-unused-variable -Wno-unused-parameter
diff --git a/Devmapper.cpp b/Devmapper.cpp
index ed498a3..2cbed86 100644
--- a/Devmapper.cpp
+++ b/Devmapper.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
+
#include <stdio.h>
#include <stdlib.h>
#include <fcntl.h>
@@ -34,6 +36,7 @@
#include <android-base/logging.h>
#include <android-base/stringprintf.h>
+#include <utils/Trace.h>
#include "Devmapper.h"
@@ -233,6 +236,7 @@
}
int Devmapper::destroyAll() {
+ ATRACE_NAME("Devmapper::destroyAll");
char *buffer = (char *) malloc(1024 * 64);
if (!buffer) {
SLOGE("Error allocating memory (%s)", strerror(errno));
diff --git a/Ext4Crypt.cpp b/Ext4Crypt.cpp
index dc2e42a..3ac6428 100644
--- a/Ext4Crypt.cpp
+++ b/Ext4Crypt.cpp
@@ -304,7 +304,7 @@
LOG(DEBUG) << "Skipping non-de-key " << entry->d_name;
continue;
}
- userid_t user_id = atoi(entry->d_name);
+ userid_t user_id = std::stoi(entry->d_name);
if (s_de_key_raw_refs.count(user_id) == 0) {
auto key_path = de_dir + "/" + entry->d_name;
KeyBuffer key;
diff --git a/Loop.cpp b/Loop.cpp
index fc3909b..1eb9865 100644
--- a/Loop.cpp
+++ b/Loop.cpp
@@ -14,8 +14,11 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
+
#include <stdio.h>
#include <stdlib.h>
+#include <dirent.h>
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
@@ -35,6 +38,7 @@
#include <android-base/logging.h>
#include <android-base/stringprintf.h>
#include <android-base/unique_fd.h>
+#include <utils/Trace.h>
#include "Loop.h"
#include "VoldUtil.h"
@@ -262,9 +266,22 @@
}
int Loop::destroyAll() {
- for (int i = 0; i < LOOP_MAX; i++) {
- auto path = StringPrintf("/dev/block/loop%d", i);
+ ATRACE_NAME("Loop::destroyAll");
+ DIR* dir;
+ struct dirent* de;
+
+ std::string root = "/dev/block/";
+ if (!(dir = opendir(root.c_str()))) {
+ PLOG(ERROR) << "Failed to opendir";
+ return -1;
+ }
+
+ // Poke through all devices looking for loops
+ while ((de = readdir(dir))) {
+ if (strncmp(de->d_name, "loop", 4) != 0) continue;
+
+ auto path = root + de->d_name;
unique_fd fd(open(path.c_str(), O_RDWR | O_CLOEXEC));
if (fd.get() == -1) {
if (errno != ENOENT) {
@@ -290,6 +307,8 @@
LOG(VERBOSE) << "Found unmanaged loop device at " << path << " named " << id;
}
}
+
+ closedir(dir);
return 0;
}
diff --git a/VoldNativeService.cpp b/VoldNativeService.cpp
index 310610c..049f5ba 100644
--- a/VoldNativeService.cpp
+++ b/VoldNativeService.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
+
#include "VoldNativeService.h"
#include "VolumeManager.h"
#include "BenchmarkTask.h"
@@ -32,6 +34,7 @@
#include <android-base/strings.h>
#include <fs_mgr.h>
#include <private/android_filesystem_config.h>
+#include <utils/Trace.h>
#ifndef LOG_TAG
#define LOG_TAG "vold"
@@ -169,10 +172,12 @@
}
#define ACQUIRE_LOCK \
- std::lock_guard<std::mutex> lock(VolumeManager::Instance()->getLock());
+ std::lock_guard<std::mutex> lock(VolumeManager::Instance()->getLock()); \
+ ATRACE_CALL();
#define ACQUIRE_CRYPT_LOCK \
- std::lock_guard<std::mutex> lock(VolumeManager::Instance()->getCryptLock());
+ std::lock_guard<std::mutex> lock(VolumeManager::Instance()->getCryptLock()); \
+ ATRACE_CALL();
} // namespace
diff --git a/VolumeManager.cpp b/VolumeManager.cpp
index 70f71f9..d441297 100644
--- a/VolumeManager.cpp
+++ b/VolumeManager.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
+
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
@@ -40,6 +42,7 @@
#include <android-base/stringprintf.h>
#include <cutils/fs.h>
#include <cutils/log.h>
+#include <utils/Trace.h>
#include <selinux/android.h>
@@ -94,6 +97,7 @@
}
int VolumeManager::updateVirtualDisk() {
+ ATRACE_NAME("VolumeManager::updateVirtualDisk");
if (property_get_bool(kPropVirtualDisk, false)) {
if (access(kPathVirtualDisk, F_OK) != 0) {
Loop::createImageFile(kPathVirtualDisk, kSizeVirtualDisk / 512);
@@ -148,6 +152,8 @@
}
int VolumeManager::start() {
+ ATRACE_NAME("VolumeManager::start");
+
// Always start from a clean slate by unmounting everything in
// directories that we own, in case we crashed.
unmountAll();
@@ -189,8 +195,8 @@
if (devType != "disk") return;
- int major = atoi(evt->findParam("MAJOR"));
- int minor = atoi(evt->findParam("MINOR"));
+ int major = std::stoi(evt->findParam("MAJOR"));
+ int minor = std::stoi(evt->findParam("MINOR"));
dev_t device = makedev(major, minor);
switch (evt->getAction()) {
@@ -548,6 +554,7 @@
int VolumeManager::unmountAll() {
std::lock_guard<std::mutex> lock(mLock);
+ ATRACE_NAME("VolumeManager::unmountAll()");
// First, try gracefully unmounting all known devices
if (mInternalEmulated != nullptr) {
diff --git a/cryptfs.cpp b/cryptfs.cpp
index adfb284..132b31f 100644
--- a/cryptfs.cpp
+++ b/cryptfs.cpp
@@ -1419,7 +1419,7 @@
*/
char ro_prop[PROPERTY_VALUE_MAX];
property_get("ro.crypto.readonly", ro_prop, "");
- if (strlen(ro_prop) > 0 && atoi(ro_prop)) {
+ if (strlen(ro_prop) > 0 && std::stoi(ro_prop)) {
struct fstab_rec* rec = fs_mgr_get_entry_for_mount_point(fstab, DATA_MNT_POINT);
rec->flags |= MS_RDONLY;
}
@@ -2553,30 +2553,23 @@
* Test if key is part of the multi-entry (field, index) sequence. Return non-zero if key is in the
* sequence and its index is greater than or equal to index. Return 0 otherwise.
*/
-static int match_multi_entry(const char *key, const char *field, unsigned index) {
- unsigned int field_len;
- unsigned int key_index;
- field_len = strlen(field);
+int match_multi_entry(const char *key, const char *field, unsigned index) {
+ std::string key_ = key;
+ std::string field_ = field;
- if (index == 0) {
- // The first key in a multi-entry field is just the filedname itself.
- if (!strcmp(key, field)) {
- return 1;
- }
+ std::string parsed_field;
+ unsigned parsed_index;
+
+ std::string::size_type split = key_.find_last_of('_');
+ if (split == std::string::npos) {
+ parsed_field = key_;
+ parsed_index = 0;
+ } else {
+ parsed_field = key_.substr(0, split);
+ parsed_index = std::stoi(key_.substr(split + 1));
}
- // Match key against "%s_%d" % (field, index)
- if (strlen(key) < field_len + 1 + 1) {
- // Need at least a '_' and a digit.
- return 0;
- }
- if (strncmp(key, field, field_len)) {
- // If the key does not begin with field, it's not a match.
- return 0;
- }
- if (1 != sscanf(&key[field_len],"_%d", &key_index)) {
- return 0;
- }
- return key_index >= index;
+
+ return parsed_field == field_ && parsed_index >= index;
}
/*
diff --git a/cryptfs.h b/cryptfs.h
index a7b650f..2169f54 100644
--- a/cryptfs.h
+++ b/cryptfs.h
@@ -222,6 +222,7 @@
extern "C" {
#endif
+ int match_multi_entry(const char *key, const char *field, unsigned index);
int wait_and_unmount(const char *mountpoint, bool kill);
typedef int (*kdf_func)(const char *passwd, const unsigned char *salt,
diff --git a/main.cpp b/main.cpp
index 37d02e7..f317f3d 100644
--- a/main.cpp
+++ b/main.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define ATRACE_TAG ATRACE_TAG_PACKAGE_MANAGER
+
#include "model/Disk.h"
#include "VolumeManager.h"
#include "NetlinkManager.h"
@@ -25,6 +27,7 @@
#include <android-base/stringprintf.h>
#include <cutils/klog.h>
#include <cutils/properties.h>
+#include <utils/Trace.h>
#include <stdio.h>
#include <stdlib.h>
@@ -51,6 +54,8 @@
setenv("ANDROID_LOG_TAGS", "*:v", 1);
android::base::InitLogging(argv, android::base::LogdLogger(android::base::SYSTEM));
+ ATRACE_BEGIN("main");
+
LOG(INFO) << "Vold 3.0 (the awakening) firing up";
LOG(VERBOSE) << "Detected support for:"
@@ -93,10 +98,12 @@
exit(1);
}
+ ATRACE_BEGIN("VoldNativeService::start");
if (android::vold::VoldNativeService::start() != android::OK) {
LOG(ERROR) << "Unable to start VoldNativeService";
exit(1);
}
+ ATRACE_END();
bool has_adoptable;
bool has_quota;
@@ -105,10 +112,12 @@
PLOG(ERROR) << "Error reading configuration... continuing anyways";
}
+ ATRACE_BEGIN("NetlinkManager::start");
if (nm->start()) {
PLOG(ERROR) << "Unable to start NetlinkManager";
exit(1);
}
+ ATRACE_END();
// This call should go after listeners are started to avoid
// a deadlock between vold and init (see b/34278978 for details)
@@ -119,6 +128,9 @@
// also the cold boot is needed in case we have flash drive
// connected before Vold launched
coldboot("/sys/block");
+
+ ATRACE_END();
+
// Eventually we'll become the monitoring thread
while(1) {
pause();
@@ -188,6 +200,7 @@
}
static void coldboot(const char *path) {
+ ATRACE_NAME("coldboot");
DIR *d = opendir(path);
if(d) {
do_coldboot(d, 0);
@@ -196,6 +209,8 @@
}
static int process_config(VolumeManager *vm, bool* has_adoptable, bool* has_quota) {
+ ATRACE_NAME("process_config");
+
fstab = fs_mgr_read_fstab_default();
if (!fstab) {
PLOG(ERROR) << "Failed to open default fstab";
diff --git a/model/Disk.cpp b/model/Disk.cpp
index f9799ad..c889a35 100644
--- a/model/Disk.cpp
+++ b/model/Disk.cpp
@@ -555,7 +555,7 @@
LOG(ERROR) << "Failed to read max minors";
return -errno;
}
- return atoi(tmp.c_str());
+ return std::stoi(tmp);
}
case kMajorBlockScsiA: case kMajorBlockScsiB: case kMajorBlockScsiC: case kMajorBlockScsiD:
case kMajorBlockScsiE: case kMajorBlockScsiF: case kMajorBlockScsiG: case kMajorBlockScsiH:
@@ -571,7 +571,7 @@
LOG(ERROR) << "Failed to read max minors";
return -errno;
}
- return atoi(tmp.c_str());
+ return std::stoi(tmp);
}
default: {
if (isVirtioBlkDevice(majorId)) {
diff --git a/tests/Android.mk b/tests/Android.mk
index 9cebd1a..3127352 100644
--- a/tests/Android.mk
+++ b/tests/Android.mk
@@ -10,7 +10,10 @@
LOCAL_STATIC_LIBRARIES := libbase libselinux libvold liblog libcrypto
-LOCAL_SRC_FILES := VolumeManager_test.cpp
+LOCAL_SRC_FILES := \
+ cryptfs_test.cpp \
+ VolumeManager_test.cpp \
+
LOCAL_MODULE := vold_tests
LOCAL_MODULE_TAGS := eng tests
diff --git a/tests/cryptfs_test.cpp b/tests/cryptfs_test.cpp
new file mode 100644
index 0000000..6875c0f
--- /dev/null
+++ b/tests/cryptfs_test.cpp
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include "../cryptfs.h"
+
+namespace android {
+
+class CryptfsTest : public testing::Test {
+protected:
+ virtual void SetUp() {
+ }
+
+ virtual void TearDown() {
+ }
+};
+
+TEST_F(CryptfsTest, MatchMultiEntryTest) {
+ ASSERT_NE(0, match_multi_entry("foo", "foo", 0));
+ ASSERT_NE(0, match_multi_entry("foo_0", "foo", 0));
+ ASSERT_NE(0, match_multi_entry("foo_1", "foo", 0));
+ ASSERT_NE(0, match_multi_entry("foo_2", "foo", 0));
+
+ ASSERT_EQ(0, match_multi_entry("foo", "foo", 1));
+ ASSERT_EQ(0, match_multi_entry("foo_0", "foo", 1));
+ ASSERT_NE(0, match_multi_entry("foo_1", "foo", 1));
+ ASSERT_NE(0, match_multi_entry("foo_2", "foo", 1));
+
+ ASSERT_EQ(0, match_multi_entry("foo", "foo", 2));
+ ASSERT_EQ(0, match_multi_entry("foo_0", "foo", 2));
+ ASSERT_EQ(0, match_multi_entry("foo_1", "foo", 2));
+ ASSERT_NE(0, match_multi_entry("foo_2", "foo", 2));
+
+ ASSERT_EQ(0, match_multi_entry("food", "foo", 0));
+ ASSERT_EQ(0, match_multi_entry("foo", "food", 0));
+ ASSERT_EQ(0, match_multi_entry("foo", "bar", 0));
+ ASSERT_EQ(0, match_multi_entry("foo_2", "bar", 0));
+}
+
+}