From 6d92997ec4285caebe94064158a8cc017bbf5caa Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 27 Oct 2015 13:40:35 -0700 Subject: [PATCH] Don't use VLAs in adb. This makes no measurable difference to the sync time; "adb sync" of everything on /system for a Nexus 9 still takes 20s. Change-Id: Ifa2626f7453937e43856b9c4ee06e1f5db0aa273 --- adb/Android.mk | 1 + adb/file_sync_client.cpp | 23 ++++++++++++----------- adb/sysdeps_win32.cpp | 5 ++--- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/adb/Android.mk b/adb/Android.mk index 2538e2e3f..c507905db 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -14,6 +14,7 @@ ADB_COMMON_CFLAGS := \ -Wall -Wextra -Werror \ -Wno-unused-parameter \ -Wno-missing-field-initializers \ + -Wvla \ -DADB_REVISION='"$(adb_version)"' \ # Define windows.h and tchar.h Unicode preprocessor symbols so that diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp index 533994766..1d668bc18 100644 --- a/adb/file_sync_client.cpp +++ b/adb/file_sync_client.cpp @@ -29,6 +29,7 @@ #include #include +#include #include "sysdeps.h" @@ -101,14 +102,14 @@ class SyncConnection { // Sending header and payload in a single write makes a noticeable // difference to "adb sync" performance. - char buf[sizeof(SyncRequest) + path_length]; - SyncRequest* req = reinterpret_cast(buf); + std::vector buf(sizeof(SyncRequest) + path_length); + SyncRequest* req = reinterpret_cast(&buf[0]); req->id = id; req->path_length = path_length; char* data = reinterpret_cast(req + 1); memcpy(data, path_and_mode, path_length); - return WriteFdExactly(fd, buf, sizeof(buf)); + return WriteFdExactly(fd, &buf[0], buf.size()); } // Sending header, payload, and footer in a single write makes a huge @@ -123,10 +124,10 @@ class SyncConnection { return false; } - char buf[sizeof(SyncRequest) + path_length + + std::vector buf(sizeof(SyncRequest) + path_length + sizeof(SyncRequest) + data_length + - sizeof(SyncRequest)]; - char* p = buf; + sizeof(SyncRequest)); + char* p = &buf[0]; SyncRequest* req_send = reinterpret_cast(p); req_send->id = ID_SEND; @@ -147,7 +148,7 @@ class SyncConnection { req_done->path_length = mtime; p += sizeof(SyncRequest); - if (!WriteFdExactly(fd, buf, (p-buf))) return false; + if (!WriteFdExactly(fd, &buf[0], (p - &buf[0]))) return false; total_bytes += data_length; return true; @@ -172,14 +173,14 @@ class SyncConnection { } bool ReportCopyFailure(const char* from, const char* to, const syncmsg& msg) { - char buffer[msg.status.msglen + 1]; - if (!ReadFdExactly(fd, buffer, msg.status.msglen)) { + std::vector buf(msg.status.msglen + 1); + if (!ReadFdExactly(fd, &buf[0], msg.status.msglen)) { fprintf(stderr, "adb: failed to copy '%s' to '%s'; failed to read reason (!): %s\n", from, to, strerror(errno)); return false; } - buffer[msg.status.msglen] = 0; - fprintf(stderr, "adb: failed to copy '%s' to '%s': %s\n", from, to, buffer); + buf[msg.status.msglen] = 0; + fprintf(stderr, "adb: failed to copy '%s' to '%s': %s\n", from, to, &buf[0]); return false; } diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index fd753200f..14d137585 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -567,12 +567,11 @@ char* adb_strerror(int err) { // Lookup the string for an unknown error. char* errmsg = strerror(-1); - char unknown_error[(errmsg == nullptr ? 0 : strlen(errmsg)) + 1]; - strcpy(unknown_error, errmsg == nullptr ? "" : errmsg); + const std::string unknown_error = (errmsg == nullptr) ? "" : errmsg; // Lookup the string for this error to see if the C Runtime has it. errmsg = strerror(err); - if ((errmsg != nullptr) && strcmp(errmsg, unknown_error)) { + if (errmsg != nullptr && unknown_error != errmsg) { // The CRT returned an error message and it is different than the error // message for an unknown error, so it is probably valid, so use it. } else {