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

Android.mk

@@ -82,11 +82,10 @@ common_static_libraries := \
 	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

Ext4Crypt.cpp

@@ -304,7 +304,7 @@ static bool load_all_de_keys() {
             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;

VolumeManager.cpp

@@ -189,8 +189,8 @@ void VolumeManager::handleBlockEvent(NetlinkEvent *evt) {
 
     if (devType != "disk") return;
 
-    int major = atoi(evt->findParam("MAJOR"));
+    int major = std::stoi(evt->findParam("MAJOR"));
-    int minor = atoi(evt->findParam("MINOR"));
+    int minor = std::stoi(evt->findParam("MINOR"));
     dev_t device = makedev(major, minor);
 
     switch (evt->getAction()) {

cryptfs.cpp

@@ -1419,7 +1419,7 @@ static int cryptfs_restart_internal(int restart_main)
          */
         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 @@ static int persist_set_key(const char *fieldname, const char *value, int encrypt
  * 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) {
+int match_multi_entry(const char *key, const char *field, unsigned index) {
-    unsigned int field_len;
+    std::string key_ = key;
-    unsigned int key_index;
+    std::string field_ = field;
-    field_len = strlen(field);
 
-    if (index == 0) {
+    std::string parsed_field;
-        // The first key in a multi-entry field is just the filedname itself.
+    unsigned parsed_index;
-        if (!strcmp(key, field)) {
+
-            return 1;
+    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) {
+    return parsed_field == field_ && parsed_index >= index;
-        // 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;
 }
 
 /*

cryptfs.h

@@ -222,6 +222,7 @@ struct crypt_persist_data {
 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,

model/Disk.cpp

@@ -555,7 +555,7 @@ int Disk::getMaxMinors() {
             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 @@ int Disk::getMaxMinors() {
             LOG(ERROR) << "Failed to read max minors";
             return -errno;
         }
-        return atoi(tmp.c_str());
+        return std::stoi(tmp);
     }
     default: {
         if (isVirtioBlkDevice(majorId)) {

tests/Android.mk

@@ -10,7 +10,10 @@ LOCAL_C_INCLUDES := \
 
 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
 

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));
+}
+
+}