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
diff --git a/adb/Android.mk b/adb/Android.mk
index 7977009..0124b26 100644
--- a/adb/Android.mk
+++ b/adb/Android.mk
@@ -15,8 +15,9 @@
ADB_COMMON_CFLAGS := \
-Wall -Wextra -Werror \
- -Wno-unused-parameter \
+ -Wformat-nonliteral \
-Wno-missing-field-initializers \
+ -Wno-unused-parameter \
-DADB_REVISION='"$(adb_version)"' \
# libadb
diff --git a/adb/adb.h b/adb/adb.h
index 1be83d7..5441d79 100644
--- a/adb/adb.h
+++ b/adb/adb.h
@@ -277,8 +277,8 @@
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);
diff --git a/adb/adb_auth_host.cpp b/adb/adb_auth_host.cpp
index e878f8b..749c7c5 100644
--- a/adb/adb_auth_host.cpp
+++ b/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 @@
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 @@
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;
}
diff --git a/adb/adb_io.h b/adb/adb_io.h
index 8d50a6d..bd3b869 100644
--- a/adb/adb_io.h
+++ b/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 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 */
diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp
index 604bd57..1028838 100644
--- a/adb/adb_utils.cpp
+++ b/adb/adb_utils.cpp
@@ -42,6 +42,11 @@
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;
diff --git a/adb/adb_utils.h b/adb/adb_utils.h
index 84f7d0c..f4ddb6c 100644
--- a/adb/adb_utils.h
+++ b/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);
diff --git a/adb/commandline.cpp b/adb/commandline.cpp
index 4adac37..59e0807 100644
--- a/adb/commandline.cpp
+++ b/adb/commandline.cpp
@@ -1447,7 +1447,8 @@
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 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 @@
}
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;
}
diff --git a/adb/fdevent.cpp b/adb/fdevent.cpp
index 0c43c5e..8997897 100644
--- a/adb/fdevent.cpp
+++ b/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;
diff --git a/adb/qemu_tracing.h b/adb/qemu_tracing.h
index ff42d4f..aefba57 100644
--- a/adb/qemu_tracing.h
+++ b/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 */
diff --git a/base/include/base/macros.h b/base/include/base/macros.h
index b1ce7c6..35db83a 100644
--- a/base/include/base/macros.h
+++ b/base/include/base/macros.h
@@ -185,4 +185,17 @@
} 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
diff --git a/base/include/base/stringprintf.h b/base/include/base/stringprintf.h
index d68af87..e17a2b5 100644
--- a/base/include/base/stringprintf.h
+++ b/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