Merge "Remove strtok from adb."

adb/adb.cpp

@@ -33,6 +33,7 @@
 #include <string>
 
 #include <base/stringprintf.h>
+#include <base/strings.h>
 
 #include "adb_auth.h"
 #include "adb_io.h"
@@ -381,84 +382,60 @@ static const char* connection_state_name(atransport *t)
 }
 #endif // ADB_HOST
 
-/* qual_overwrite is used to overwrite a qualifier string.  dst is a
+// qual_overwrite is used to overwrite a qualifier string.  dst is a
- * pointer to a char pointer.  It is assumed that if *dst is non-NULL, it
+// pointer to a char pointer.  It is assumed that if *dst is non-NULL, it
- * was malloc'ed and needs to freed.  *dst will be set to a dup of src.
+// was malloc'ed and needs to freed.  *dst will be set to a dup of src.
- */
+// TODO: switch to std::string for these atransport fields instead.
-static void qual_overwrite(char **dst, const char *src)
+static void qual_overwrite(char** dst, const std::string& src) {
-{
-    if (!dst)
-        return;
-
     free(*dst);
-    *dst = NULL;
+    *dst = strdup(src.c_str());
-
-    if (!src || !*src)
-        return;
-
-    *dst = strdup(src);
 }
 
-void parse_banner(char *banner, atransport *t)
+void parse_banner(const char* banner, atransport* t) {
-{
-    static const char *prop_seps = ";";
-    static const char key_val_sep = '=';
-    char *cp;
-    char *type;
-
     D("parse_banner: %s\n", banner);
-    type = banner;
+
-    cp = strchr(type, ':');
+    // The format is something like:
-    if (cp) {
+    // "device::ro.product.name=x;ro.product.model=y;ro.product.device=z;".
-        *cp++ = 0;
+    std::vector<std::string> pieces = android::base::Split(banner, ":");
-        /* Nothing is done with second field. */
+
-        cp = strchr(cp, ':');
+    if (pieces.size() > 2) {
-        if (cp) {
+        const std::string& props = pieces[2];
-            char *save;
+        for (auto& prop : android::base::Split(props, ";")) {
-            char *key;
+            // The list of properties was traditionally ;-terminated rather than ;-separated.
-            key = adb_strtok_r(cp + 1, prop_seps, &save);
+            if (prop.empty()) continue;
-            while (key) {
+
-                cp = strchr(key, key_val_sep);
+            std::vector<std::string> key_value = android::base::Split(prop, "=");
-                if (cp) {
+            if (key_value.size() != 2) continue;
-                    *cp++ = '\0';
+
-                    if (!strcmp(key, "ro.product.name"))
+            const std::string& key = key_value[0];
-                        qual_overwrite(&t->product, cp);
+            const std::string& value = key_value[1];
-                    else if (!strcmp(key, "ro.product.model"))
+            if (key == "ro.product.name") {
-                        qual_overwrite(&t->model, cp);
+                qual_overwrite(&t->product, value);
-                    else if (!strcmp(key, "ro.product.device"))
+            } else if (key == "ro.product.model") {
-                        qual_overwrite(&t->device, cp);
+                qual_overwrite(&t->model, value);
-                }
+            } else if (key == "ro.product.device") {
-                key = adb_strtok_r(NULL, prop_seps, &save);
+                qual_overwrite(&t->device, value);
             }
         }
     }
 
-    if(!strcmp(type, "bootloader")){
+    const std::string& type = pieces[0];
+    if (type == "bootloader") {
         D("setting connection_state to CS_BOOTLOADER\n");
         t->connection_state = CS_BOOTLOADER;
         update_transports();
-        return;
+    } else if (type == "device") {
-    }
-
-    if(!strcmp(type, "device")) {
         D("setting connection_state to CS_DEVICE\n");
         t->connection_state = CS_DEVICE;
         update_transports();
-        return;
+    } else if (type == "recovery") {
-    }
-
-    if(!strcmp(type, "recovery")) {
         D("setting connection_state to CS_RECOVERY\n");
         t->connection_state = CS_RECOVERY;
         update_transports();
-        return;
+    } else if (type == "sideload") {
-    }
-
-    if(!strcmp(type, "sideload")) {
         D("setting connection_state to CS_SIDELOAD\n");
         t->connection_state = CS_SIDELOAD;
         update_transports();
-        return;
     }
 
     t->connection_state = CS_HOST;
@@ -493,7 +470,7 @@ void handle_packet(apacket *p, atransport *t)
             handle_offline(t);
         }
 
-        parse_banner((char*) p->data, t);
+        parse_banner(reinterpret_cast<const char*>(p->data), t);
 
         if (HOST || !auth_enabled) {
             handle_online(t);

adb/adb_auth_host.cpp

@@ -43,6 +43,7 @@
 #include "mincrypt/rsa.h"
 #undef RSA_verify
 
+#include <base/strings.h>
 #include <cutils/list.h>
 
 #include <openssl/evp.h>
@@ -172,7 +173,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path)
         return 0;
     }
 
-    outfile = fopen(path, "w");
+    outfile = fopen(path, "we");
     if (!outfile) {
         D("Failed to open '%s'\n", path);
         return 0;
@@ -191,7 +192,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path)
     encoded_length = 1 + ((sizeof(pkey) + 2) / 3 * 4);
 #endif
 
-    encoded = reinterpret_cast<uint8_t*>(malloc(encoded_length));
+    encoded = new uint8_t[encoded_length];
     if (encoded == nullptr) {
         D("Allocation failure");
         goto out;
@@ -212,9 +213,7 @@ static int write_public_keyfile(RSA *private_key, const char *private_key_path)
     if (outfile != NULL) {
         fclose(outfile);
     }
-    if (encoded != NULL) {
+    delete[] encoded;
-        free(encoded);
-    }
     return ret;
 }
 
@@ -240,7 +239,7 @@ static int generate_key(const char *file)
 
     old_mask = umask(077);
 
-    f = fopen(file, "w");
+    f = fopen(file, "we");
     if (!f) {
         D("Failed to open '%s'\n", file);
         umask(old_mask);
@@ -274,30 +273,24 @@ static int read_key(const char *file, struct listnode *list)
 {
     D("read_key '%s'\n", file);
 
-    FILE* f = fopen(file, "r");
+    FILE* fp = fopen(file, "re");
-    if (!f) {
+    if (!fp) {
-        D("Failed to open '%s'\n", file);
+        D("Failed to open '%s': %s\n", file, strerror(errno));
         return 0;
     }
 
-    adb_private_key* key = reinterpret_cast<adb_private_key*>(
+    adb_private_key* key = new adb_private_key;
-        malloc(sizeof(adb_private_key)));
-    if (!key) {
-        D("Failed to alloc key\n");
-        fclose(f);
-        return 0;
-    }
     key->rsa = RSA_new();
 
-    if (!PEM_read_RSAPrivateKey(f, &key->rsa, NULL, NULL)) {
+    if (!PEM_read_RSAPrivateKey(fp, &key->rsa, NULL, NULL)) {
         D("Failed to read key\n");
-        fclose(f);
+        fclose(fp);
         RSA_free(key->rsa);
-        free(key);
+        delete key;
         return 0;
     }
 
-    fclose(f);
+    fclose(fp);
     list_add_tail(list, &key->node);
     return 1;
 }
@@ -362,29 +355,16 @@ static int get_user_key(struct listnode *list)
     return read_key(path, list);
 }
 
-static void get_vendor_keys(struct listnode *list)
+static void get_vendor_keys(struct listnode* key_list) {
-{
+    const char* adb_keys_path = getenv("ADB_VENDOR_KEYS");
-    const char *adb_keys_path;
+    if (adb_keys_path == nullptr) {
-    char keys_path[MAX_PAYLOAD];
-    char *path;
-    char *save;
-    struct stat buf;
-
-    adb_keys_path = getenv("ADB_VENDOR_KEYS");
-    if (!adb_keys_path)
         return;
-    strncpy(keys_path, adb_keys_path, sizeof(keys_path));
+    }
 
-    path = adb_strtok_r(keys_path, ENV_PATH_SEPARATOR_STR, &save);
+    for (auto& path : android::base::Split(adb_keys_path, ENV_PATH_SEPARATOR_STR)) {
-    while (path) {
+        if (!read_key(path.c_str(), key_list)) {
-        D("Reading: '%s'\n", path);
+            D("Failed to read '%s'\n", path.c_str());
-
+        }
-        if (stat(path, &buf))
-            D("Can't read '%s'\n", path);
-        else if (!read_key(path, list))
-            D("Failed to read '%s'\n", path);
-
-        path = adb_strtok_r(NULL, ENV_PATH_SEPARATOR_STR, &save);
     }
 }
 

adb/sysdeps.h

@@ -271,8 +271,6 @@ static __inline__  int  adb_is_absolute_host_path( const char*  path )
     return isalpha(path[0]) && path[1] == ':' && path[2] == '\\';
 }
 
-extern char*  adb_strtok_r(char *str, const char *delim, char **saveptr);
-
 #else /* !_WIN32 a.k.a. Unix */
 
 #include "fdevent.h"
@@ -517,19 +515,11 @@ static __inline__  int  adb_is_absolute_host_path( const char*  path )
     return path[0] == '/';
 }
 
-static __inline__ char*  adb_strtok_r(char *str, const char *delim, char **saveptr)
-{
-    return strtok_r(str, delim, saveptr);
-}
-
 static __inline__ unsigned long adb_thread_id()
 {
     return (unsigned long)pthread_self();
 }
 
-#undef   strtok_r
-#define  strtok_r  ___xxx_strtok_r
-
 #endif /* !_WIN32 */
 
 #endif /* _ADB_SYSDEPS_H */

adb/sysdeps_win32.cpp

@@ -2151,85 +2151,6 @@ adb_sysdeps_init( void )
     InitializeCriticalSection( &_win32_lock );
 }
 
-/* Windows doesn't have strtok_r.  Use the one from bionic. */
-
-/*
- * Copyright (c) 1988 Regents of the University of California.
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * 1. Redistributions of source code must retain the above copyright
- *    notice, this list of conditions and the following disclaimer.
- * 2. Redistributions in binary form must reproduce the above copyright
- *    notice, this list of conditions and the following disclaimer in the
- *    documentation and/or other materials provided with the distribution.
- * 3. Neither the name of the University nor the names of its contributors
- *    may be used to endorse or promote products derived from this software
- *    without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
- * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED.  IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
- * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS
- * OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
- * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
- * LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY
- * OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-char *
-adb_strtok_r(char *s, const char *delim, char **last)
-{
-	char *spanp;
-	int c, sc;
-	char *tok;
-
-
-	if (s == NULL && (s = *last) == NULL)
-		return (NULL);
-
-	/*
-	 * Skip (span) leading delimiters (s += strspn(s, delim), sort of).
-	 */
-cont:
-	c = *s++;
-	for (spanp = (char *)delim; (sc = *spanp++) != 0;) {
-		if (c == sc)
-			goto cont;
-	}
-
-	if (c == 0) {		/* no non-delimiter characters */
-		*last = NULL;
-		return (NULL);
-	}
-	tok = s - 1;
-
-	/*
-	 * Scan token (scan for delimiters: s += strcspn(s, delim), sort of).
-	 * Note that delim must have one NUL; we stop if we see that, too.
-	 */
-	for (;;) {
-		c = *s++;
-		spanp = (char *)delim;
-		do {
-			if ((sc = *spanp++) == c) {
-				if (c == 0)
-					s = NULL;
-				else
-					s[-1] = 0;
-				*last = s;
-				return (tok);
-			}
-		} while (sc != 0);
-	}
-	/* NOTREACHED */
-}
-
 /**************************************************************************/
 /**************************************************************************/
 /*****                                                                *****/

base/include/base/strings.h

@@ -27,8 +27,6 @@ namespace base {
 //
 // The string is split at each occurrence of a character in delimiters.
 //
-// Empty splits will be omitted. I.e. Split("a,,b", ",") -> {"a", "b"}
-//
 // The empty string is not a valid delimiter list.
 std::vector<std::string> Split(const std::string& s,
                                const std::string& delimiters);

base/strings.cpp

@@ -32,24 +32,17 @@ std::vector<std::string> Split(const std::string& s,
                                const std::string& delimiters) {
   CHECK_NE(delimiters.size(), 0U);
 
-  std::vector<std::string> split;
+  std::vector<std::string> result;
-  if (s.size() == 0) {
-    // Split("", d) returns {} rather than {""}.
-    return split;
-  }
 
   size_t base = 0;
   size_t found;
   do {
     found = s.find_first_of(delimiters, base);
-    if (found != base) {
+    result.push_back(s.substr(base, found - base));
-      split.push_back(s.substr(base, found - base));
-    }
-
     base = found + 1;
   } while (found != s.npos);
 
-  return split;
+  return result;
 }
 
 std::string Trim(const std::string& s) {

base/strings_test.cpp

@@ -23,7 +23,8 @@
 
 TEST(strings, split_empty) {
   std::vector<std::string> parts = android::base::Split("", ",");
-  ASSERT_EQ(0U, parts.size());
+  ASSERT_EQ(1U, parts.size());
+  ASSERT_EQ("", parts[0]);
 }
 
 TEST(strings, split_single) {
@@ -42,9 +43,10 @@ TEST(strings, split_simple) {
 
 TEST(strings, split_with_empty_part) {
   std::vector<std::string> parts = android::base::Split("foo,,bar", ",");
-  ASSERT_EQ(2U, parts.size());
+  ASSERT_EQ(3U, parts.size());
   ASSERT_EQ("foo", parts[0]);
-  ASSERT_EQ("bar", parts[1]);
+  ASSERT_EQ("", parts[1]);
+  ASSERT_EQ("bar", parts[2]);
 }
 
 TEST(strings, split_null_char) {
@@ -65,9 +67,10 @@ TEST(strings, split_any) {
 
 TEST(strings, split_any_with_empty_part) {
   std::vector<std::string> parts = android::base::Split("foo:,bar", ",:");
-  ASSERT_EQ(2U, parts.size());
+  ASSERT_EQ(3U, parts.size());
   ASSERT_EQ("foo", parts[0]);
-  ASSERT_EQ("bar", parts[1]);
+  ASSERT_EQ("", parts[1]);
+  ASSERT_EQ("bar", parts[2]);
 }
 
 TEST(strings, trim_empty) {