Modernize string handling in BandwidthController

This change is preparation for removal of xt_quota2 in favor of NFLOG.
Note that the scope of changes is mostly limited to mechanical single
line changes from "const char*" to "const std::string&".

Test: as follows
    - built
    - flashed
    - booted
    - "runtest -x .../netd_unit_test.cpp" passes
    - "runtest -x .../netd_integration_test.cpp" passes
Bug: 38143143
Bug: 28362720

Change-Id: I56ba810ff6fa2f409e32d86508cfdb1a81a50a4e
diff --git a/server/BandwidthController.cpp b/server/BandwidthController.cpp
index 5260487..9aace9d 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 @@
 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 @@
     "-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 @@
     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 @@
     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 @@
     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 @@
     return iptablesRestoreFunction(V4V6, commands, nullptr);
 }
 
-int BandwidthController::disableBandwidthControl(void) {
+int BandwidthController::disableBandwidthControl() {
 
     flushCleanTables(false);
     return 0;
@@ -307,61 +314,41 @@
     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);
+    return manipulateSpecialApps(toStrVec(numUids, appUids), NICE_CHAIN,
+                                 IptJumpReturn, IptOpDelete);
 }
 
-int BandwidthController::manipulateNaughtyApps(int numUids, char *appStrUids[], IptOp op) {
-    return manipulateSpecialApps(numUids, appStrUids, "bw_penalty_box", IptJumpReject, op);
-}
-
-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 @@
     }
 
     // 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 @@
         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 @@
         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 @@
     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 @@
     }
     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 @@
             }
             sharedQuotaBytes = maxBytes;
         }
-        sharedQuotaIfaces.push_front(ifaceName);
+        sharedQuotaIfaces.push_front(iface);
 
     }
 
@@ -541,36 +520,29 @@
      * 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 @@
     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 @@
         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 @@
             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 @@
      * 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 @@
     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 @@
     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 @@
     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 @@
     return res;
 }
 
-int BandwidthController::setGlobalAlertInForwardChain(void) {
+int BandwidthController::setGlobalAlertInForwardChain() {
     const char *alertName = ALERT_GLOBAL_NAME;
     int res = 0;
 
@@ -800,7 +757,7 @@
     return res;
 }
 
-int BandwidthController::removeGlobalAlert(void) {
+int BandwidthController::removeGlobalAlert() {
 
     const char *alertName = ALERT_GLOBAL_NAME;
     int res = 0;
@@ -817,7 +774,7 @@
     return res;
 }
 
-int BandwidthController::removeGlobalAlertInForwardChain(void) {
+int BandwidthController::removeGlobalAlertInForwardChain() {
     int res = 0;
     const char *alertName = ALERT_GLOBAL_NAME;
 
@@ -853,15 +810,15 @@
     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 @@
     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 @@
     }
 
     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 @@
         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 @@
     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 @@
 
     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 @@
     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 @@
     }
 
     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);