Enable "cert-err34-c" tidy checks.
Now that we've moved to Binder, we only have a few lingering atoi()
usages that are cleaned up in this CL.
Rewrite match_multi_entry() entirely, with tests to verify both old
and new implementations.
Test: adb shell /data/nativetest/vold_tests/vold_tests
Bug: 36655947
Change-Id: Ib79dc1ddc2366db4d5b4e1a1e2ed9456a06a983e
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/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/VolumeManager.cpp b/VolumeManager.cpp
index 70f71f9..6bed2bf 100644
--- a/VolumeManager.cpp
+++ b/VolumeManager.cpp
@@ -189,8 +189,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()) {
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/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));
+}
+
+}