Merge "Modernize string handling in BandwidthController" am: e219066110 am: 157607b449

am: 5129e90104

Change-Id: I63b34234b895911d8618ff8606b5fc1a5d4d1029

5 files changed, 168 insertions, 218 deletions

diff --git a/server/Android.mk b/server/Android.mk
index 9973ef59..5edbd204 100644
--- a/server/Android.mk
+++ b/server/Android.mk
@@ -180,7 +180,7 @@ LOCAL_SRC_FILES := \
         ../tests/tun_interface.cpp \
 
 LOCAL_MODULE_TAGS := tests
-LOCAL_STATIC_LIBRARIES := libgmock
+LOCAL_STATIC_LIBRARIES := libgmock libpcap
 LOCAL_SHARED_LIBRARIES := \
         libbase \
         libbinder \
@@ -190,7 +190,6 @@ LOCAL_SHARED_LIBRARIES := \
         libnetutils \
         libnetdutils \
         libnl \
-        libpcap \
         libsysutils \
         libutils \
 
diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 52604872..9aace9da 100644
--- a/server/BandwidthController.cpp
+++ b/server/BandwidthController.cpp
@@ -58,11 +58,11 @@
 
 /* Alphabetical */
 #define ALERT_IPT_TEMPLATE "%s %s -m quota2 ! --quota %" PRId64" --name %s\n"
-const char* BandwidthController::LOCAL_INPUT = "bw_INPUT";
-const char* BandwidthController::LOCAL_FORWARD = "bw_FORWARD";
-const char* BandwidthController::LOCAL_OUTPUT = "bw_OUTPUT";
-const char* BandwidthController::LOCAL_RAW_PREROUTING = "bw_raw_PREROUTING";
-const char* BandwidthController::LOCAL_MANGLE_POSTROUTING = "bw_mangle_POSTROUTING";
+const char BandwidthController::LOCAL_INPUT[] = "bw_INPUT";
+const char BandwidthController::LOCAL_FORWARD[] = "bw_FORWARD";
+const char BandwidthController::LOCAL_OUTPUT[] = "bw_OUTPUT";
+const char BandwidthController::LOCAL_RAW_PREROUTING[] = "bw_raw_PREROUTING";
+const char BandwidthController::LOCAL_MANGLE_POSTROUTING[] = "bw_mangle_POSTROUTING";
 
 auto BandwidthController::execFunction = android_fork_execvp;
 auto BandwidthController::popenFunction = popen;
@@ -76,7 +76,6 @@ namespace {
 const char ALERT_GLOBAL_NAME[] = "globalAlert";
 const int  MAX_CMD_ARGS = 32;
 const int  MAX_CMD_LEN = 1024;
-const int  MAX_IFACENAME_LEN = 64;
 const int  MAX_IPT_OUTPUT_LINE_LEN = 256;
 const std::string NEW_CHAIN_COMMAND = "-N ";
 const std::string GET_TETHER_STATS_COMMAND = StringPrintf(
@@ -84,6 +83,8 @@ const std::string GET_TETHER_STATS_COMMAND = StringPrintf(
     "-nvx -L %s\n"
     "COMMIT\n", NatController::LOCAL_TETHER_COUNTERS_CHAIN);
 
+const char NAUGHTY_CHAIN[] = "bw_penalty_box";
+const char NICE_CHAIN[] = "bw_happy_box";
 
 /**
  * Some comments about the rules:
@@ -194,30 +195,36 @@ static const std::vector<std::string> IPT_BASIC_ACCOUNTING_COMMANDS = {
     COMMIT_AND_CLOSE
 };
 
+std::vector<std::string> toStrVec(int num, char* strs[]) {
+    std::vector<std::string> tmp;
+    for (int i = 0; i < num; ++i) {
+        tmp.emplace_back(strs[i]);
+    }
+    return tmp;
+}
 
 }  // namespace
 
-BandwidthController::BandwidthController(void) {
+BandwidthController::BandwidthController() {
 }
 
-int BandwidthController::runIpxtablesCmd(const char *cmd, IptJumpOp jumpHandling,
+int BandwidthController::runIpxtablesCmd(const std::string& cmd, IptJumpOp jumpHandling,
                                          IptFailureLog failureHandling) {
     int res = 0;
 
-    ALOGV("runIpxtablesCmd(cmd=%s)", cmd);
+    ALOGV("runIpxtablesCmd(cmd=%s)", cmd.c_str());
     res |= runIptablesCmd(cmd, jumpHandling, IptIpV4, failureHandling);
     res |= runIptablesCmd(cmd, jumpHandling, IptIpV6, failureHandling);
     return res;
 }
 
-int BandwidthController::StrncpyAndCheck(char *buffer, const char *src, size_t buffSize) {
-
+int BandwidthController::StrncpyAndCheck(char* buffer, const std::string& src, size_t buffSize) {
     memset(buffer, '\0', buffSize);  // strncpy() is not filling leftover with '\0'
-    strncpy(buffer, src, buffSize);
+    strncpy(buffer, src.c_str(), buffSize);
     return buffer[buffSize - 1];
 }
 
-int BandwidthController::runIptablesCmd(const char *cmd, IptJumpOp jumpHandling,
+int BandwidthController::runIptablesCmd(const std::string& cmd, IptJumpOp jumpHandling,
                                         IptIpVer iptVer, IptFailureLog failureHandling) {
     char buffer[MAX_CMD_LEN];
     const char *argv[MAX_CMD_ARGS];
@@ -233,7 +240,7 @@ int BandwidthController::runIptablesCmd(const char *cmd, IptJumpOp jumpHandling,
     fullCmd.insert(0, " -w ");
     fullCmd.insert(0, iptVer == IptIpV4 ? IPTABLES_PATH : IP6TABLES_PATH);
 
-    if (StrncpyAndCheck(buffer, fullCmd.c_str(), sizeof(buffer))) {
+    if (StrncpyAndCheck(buffer, fullCmd, sizeof(buffer))) {
         ALOGE("iptables command too long");
         return -1;
     }
@@ -266,7 +273,7 @@ void BandwidthController::flushCleanTables(bool doClean) {
     iptablesRestoreFunction(V4V6, commands, nullptr);
 }
 
-int BandwidthController::setupIptablesHooks(void) {
+int BandwidthController::setupIptablesHooks() {
     /* flush+clean is allowed to fail */
     flushCleanTables(true);
     return 0;
@@ -293,7 +300,7 @@ int BandwidthController::enableBandwidthControl(bool force) {
     return iptablesRestoreFunction(V4V6, commands, nullptr);
 }
 
-int BandwidthController::disableBandwidthControl(void) {
+int BandwidthController::disableBandwidthControl() {
 
     flushCleanTables(false);
     return 0;
@@ -307,61 +314,41 @@ int BandwidthController::enableDataSaver(bool enable) {
     return iptablesRestoreFunction(V4V6, cmd, nullptr);
 }
 
-int BandwidthController::runCommands(int numCommands, const char *commands[],
-                                     RunCmdErrHandling cmdErrHandling) {
-    int res = 0;
-    IptFailureLog failureLogging = IptFailShow;
-    if (cmdErrHandling == RunCmdFailureOk) {
-        failureLogging = IptFailHide;
-    }
-    ALOGV("runCommands(): %d commands", numCommands);
-    for (int cmdNum = 0; cmdNum < numCommands; cmdNum++) {
-        res = runIpxtablesCmd(commands[cmdNum], IptJumpNoAdd, failureLogging);
-        if (res && cmdErrHandling != RunCmdFailureOk)
-            return res;
-    }
-    return 0;
-}
-
 int BandwidthController::addNaughtyApps(int numUids, char *appUids[]) {
-    return manipulateNaughtyApps(numUids, appUids, IptOpInsert);
+    return manipulateSpecialApps(toStrVec(numUids, appUids), NAUGHTY_CHAIN,
+                                 IptJumpReject, IptOpInsert);
 }
 
 int BandwidthController::removeNaughtyApps(int numUids, char *appUids[]) {
-    return manipulateNaughtyApps(numUids, appUids, IptOpDelete);
+    return manipulateSpecialApps(toStrVec(numUids, appUids), NAUGHTY_CHAIN,
+                                 IptJumpReject, IptOpDelete);
 }
 
 int BandwidthController::addNiceApps(int numUids, char *appUids[]) {
-    return manipulateNiceApps(numUids, appUids, IptOpInsert);
+    return manipulateSpecialApps(toStrVec(numUids, appUids), NICE_CHAIN,
+                                 IptJumpReturn, IptOpInsert);
 }
 
 int BandwidthController::removeNiceApps(int numUids, char *appUids[]) {
-    return manipulateNiceApps(numUids, appUids, IptOpDelete);
-}
-
-int BandwidthController::manipulateNaughtyApps(int numUids, char *appStrUids[], IptOp op) {
-    return manipulateSpecialApps(numUids, appStrUids, "bw_penalty_box", IptJumpReject, op);
+    return manipulateSpecialApps(toStrVec(numUids, appUids), NICE_CHAIN,
+                                 IptJumpReturn, IptOpDelete);
 }
 
-int BandwidthController::manipulateNiceApps(int numUids, char *appStrUids[], IptOp op) {
-    return manipulateSpecialApps(numUids, appStrUids, "bw_happy_box", IptJumpReturn, op);
-}
-
-int BandwidthController::manipulateSpecialApps(int numUids, char *appStrUids[],
-                                               const char *chain,
-                                               IptJumpOp jumpHandling, IptOp op) {
+int BandwidthController::manipulateSpecialApps(const std::vector<std::string>& appStrUids,
+                                               const std::string& chain, IptJumpOp jumpHandling,
+                                               IptOp op) {
     std::string cmd = "*filter\n";
-    for (int uidNum = 0; uidNum < numUids; uidNum++) {
-        StringAppendF(&cmd, "%s %s -m owner --uid-owner %s%s\n", opToString(op), chain,
-                      appStrUids[uidNum], jumpToString(jumpHandling));
+    for (const auto& appStrUid : appStrUids) {
+        StringAppendF(&cmd, "%s %s -m owner --uid-owner %s%s\n", opToString(op), chain.c_str(),
+                      appStrUid.c_str(), jumpToString(jumpHandling));
     }
     StringAppendF(&cmd, "COMMIT\n");
     return iptablesRestoreFunction(V4V6, cmd, nullptr);
 }
 
-std::string BandwidthController::makeIptablesQuotaCmd(IptFullOp op, const char *costName, int64_t quota) {
+std::string BandwidthController::makeIptablesQuotaCmd(IptFullOp op, const std::string& costName,
+                                                      int64_t quota) {
     std::string res;
-    char *buff;
     const char *opFlag;
 
     ALOGV("makeIptablesQuotaCmd(%d, %" PRId64")", op, quota);
@@ -379,14 +366,12 @@ std::string BandwidthController::makeIptablesQuotaCmd(IptFullOp op, const char *
     }
 
     // The requried IP version specific --jump REJECT ... will be added later.
-    asprintf(&buff, "%s bw_costly_%s -m quota2 ! --quota %" PRId64" --name %s", opFlag, costName, quota,
-             costName);
-    res = buff;
-    free(buff);
+    StringAppendF(&res, "%s bw_costly_%s -m quota2 ! --quota %" PRId64 " --name %s", opFlag,
+                  costName.c_str(), quota, costName.c_str());
     return res;
 }
 
-int BandwidthController::prepCostlyIface(const char *ifn, QuotaType quotaType) {
+int BandwidthController::prepCostlyIface(const std::string& ifn, QuotaType quotaType) {
     char cmd[MAX_CMD_LEN];
     int res = 0, res1, res2;
     int ruleInsertPos = 1;
@@ -423,27 +408,29 @@ int BandwidthController::prepCostlyIface(const char *ifn, QuotaType quotaType) {
         ruleInsertPos = 2;
     }
 
-    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn.c_str(), costCString);
     runIpxtablesCmd(cmd, IptJumpNoAdd, IptFailHide);
 
-    snprintf(cmd, sizeof(cmd), "-I bw_INPUT %d -i %s --jump %s", ruleInsertPos, ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-I bw_INPUT %d -i %s --jump %s", ruleInsertPos, ifn.c_str(),
+             costCString);
     res |= runIpxtablesCmd(cmd, IptJumpNoAdd);
 
-    snprintf(cmd, sizeof(cmd), "-D bw_OUTPUT -o %s --jump %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_OUTPUT -o %s --jump %s", ifn.c_str(), costCString);
     runIpxtablesCmd(cmd, IptJumpNoAdd, IptFailHide);
 
-    snprintf(cmd, sizeof(cmd), "-I bw_OUTPUT %d -o %s --jump %s", ruleInsertPos, ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-I bw_OUTPUT %d -o %s --jump %s", ruleInsertPos, ifn.c_str(),
+             costCString);
     res |= runIpxtablesCmd(cmd, IptJumpNoAdd);
 
-    snprintf(cmd, sizeof(cmd), "-D bw_FORWARD -o %s --jump %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_FORWARD -o %s --jump %s", ifn.c_str(), costCString);
     runIpxtablesCmd(cmd, IptJumpNoAdd, IptFailHide);
-    snprintf(cmd, sizeof(cmd), "-A bw_FORWARD -o %s --jump %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-A bw_FORWARD -o %s --jump %s", ifn.c_str(), costCString);
     res |= runIpxtablesCmd(cmd, IptJumpNoAdd);
 
     return res;
 }
 
-int BandwidthController::cleanupCostlyIface(const char *ifn, QuotaType quotaType) {
+int BandwidthController::cleanupCostlyIface(const std::string& ifn, QuotaType quotaType) {
     char cmd[MAX_CMD_LEN];
     int res = 0;
     std::string costString;
@@ -460,10 +447,10 @@ int BandwidthController::cleanupCostlyIface(const char *ifn, QuotaType quotaType
         break;
     }
 
-    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn, costCString);
+    snprintf(cmd, sizeof(cmd), "-D bw_INPUT -i %s --jump %s", ifn.c_str(), costCString);
     res |= runIpxtablesCmd(cmd, IptJumpNoAdd);
     for (const auto tableName : {LOCAL_OUTPUT, LOCAL_FORWARD}) {
-        snprintf(cmd, sizeof(cmd), "-D %s -o %s --jump %s", tableName, ifn, costCString);
+        snprintf(cmd, sizeof(cmd), "-D %s -o %s --jump %s", tableName, ifn.c_str(), costCString);
         res |= runIpxtablesCmd(cmd, IptJumpNoAdd);
     }
 
@@ -477,13 +464,10 @@ int BandwidthController::cleanupCostlyIface(const char *ifn, QuotaType quotaType
     return res;
 }
 
-int BandwidthController::setInterfaceSharedQuota(const char *iface, int64_t maxBytes) {
-    char ifn[MAX_IFACENAME_LEN];
+int BandwidthController::setInterfaceSharedQuota(const std::string& iface, int64_t maxBytes) {
     int res = 0;
     std::string quotaCmd;
-    std::string ifaceName;
-    ;
-    const char *costName = "shared";
+    const char costName[] = "shared";
     std::list<std::string>::iterator it;
 
     if (!maxBytes) {
@@ -493,24 +477,19 @@ int BandwidthController::setInterfaceSharedQuota(const char *iface, int64_t maxB
     }
     if (!isIfaceName(iface))
         return -1;
-    if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
-        ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
-        return -1;
-    }
-    ifaceName = ifn;
 
     if (maxBytes == -1) {
-        return removeInterfaceSharedQuota(ifn);
+        return removeInterfaceSharedQuota(iface);
     }
 
     /* Insert ingress quota. */
     for (it = sharedQuotaIfaces.begin(); it != sharedQuotaIfaces.end(); it++) {
-        if (*it == ifaceName)
+        if (*it == iface)
             break;
     }
 
     if (it == sharedQuotaIfaces.end()) {
-        res |= prepCostlyIface(ifn, QuotaShared);
+        res |= prepCostlyIface(iface, QuotaShared);
         if (sharedQuotaIfaces.empty()) {
             quotaCmd = makeIptablesQuotaCmd(IptFullOpInsert, costName, maxBytes);
             res |= runIpxtablesCmd(quotaCmd.c_str(), IptJumpReject);
@@ -520,7 +499,7 @@ int BandwidthController::setInterfaceSharedQuota(const char *iface, int64_t maxB
             }
             sharedQuotaBytes = maxBytes;
         }
-        sharedQuotaIfaces.push_front(ifaceName);
+        sharedQuotaIfaces.push_front(iface);
 
     }
 
@@ -541,36 +520,29 @@ int BandwidthController::setInterfaceSharedQuota(const char *iface, int64_t maxB
      * For now callers needs to choose if they want to "ndc bandwidth enable"
      * which resets everything.
      */
-    removeInterfaceSharedQuota(ifn);
+    removeInterfaceSharedQuota(iface);
     return -1;
 }
 
 /* It will also cleanup any shared alerts */
-int BandwidthController::removeInterfaceSharedQuota(const char *iface) {
-    char ifn[MAX_IFACENAME_LEN];
+int BandwidthController::removeInterfaceSharedQuota(const std::string& iface) {
     int res = 0;
-    std::string ifaceName;
     std::list<std::string>::iterator it;
-    const char *costName = "shared";
+    const char costName[] = "shared";
 
     if (!isIfaceName(iface))
         return -1;
-    if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
-        ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
-        return -1;
-    }
-    ifaceName = ifn;
 
     for (it = sharedQuotaIfaces.begin(); it != sharedQuotaIfaces.end(); it++) {
-        if (*it == ifaceName)
+        if (*it == iface)
             break;
     }
     if (it == sharedQuotaIfaces.end()) {
-        ALOGE("No such iface %s to delete", ifn);
+        ALOGE("No such iface %s to delete", iface.c_str());
         return -1;
     }
 
-    res |= cleanupCostlyIface(ifn, QuotaShared);
+    res |= cleanupCostlyIface(iface, QuotaShared);
     sharedQuotaIfaces.erase(it);
 
     if (sharedQuotaIfaces.empty()) {
@@ -586,11 +558,9 @@ int BandwidthController::removeInterfaceSharedQuota(const char *iface) {
     return res;
 }
 
-int BandwidthController::setInterfaceQuota(const char *iface, int64_t maxBytes) {
-    char ifn[MAX_IFACENAME_LEN];
+int BandwidthController::setInterfaceQuota(const std::string& iface, int64_t maxBytes) {
     int res = 0;
-    std::string ifaceName;
-    const char *costName;
+    const auto& costName = iface;
     std::list<QuotaInfo>::iterator it;
     std::string quotaCmd;
 
@@ -606,22 +576,15 @@ int BandwidthController::setInterfaceQuota(const char *iface, int64_t maxBytes)
         return removeInterfaceQuota(iface);
     }
 
-    if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
-        ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
-        return -1;
-    }
-    ifaceName = ifn;
-    costName = iface;
-
     /* Insert ingress quota. */
     for (it = quotaIfaces.begin(); it != quotaIfaces.end(); it++) {
-        if (it->ifaceName == ifaceName)
+        if (it->ifaceName == iface)
             break;
     }
 
     if (it == quotaIfaces.end()) {
         /* Preparing the iface adds a penalty/happy box check */
-        res |= prepCostlyIface(ifn, QuotaUnique);
+        res |= prepCostlyIface(iface, QuotaUnique);
         /*
          * The rejecting quota limit should go after the penalty/happy box checks
          * or else a naughty app could just eat up the quota.
@@ -634,12 +597,12 @@ int BandwidthController::setInterfaceQuota(const char *iface, int64_t maxBytes)
             goto fail;
         }
 
-        quotaIfaces.push_front(QuotaInfo(ifaceName, maxBytes, 0));
+        quotaIfaces.push_front(QuotaInfo(iface, maxBytes, 0));
 
     } else {
         res |= updateQuota(costName, maxBytes);
         if (res) {
-            ALOGE("Failed update quota for %s", iface);
+            ALOGE("Failed update quota for %s", iface.c_str());
             goto fail;
         }
         it->quota = maxBytes;
@@ -653,7 +616,7 @@ int BandwidthController::setInterfaceQuota(const char *iface, int64_t maxBytes)
      * For now callers needs to choose if they want to "ndc bandwidth enable"
      * which resets everything.
      */
-    removeInterfaceSharedQuota(ifn);
+    removeInterfaceSharedQuota(iface);
     return -1;
 }
 
@@ -661,19 +624,16 @@ int BandwidthController::getInterfaceSharedQuota(int64_t *bytes) {
     return getInterfaceQuota("shared", bytes);
 }
 
-int BandwidthController::getInterfaceQuota(const char *costName, int64_t *bytes) {
+int BandwidthController::getInterfaceQuota(const std::string& iface, int64_t* bytes) {
     FILE *fp;
-    char *fname;
+    const std::string fname = "/proc/net/xt_quota/" + iface;
     int scanRes;
 
-    if (!isIfaceName(costName))
-        return -1;
+    if (!isIfaceName(iface)) return -1;
 
-    asprintf(&fname, "/proc/net/xt_quota/%s", costName);
-    fp = fopen(fname, "re");
-    free(fname);
+    fp = fopen(fname.c_str(), "re");
     if (!fp) {
-        ALOGE("Reading quota %s failed (%s)", costName, strerror(errno));
+        ALOGE("Reading quota %s failed (%s)", iface.c_str(), strerror(errno));
         return -1;
     }
     scanRes = fscanf(fp, "%" SCNd64, bytes);
@@ -682,53 +642,45 @@ int BandwidthController::getInterfaceQuota(const char *costName, int64_t *bytes)
     return scanRes == 1 ? 0 : -1;
 }
 
-int BandwidthController::removeInterfaceQuota(const char *iface) {
-
-    char ifn[MAX_IFACENAME_LEN];
+int BandwidthController::removeInterfaceQuota(const std::string& iface) {
     int res = 0;
-    std::string ifaceName;
     std::list<QuotaInfo>::iterator it;
 
     if (!isIfaceName(iface))
         return -1;
-    if (StrncpyAndCheck(ifn, iface, sizeof(ifn))) {
-        ALOGE("Interface name longer than %d", MAX_IFACENAME_LEN);
-        return -1;
-    }
-    ifaceName = ifn;
 
     for (it = quotaIfaces.begin(); it != quotaIfaces.end(); it++) {
-        if (it->ifaceName == ifaceName)
+        if (it->ifaceName == iface)
             break;
     }
 
     if (it == quotaIfaces.end()) {
-        ALOGE("No such iface %s to delete", ifn);
+        ALOGE("No such iface %s to delete", iface.c_str());
         return -1;
     }
 
     /* This also removes the quota command of CostlyIface chain. */
-    res |= cleanupCostlyIface(ifn, QuotaUnique);
+    res |= cleanupCostlyIface(iface, QuotaUnique);
 
     quotaIfaces.erase(it);
 
     return res;
 }
 
-int BandwidthController::updateQuota(const char *quotaName, int64_t bytes) {
+int BandwidthController::updateQuota(const std::string& quotaName, int64_t bytes) {
     FILE *fp;
     char *fname;
 
     if (!isIfaceName(quotaName)) {
-        ALOGE("updateQuota: Invalid quotaName \"%s\"", quotaName);
+        ALOGE("updateQuota: Invalid quotaName \"%s\"", quotaName.c_str());
         return -1;
     }
 
-    asprintf(&fname, "/proc/net/xt_quota/%s", quotaName);
+    asprintf(&fname, "/proc/net/xt_quota/%s", quotaName.c_str());
     fp = fopen(fname, "we");
     free(fname);
     if (!fp) {
-        ALOGE("Updating quota %s failed (%s)", quotaName, strerror(errno));
+        ALOGE("Updating quota %s failed (%s)", quotaName.c_str(), strerror(errno));
         return -1;
     }
     fprintf(fp, "%" PRId64"\n", bytes);
@@ -736,23 +688,28 @@ int BandwidthController::updateQuota(const char *quotaName, int64_t bytes) {
     return 0;
 }
 
-int BandwidthController::runIptablesAlertCmd(IptOp op, const char *alertName, int64_t bytes) {
+int BandwidthController::runIptablesAlertCmd(IptOp op, const std::string& alertName,
+                                             int64_t bytes) {
     const char *opFlag = opToString(op);
     std::string alertQuotaCmd = "*filter\n";
 
     // TODO: consider using an alternate template for the delete that does not include the --quota
     // value. This code works because the --quota value is ignored by deletes
-    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_INPUT", bytes, alertName);
-    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_OUTPUT", bytes, alertName);
+    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_INPUT", bytes,
+                  alertName.c_str());
+    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_OUTPUT", bytes,
+                  alertName.c_str());
     StringAppendF(&alertQuotaCmd, "COMMIT\n");
 
     return iptablesRestoreFunction(V4V6, alertQuotaCmd, nullptr);
 }
 
-int BandwidthController::runIptablesAlertFwdCmd(IptOp op, const char *alertName, int64_t bytes) {
+int BandwidthController::runIptablesAlertFwdCmd(IptOp op, const std::string& alertName,
+                                                int64_t bytes) {
     const char *opFlag = opToString(op);
     std::string alertQuotaCmd = "*filter\n";
-    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_FORWARD", bytes, alertName);
+    StringAppendF(&alertQuotaCmd, ALERT_IPT_TEMPLATE, opFlag, "bw_FORWARD", bytes,
+                  alertName.c_str());
     StringAppendF(&alertQuotaCmd, "COMMIT\n");
 
     return iptablesRestoreFunction(V4V6, alertQuotaCmd, nullptr);
@@ -779,7 +736,7 @@ int BandwidthController::setGlobalAlert(int64_t bytes) {
     return res;
 }
 
-int BandwidthController::setGlobalAlertInForwardChain(void) {
+int BandwidthController::setGlobalAlertInForwardChain() {
     const char *alertName = ALERT_GLOBAL_NAME;
     int res = 0;
 
@@ -800,7 +757,7 @@ int BandwidthController::setGlobalAlertInForwardChain(void) {
     return res;
 }
 
-int BandwidthController::removeGlobalAlert(void) {
+int BandwidthController::removeGlobalAlert() {
 
     const char *alertName = ALERT_GLOBAL_NAME;
     int res = 0;
@@ -817,7 +774,7 @@ int BandwidthController::removeGlobalAlert(void) {
     return res;
 }
 
-int BandwidthController::removeGlobalAlertInForwardChain(void) {
+int BandwidthController::removeGlobalAlertInForwardChain() {
     int res = 0;
     const char *alertName = ALERT_GLOBAL_NAME;
 
@@ -853,15 +810,15 @@ int BandwidthController::setSharedAlert(int64_t bytes) {
     return setCostlyAlert("shared", bytes, &sharedAlertBytes);
 }
 
-int BandwidthController::removeSharedAlert(void) {
+int BandwidthController::removeSharedAlert() {
     return removeCostlyAlert("shared", &sharedAlertBytes);
 }
 
-int BandwidthController::setInterfaceAlert(const char *iface, int64_t bytes) {
+int BandwidthController::setInterfaceAlert(const std::string& iface, int64_t bytes) {
     std::list<QuotaInfo>::iterator it;
 
     if (!isIfaceName(iface)) {
-        ALOGE("setInterfaceAlert: Invalid iface \"%s\"", iface);
+        ALOGE("setInterfaceAlert: Invalid iface \"%s\"", iface.c_str());
         return -1;
     }
 
@@ -882,11 +839,11 @@ int BandwidthController::setInterfaceAlert(const char *iface, int64_t bytes) {
     return setCostlyAlert(iface, bytes, &it->alert);
 }
 
-int BandwidthController::removeInterfaceAlert(const char *iface) {
+int BandwidthController::removeInterfaceAlert(const std::string& iface) {
     std::list<QuotaInfo>::iterator it;
 
     if (!isIfaceName(iface)) {
-        ALOGE("removeInterfaceAlert: Invalid iface \"%s\"", iface);
+        ALOGE("removeInterfaceAlert: Invalid iface \"%s\"", iface.c_str());
         return -1;
     }
 
@@ -896,21 +853,22 @@ int BandwidthController::removeInterfaceAlert(const char *iface) {
     }
 
     if (it == quotaIfaces.end()) {
-        ALOGE("No prior alert set for interface %s", iface);
+        ALOGE("No prior alert set for interface %s", iface.c_str());
         return -1;
     }
 
     return removeCostlyAlert(iface, &it->alert);
 }
 
-int BandwidthController::setCostlyAlert(const char *costName, int64_t bytes, int64_t *alertBytes) {
+int BandwidthController::setCostlyAlert(const std::string& costName, int64_t bytes,
+                                        int64_t* alertBytes) {
     char *alertQuotaCmd;
     char *chainName;
     int res = 0;
     char *alertName;
 
     if (!isIfaceName(costName)) {
-        ALOGE("setCostlyAlert: Invalid costName \"%s\"", costName);
+        ALOGE("setCostlyAlert: Invalid costName \"%s\"", costName.c_str());
         return -1;
     }
 
@@ -918,11 +876,11 @@ int BandwidthController::setCostlyAlert(const char *costName, int64_t bytes, int
         ALOGE("Invalid bytes value. 1..max_int64.");
         return -1;
     }
-    asprintf(&alertName, "%sAlert", costName);
+    asprintf(&alertName, "%sAlert", costName.c_str());
     if (*alertBytes) {
         res = updateQuota(alertName, *alertBytes);
     } else {
-        asprintf(&chainName, "bw_costly_%s", costName);
+        asprintf(&chainName, "bw_costly_%s", costName.c_str());
         asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, "-A", chainName, bytes, alertName);
         res |= runIpxtablesCmd(alertQuotaCmd, IptJumpNoAdd);
         free(alertQuotaCmd);
@@ -933,24 +891,24 @@ int BandwidthController::setCostlyAlert(const char *costName, int64_t bytes, int
     return res;
 }
 
-int BandwidthController::removeCostlyAlert(const char *costName, int64_t *alertBytes) {
+int BandwidthController::removeCostlyAlert(const std::string& costName, int64_t* alertBytes) {
     char *alertQuotaCmd;
     char *chainName;
     char *alertName;
     int res = 0;
 
     if (!isIfaceName(costName)) {
-        ALOGE("removeCostlyAlert: Invalid costName \"%s\"", costName);
+        ALOGE("removeCostlyAlert: Invalid costName \"%s\"", costName.c_str());
         return -1;
     }
 
     if (!*alertBytes) {
-        ALOGE("No prior alert set for %s alert", costName);
+        ALOGE("No prior alert set for %s alert", costName.c_str());
         return -1;
     }
 
-    asprintf(&alertName, "%sAlert", costName);
-    asprintf(&chainName, "bw_costly_%s", costName);
+    asprintf(&alertName, "%sAlert", costName.c_str());
+    asprintf(&chainName, "bw_costly_%s", costName.c_str());
     asprintf(&alertQuotaCmd, ALERT_IPT_TEMPLATE, "-D", chainName, *alertBytes, alertName);
     res |= runIpxtablesCmd(alertQuotaCmd, IptJumpNoAdd);
     free(alertQuotaCmd);
@@ -1005,9 +963,7 @@ int BandwidthController::addForwardChainStats(const TetherStats& filter,
 
     bool filterPair = filter.intIface[0] && filter.extIface[0];
 
-    char *filterMsg = filter.getStatsLine();
-    ALOGV("filter: %s",  filterMsg);
-    free(filterMsg);
+    ALOGV("filter: %s",  filter.getStatsLine().c_str());
 
     stats = filter;
 
@@ -1099,10 +1055,10 @@ int BandwidthController::addForwardChainStats(const TetherStats& filter,
     return 0;
 }
 
-char *BandwidthController::TetherStats::getStatsLine(void) const {
-    char *msg;
-    asprintf(&msg, "%s %s %" PRId64" %" PRId64" %" PRId64" %" PRId64, intIface.c_str(), extIface.c_str(),
-            rxBytes, rxPackets, txBytes, txPackets);
+std::string BandwidthController::TetherStats::getStatsLine() const {
+    std::string msg;
+    StringAppendF(&msg, "%s %s %" PRId64" %" PRId64" %" PRId64" %" PRId64, intIface.c_str(),
+                  extIface.c_str(), rxBytes, rxPackets, txBytes, txPackets);
     return msg;
 }
 
@@ -1127,10 +1083,12 @@ int BandwidthController::getTetherStats(SocketClient *cli, TetherStats& filter,
     }
 
     if (filter.intIface[0] && filter.extIface[0] && statsList.size() == 1) {
-        cli->sendMsg(ResponseCode::TetheringStatsResult, statsList[0].getStatsLine(), false);
+        cli->sendMsg(ResponseCode::TetheringStatsResult,
+                     statsList[0].getStatsLine().c_str(), false);
     } else {
         for (const auto& stats: statsList) {
-            cli->sendMsg(ResponseCode::TetheringStatsListResult, stats.getStatsLine(), false);
+            cli->sendMsg(ResponseCode::TetheringStatsListResult,
+                         stats.getStatsLine().c_str(), false);
         }
         if (res == 0) {
             cli->sendMsg(ResponseCode::CommandOkay, "Tethering stats list completed", false);
diff --git a/server/BandwidthController.h b/server/BandwidthController.h
index 0a51346b..b739841e 100644
--- a/server/BandwidthController.h
+++ b/server/BandwidthController.h
@@ -18,7 +18,8 @@
 
 #include <list>
 #include <string>
-#include <utility>  // for pair
+#include <utility>
+#include <vector>
 
 #include <sysutils/SocketClient.h>
 #include <utils/RWLock.h>
@@ -31,9 +32,7 @@ public:
 
     class TetherStats {
     public:
-        TetherStats(void)
-                : rxBytes(-1), rxPackets(-1),
-                    txBytes(-1), txPackets(-1) {};
+        TetherStats() = default;
         TetherStats(std::string intIfn, std::string extIfn,
                 int64_t rxB, int64_t rxP,
                 int64_t txB, int64_t txP)
@@ -44,14 +43,16 @@ public:
         std::string intIface;
         /* External interface. Same as NatController's notion. */
         std::string extIface;
-        int64_t rxBytes, rxPackets;
-        int64_t txBytes, txPackets;
+        int64_t rxBytes = -1;
+        int64_t rxPackets = -1;
+        int64_t txBytes = -1;
+        int64_t txPackets = -1;
         /*
          * Allocates a new string representing this:
          * intIface extIface rx_bytes rx_packets tx_bytes tx_packets
          * The caller is responsible for free()'ing the returned ptr.
          */
-        char *getStatsLine(void) const;
+        std::string getStatsLine() const;
 
         bool addStatsIfMatch(const TetherStats& other) {
             if (intIface == other.intIface && extIface == other.extIface) {
@@ -67,19 +68,19 @@ public:
 
     BandwidthController();
 
-    int setupIptablesHooks(void);
+    int setupIptablesHooks();
 
     int enableBandwidthControl(bool force);
-    int disableBandwidthControl(void);
+    int disableBandwidthControl();
     int enableDataSaver(bool enable);
 
-    int setInterfaceSharedQuota(const char *iface, int64_t bytes);
+    int setInterfaceSharedQuota(const std::string& iface, int64_t bytes);
     int getInterfaceSharedQuota(int64_t *bytes);
-    int removeInterfaceSharedQuota(const char *iface);
+    int removeInterfaceSharedQuota(const std::string& iface);
 
-    int setInterfaceQuota(const char *iface, int64_t bytes);
-    int getInterfaceQuota(const char *iface, int64_t *bytes);
-    int removeInterfaceQuota(const char *iface);
+    int setInterfaceQuota(const std::string& iface, int64_t bytes);
+    int getInterfaceQuota(const std::string& iface, int64_t* bytes);
+    int removeInterfaceQuota(const std::string& iface);
 
     int addNaughtyApps(int numUids, char *appUids[]);
     int removeNaughtyApps(int numUids, char *appUids[]);
@@ -87,15 +88,15 @@ public:
     int removeNiceApps(int numUids, char *appUids[]);
 
     int setGlobalAlert(int64_t bytes);
-    int removeGlobalAlert(void);
-    int setGlobalAlertInForwardChain(void);
-    int removeGlobalAlertInForwardChain(void);
+    int removeGlobalAlert();
+    int setGlobalAlertInForwardChain();
+    int removeGlobalAlertInForwardChain();
 
     int setSharedAlert(int64_t bytes);
-    int removeSharedAlert(void);
+    int removeSharedAlert();
 
-    int setInterfaceAlert(const char *iface, int64_t bytes);
-    int removeInterfaceAlert(const char *iface);
+    int setInterfaceAlert(const std::string& iface, int64_t bytes);
+    int removeInterfaceAlert(const std::string& iface);
 
     /*
      * For single pair of ifaces, stats should have ifaceIn and ifaceOut initialized.
@@ -107,13 +108,13 @@ public:
      */
     int getTetherStats(SocketClient *cli, TetherStats &stats, std::string &extraProcessingInfo);
 
-    static const char* LOCAL_INPUT;
-    static const char* LOCAL_FORWARD;
-    static const char* LOCAL_OUTPUT;
-    static const char* LOCAL_RAW_PREROUTING;
-    static const char* LOCAL_MANGLE_POSTROUTING;
+    static const char LOCAL_INPUT[];
+    static const char LOCAL_FORWARD[];
+    static const char LOCAL_OUTPUT[];
+    static const char LOCAL_RAW_PREROUTING[];
+    static const char LOCAL_MANGLE_POSTROUTING[];
 
-protected:
+  private:
     class QuotaInfo {
     public:
       QuotaInfo(std::string ifn, int64_t q, int64_t a)
@@ -135,37 +136,30 @@ protected:
     enum IptFailureLog { IptFailShow, IptFailHide = IptFailShow };
 #endif
 
-    int manipulateSpecialApps(int numUids, char *appStrUids[],
-                               const char *chain,
-                               IptJumpOp jumpHandling, IptOp appOp);
-    int manipulateNaughtyApps(int numUids, char *appStrUids[], IptOp appOp);
-    int manipulateNiceApps(int numUids, char *appStrUids[], IptOp appOp);
+    int manipulateSpecialApps(const std::vector<std::string>& appStrUids, const std::string& chain,
+                              IptJumpOp jumpHandling, IptOp appOp);
 
-    int prepCostlyIface(const char *ifn, QuotaType quotaType);
-    int cleanupCostlyIface(const char *ifn, QuotaType quotaType);
+    int prepCostlyIface(const std::string& ifn, QuotaType quotaType);
+    int cleanupCostlyIface(const std::string& ifn, QuotaType quotaType);
 
-    std::string makeIptablesSpecialAppCmd(IptOp op, int uid, const char *chain);
-    std::string makeIptablesQuotaCmd(IptFullOp op, const char *costName, int64_t quota);
+    std::string makeIptablesQuotaCmd(IptFullOp op, const std::string& costName, int64_t quota);
 
-    int runIptablesAlertCmd(IptOp op, const char *alertName, int64_t bytes);
-    int runIptablesAlertFwdCmd(IptOp op, const char *alertName, int64_t bytes);
+    int runIptablesAlertCmd(IptOp op, const std::string& alertName, int64_t bytes);
+    int runIptablesAlertFwdCmd(IptOp op, const std::string& alertName, int64_t bytes);
 
-    /* Runs for both ipv4 and ipv6 iptables */
-    int runCommands(int numCommands, const char *commands[], RunCmdErrHandling cmdErrHandling);
     /* Runs for both ipv4 and ipv6 iptables, appends -j REJECT --reject-with ...  */
-    static int runIpxtablesCmd(const char *cmd, IptJumpOp jumpHandling,
+    static int runIpxtablesCmd(const std::string& cmd, IptJumpOp jumpHandling,
                                IptFailureLog failureHandling = IptFailShow);
-    static int runIptablesCmd(const char *cmd, IptJumpOp jumpHandling, IptIpVer iptIpVer,
+    static int runIptablesCmd(const std::string& cmd, IptJumpOp jumpHandling, IptIpVer iptIpVer,
                               IptFailureLog failureHandling = IptFailShow);
 
-
     // Provides strncpy() + check overflow.
-    static int StrncpyAndCheck(char *buffer, const char *src, size_t buffSize);
+    static int StrncpyAndCheck(char* buffer, const std::string& src, size_t buffSize);
 
-    int updateQuota(const char *alertName, int64_t bytes);
+    int updateQuota(const std::string& alertName, int64_t bytes);
 
-    int setCostlyAlert(const char *costName, int64_t bytes, int64_t *alertBytes);
-    int removeCostlyAlert(const char *costName, int64_t *alertBytes);
+    int setCostlyAlert(const std::string& costName, int64_t bytes, int64_t* alertBytes);
+    int removeCostlyAlert(const std::string& costName, int64_t* alertBytes);
 
     typedef std::vector<TetherStats> TetherStatsList;
 
diff --git a/server/NetdConstants.cpp b/server/NetdConstants.cpp
index 0d87264d..0a0ca5da 100644
--- a/server/NetdConstants.cpp
+++ b/server/NetdConstants.cpp
@@ -140,10 +140,9 @@ int execIptablesRestoreCommand(IptablesTarget target, const std::string& table,
  * Check an interface name for plausibility. This should e.g. help against
  * directory traversal.
  */
-bool isIfaceName(const char *name) {
+bool isIfaceName(const std::string& name) {
     size_t i;
-    size_t name_len = strlen(name);
-    if ((name_len == 0) || (name_len > IFNAMSIZ)) {
+    if ((name.empty()) || (name.size() > IFNAMSIZ)) {
         return false;
     }
 
@@ -152,7 +151,7 @@ bool isIfaceName(const char *name) {
         return false;
     }
 
-    for (i = 1; i < name_len; i++) {
+    for (i = 1; i < name.size(); i++) {
         if (!isalnum(name[i]) && (name[i] != '_') && (name[i] != '-') && (name[i] != ':')) {
             return false;
         }
diff --git a/server/NetdConstants.h b/server/NetdConstants.h
index 4bb261e7..54ed8126 100644
--- a/server/NetdConstants.h
+++ b/server/NetdConstants.h
@@ -49,7 +49,7 @@ int execIptablesRestoreWithOutput(IptablesTarget target, const std::string& comm
                                   std::string *output);
 int execIptablesRestoreCommand(IptablesTarget target, const std::string& table,
                                const std::string& command, std::string *output);
-bool isIfaceName(const char *name);
+bool isIfaceName(const std::string& name);
 int parsePrefix(const char *prefix, uint8_t *family, void *address, int size, uint8_t *prefixlen);
 void blockSigpipe();