Turn on -Wformat-nonliteral.

Apparently there are two classes of this warning in clang.
-Wformat-security is only emitted for cases of
`func(nonliteral_fmt_string)` (no args), and -Wformat-nonliteral is
emitted for cases *with* arguments. For whatever reason, the latter
isn't included in -Wextra and must be manually enabled.

To make this more easily portable to Windows, move the existing
gnu_printf/__printf__ decision into base/macros.h as ATTRIBUTE_FORMAT.

Change-Id: I3b0990e1d1f0a2e9c13b32f5cd60478946cb5fc6

adb/Android.mk

@@ -15,8 +15,9 @@ adb_version := $(shell git -C $(LOCAL_PATH) rev-parse --short=12 HEAD 2>/dev/nul
 
 ADB_COMMON_CFLAGS := \
     -Wall -Wextra -Werror \
-    -Wno-unused-parameter \
+    -Wformat-nonliteral \
     -Wno-missing-field-initializers \
+    -Wno-unused-parameter \
     -DADB_REVISION='"$(adb_version)"' \
 
 # libadb

adb/adb.h

@@ -277,8 +277,8 @@ asocket *create_remote_socket(unsigned id, atransport *t);
 void connect_to_remote(asocket *s, const char *destination);
 void connect_to_smartsocket(asocket *s);
 
-void fatal(const char *fmt, ...);
-void fatal_errno(const char *fmt, ...);
+void fatal(const char *fmt, ...) ATTRIBUTE_FORMAT(1, 2);
+void fatal_errno(const char *fmt, ...) ATTRIBUTE_FORMAT(1, 2);
 
 void handle_packet(apacket *p, atransport *t);
 

adb/adb_auth_host.cpp

@@ -43,6 +43,7 @@
 #include "mincrypt/rsa.h"
 #undef RSA_verify
 
+#include <base/logging.h>
 #include <base/strings.h>
 #include <cutils/list.h>
 
@@ -56,8 +57,10 @@
 #include <openssl/base64.h>
 #endif
 
-#define ANDROID_PATH   ".android"
-#define ADB_KEY_FILE   "adbkey"
+#include "adb_utils.h"
+
+const char kAndroidPath[] = ".android";
+const char kAdbKeyFile[] = "adbkey";
 
 struct adb_private_key {
     struct listnode node;
@@ -295,64 +298,58 @@ static int read_key(const char *file, struct listnode *list)
     return 1;
 }
 
-static int get_user_keyfilepath(char *filename, size_t len)
-{
-    const char *format, *home;
-    char android_dir[PATH_MAX];
-    struct stat buf;
+static bool get_user_keyfilepath(std::string* filename) {
+    CHECK(filename != nullptr);
+
 #ifdef _WIN32
-    char path[PATH_MAX];
-    home = getenv("ANDROID_SDK_HOME");
-    if (!home) {
+    const char* home = getenv("ANDROID_SDK_HOME");
+    if (home == nullptr) {
+        char path[PATH_MAX];
         SHGetFolderPath(NULL, CSIDL_PROFILE, NULL, 0, path);
         home = path;
     }
-    format = "%s\\%s";
 #else
-    home = getenv("HOME");
-    if (!home)
-        return -1;
-    format = "%s/%s";
+    const char* home = getenv("HOME");
+    if (home == nullptr)
+        return false;
 #endif
 
     D("home '%s'\n", home);
 
-    if (snprintf(android_dir, sizeof(android_dir), format, home,
-                        ANDROID_PATH) >= (int)sizeof(android_dir))
-        return -1;
+    const std::string android_dir = android::base::Join(
+        std::vector<std::string>({home, kAndroidPath}), OS_PATH_SEPARATOR);
 
-    if (stat(android_dir, &buf)) {
-        if (adb_mkdir(android_dir, 0750) < 0) {
-            D("Cannot mkdir '%s'", android_dir);
-            return -1;
+    if (!directory_exists(android_dir)) {
+        if (adb_mkdir(android_dir.c_str(), 0750) == -1) {
+            D("Cannot mkdir '%s'", android_dir.c_str());
+            return false;
         }
     }
 
-    return snprintf(filename, len, format, android_dir, ADB_KEY_FILE);
+    *filename = android::base::Join(
+        std::vector<std::string>({android_dir, kAdbKeyFile}),
+        OS_PATH_SEPARATOR);
+    return true;
 }
 
 static int get_user_key(struct listnode *list)
 {
-    struct stat buf;
-    char path[PATH_MAX];
-    int ret;
-
-    ret = get_user_keyfilepath(path, sizeof(path));
-    if (ret < 0 || ret >= (signed)sizeof(path)) {
+    std::string path;
+    if (!get_user_keyfilepath(&path)) {
         D("Error getting user key filename");
         return 0;
     }
 
-    D("user key '%s'\n", path);
+    D("user key '%s'\n", path.c_str());
 
-    if (stat(path, &buf) == -1) {
-        if (!generate_key(path)) {
+    if (!file_exists(path)) {
+        if (!generate_key(path.c_str())) {
             D("Failed to generate new key\n");
             return 0;
         }
     }
 
-    return read_key(path, list);
+    return read_key(path.c_str(), list);
 }
 
 static void get_vendor_keys(struct listnode* key_list) {
@@ -411,27 +408,26 @@ void *adb_auth_nextkey(void *current)
 
 int adb_auth_get_userkey(unsigned char *data, size_t len)
 {
-    char path[PATH_MAX];
-    int ret = get_user_keyfilepath(path, sizeof(path) - 4);
-    if (ret < 0 || ret >= (signed)(sizeof(path) - 4)) {
+    std::string path;
+    if (!get_user_keyfilepath(&path)) {
         D("Error getting user key filename");
         return 0;
     }
-    strcat(path, ".pub");
+    path += ".pub";
 
     // TODO(danalbert): ReadFileToString
     // Note that on Windows, load_file() does not do CR/LF translation, but
     // ReadFileToString() uses the C Runtime which uses CR/LF translation by
     // default (by is overridable with _setmode()).
     unsigned size;
-    char* file_data = reinterpret_cast<char*>(load_file(path, &size));
+    void* file_data = load_file(path.c_str(), &size);
     if (file_data == nullptr) {
-        D("Can't load '%s'\n", path);
+        D("Can't load '%s'\n", path.c_str());
         return 0;
     }
 
     if (len < (size_t)(size + 1)) {
-        D("%s: Content too large ret=%d\n", path, size);
+        D("%s: Content too large ret=%d\n", path.c_str(), size);
         free(file_data);
         return 0;
     }

adb/adb_io.h

@@ -21,6 +21,8 @@
 
 #include <string>
 
+#include "base/macros.h"
+
 // Sends the protocol "OKAY" message.
 bool SendOkay(int fd);
 
@@ -54,6 +56,6 @@ bool WriteFdExactly(int fd, const char* s);
 bool WriteFdExactly(int fd, const std::string& s);
 
 // Same as above, but formats the string to send.
-bool WriteFdFmt(int fd, const char* fmt, ...) __attribute__((__format__(__printf__, 2, 3)));
+bool WriteFdFmt(int fd, const char* fmt, ...) ATTRIBUTE_FORMAT(2, 3);
 
 #endif /* ADB_IO_H */

adb/adb_utils.cpp

@@ -42,6 +42,11 @@ bool directory_exists(const std::string& path) {
   return lstat(path.c_str(), &sb) != -1 && S_ISDIR(sb.st_mode);
 }
 
+bool file_exists(const std::string& path) {
+  struct stat sb;
+  return lstat(path.c_str(), &sb) != -1 && S_ISREG(sb.st_mode);
+}
+
 std::string escape_arg(const std::string& s) {
   std::string result = s;
 

adb/adb_utils.h

@@ -21,6 +21,7 @@
 
 bool getcwd(std::string* cwd);
 bool directory_exists(const std::string& path);
+bool file_exists(const std::string& path);
 
 std::string escape_arg(const std::string& s);
 

adb/commandline.cpp

@@ -1447,7 +1447,8 @@ static int uninstall_app(TransportType transport, const char* serial, int argc,
     return pm_command(transport, serial, argc, argv);
 }
 
-static int delete_file(TransportType transport, const char* serial, char* filename) {
+static int delete_file(TransportType transport, const char* serial,
+                       const char* filename) {
     std::string cmd = "shell:rm -f " + escape_arg(filename);
     return send_shell_command(transport, serial, cmd);
 }
@@ -1464,8 +1465,8 @@ static const char* get_basename(const char* filename)
 }
 
 static int install_app(TransportType transport, const char* serial, int argc, const char** argv) {
-    static const char *const DATA_DEST = "/data/local/tmp/%s";
-    static const char *const SD_DEST = "/sdcard/tmp/%s";
+    static const char *const DATA_DEST = "/data/local/tmp";
+    static const char *const SD_DEST = "/sdcard/tmp";
     const char* where = DATA_DEST;
     int i;
     struct stat sb;
@@ -1499,19 +1500,20 @@ static int install_app(TransportType transport, const char* serial, int argc, co
     }
 
     const char* apk_file = argv[last_apk];
-    char apk_dest[PATH_MAX];
-    snprintf(apk_dest, sizeof apk_dest, where, get_basename(apk_file));
-    int err = do_sync_push(apk_file, apk_dest, 0 /* no show progress */);
+    const std::string apk_dest =
+        android::base::StringPrintf("%s/%s", where, get_basename(apk_file));
+    int err = do_sync_push(apk_file, apk_dest.c_str(), /* show_progress = */ 0);
     if (err) {
         goto cleanup_apk;
     } else {
-        argv[last_apk] = apk_dest; /* destination name, not source location */
+        // Destination name, not source location.
+        argv[last_apk] = apk_dest.c_str();
     }
 
     err = pm_command(transport, serial, argc, argv);
 
 cleanup_apk:
-    delete_file(transport, serial, apk_dest);
+    delete_file(transport, serial, apk_dest.c_str());
     return err;
 }
 

adb/fdevent.cpp

@@ -30,6 +30,8 @@
 #include <sys/ioctl.h>
 #include <unistd.h>
 
+#include "base/macros.h"
+
 #include "adb_io.h"
 #include "adb_trace.h"
 
@@ -44,6 +46,7 @@
 // of the shell's pseudo-tty master. I.e. force close it.
 int SHELL_EXIT_NOTIFY_FD = -1;
 
+static void fatal(const char *fn, const char *fmt, ...) ATTRIBUTE_FORMAT(2, 3);
 static void fatal(const char *fn, const char *fmt, ...)
 {
     va_list ap;

adb/qemu_tracing.h

@@ -21,8 +21,10 @@
 #ifndef __QEMU_TRACING_H
 #define __QEMU_TRACING_H
 
+#include "base/macros.h"
+
 /* Initializes connection with the adb-debug qemud service in the emulator. */
 int adb_qemu_trace_init(void);
-void adb_qemu_trace(const char* fmt, ...);
+void adb_qemu_trace(const char* fmt, ...) ATTRIBUTE_FORMAT(1, 2);
 
 #endif /* __QEMU_TRACING_H */

base/include/base/macros.h

@@ -185,4 +185,17 @@ void UNUSED(const T&...) {
   } while (0)
 #endif
 
+// These printf-like functions are implemented in terms of vsnprintf, so they
+// use the same attribute for compile-time format string checking. On Windows,
+// if the mingw version of vsnprintf is used, so use `gnu_printf' which allows z
+// in %zd and PRIu64 (and related) to be recognized by the compile-time
+// checking.
+#if defined(__USE_MINGW_ANSI_STDIO) && __USE_MINGW_ANSI_STDIO
+#define ATTRIBUTE_FORMAT(fmt, args) \
+  __attribute__((format(gnu_printf, fmt, args)))
+#else
+#define ATTRIBUTE_FORMAT(fmt, args) \
+  __attribute__((format(__printf__, fmt, args)))
+#endif
+
 #endif  // UTILS_MACROS_H

base/include/base/stringprintf.h

@@ -20,35 +20,21 @@
 #include <stdarg.h>
 #include <string>
 
+#include "base/macros.h"
+
 namespace android {
 namespace base {
 
-// These printf-like functions are implemented in terms of vsnprintf, so they
-// use the same attribute for compile-time format string checking. On Windows,
-// if the mingw version of vsnprintf is used, use `gnu_printf' which allows z
-// in %zd and PRIu64 (and related) to be recognized by the compile-time
-// checking.
-#define FORMAT_ARCHETYPE __printf__
-#ifdef __USE_MINGW_ANSI_STDIO
-#if __USE_MINGW_ANSI_STDIO
-#undef FORMAT_ARCHETYPE
-#define FORMAT_ARCHETYPE gnu_printf
-#endif
-#endif
-
 // Returns a string corresponding to printf-like formatting of the arguments.
-std::string StringPrintf(const char* fmt, ...)
-    __attribute__((__format__(FORMAT_ARCHETYPE, 1, 2)));
+std::string StringPrintf(const char* fmt, ...) ATTRIBUTE_FORMAT(1, 2);
 
 // Appends a printf-like formatting of the arguments to 'dst'.
 void StringAppendF(std::string* dst, const char* fmt, ...)
-    __attribute__((__format__(FORMAT_ARCHETYPE, 2, 3)));
+    ATTRIBUTE_FORMAT(2, 3);
 
 // Appends a printf-like formatting of the arguments to 'dst'.
 void StringAppendV(std::string* dst, const char* format, va_list ap)
-    __attribute__((__format__(FORMAT_ARCHETYPE, 2, 0)));
-
-#undef FORMAT_ARCHETYPE
+    ATTRIBUTE_FORMAT(2, 0);
 
 }  // namespace base
 }  // namespace android