Address a bunch of clang-tidy complaints.

There were a bunch more unreasonable/incorrect ones, but these ones
seemed legit. Nothing very interesting, though.

Bug: N/A
Test: ran tests, benchmarks
Change-Id: If66971194d4a7b4bf6d0251bedb88e8cdc88a76f

benchmarks/bionic_benchmarks.cpp

@@ -305,7 +305,7 @@ args_vector_t* ResolveArgs(args_vector_t* to_populate, std::string args,
 }
 
 void RegisterGoogleBenchmarks(bench_opts_t primary_opts, bench_opts_t secondary_opts,
-                         std::string fn_name, args_vector_t* run_args) {
+                              const std::string& fn_name, args_vector_t* run_args) {
   if (g_str_to_func.find(fn_name) == g_str_to_func.end()) {
     errx(1, "ERROR: No benchmark for function %s", fn_name.c_str());
   }
@@ -320,7 +320,7 @@ void RegisterGoogleBenchmarks(bench_opts_t primary_opts, bench_opts_t secondary_
   }
 
   benchmark_func_t benchmark_function = g_str_to_func.at(fn_name).first;
-  for (std::vector<int> args : (*run_args)) {
+  for (const std::vector<int>& args : (*run_args)) {
     auto registration = benchmark::RegisterBenchmark(fn_name.c_str(), LockAndRun,
                                                      benchmark_function,
                                                      cpu_to_use)->Args(args);
@@ -335,13 +335,13 @@ void RegisterCliBenchmarks(bench_opts_t cmdline_opts,
   // Register any of the extra benchmarks that were specified in the options.
   args_vector_t arg_vector;
   args_vector_t* run_args = &arg_vector;
-  for (std::string extra_fn : cmdline_opts.extra_benchmarks) {
+  for (const std::string& extra_fn : cmdline_opts.extra_benchmarks) {
     android::base::Trim(extra_fn);
-    size_t first_space_pos = extra_fn.find(" ");
+    size_t first_space_pos = extra_fn.find(' ');
     std::string fn_name = extra_fn.substr(0, first_space_pos);
     std::string cmd_args;
     if (first_space_pos != std::string::npos) {
-      cmd_args = extra_fn.substr(extra_fn.find(" ") + 1);
+      cmd_args = extra_fn.substr(extra_fn.find(' ') + 1);
     } else {
       cmd_args = "";
     }

benchmarks/stdio_benchmark.cpp

@@ -29,7 +29,7 @@ static void FillFile(TemporaryFile& tf) {
   memset(line, 'x', sizeof(line));
   line[sizeof(line) - 1] = '\0';
 
-  FILE* fp = fopen(tf.path, "w");
+  FILE* fp = fopen(tf.path, "we");
   for (size_t i = 0; i < 4096; ++i) fputs(line, fp);
   fclose(fp);
 }

benchmarks/util.h

@@ -29,7 +29,7 @@ extern std::mutex g_map_lock;
 
 extern std::map<std::string, std::pair<benchmark_func_t, std::string>> g_str_to_func;
 
-static int  __attribute__((unused)) EmplaceBenchmark (std::string fn_name, benchmark_func_t fn_ptr, std::string arg = "") {
+static int  __attribute__((unused)) EmplaceBenchmark(const std::string& fn_name, benchmark_func_t fn_ptr, const std::string& arg = "") {
   g_map_lock.lock();
   g_str_to_func.emplace(std::string(fn_name), std::make_pair(fn_ptr, arg));
   g_map_lock.unlock();

libc/bionic/icu.cpp

@@ -68,7 +68,7 @@ static bool __find_icu() {
   }
   free(namelist);
 
-  if (max_version == -1 || max_version < ICUDATA_VERSION_MIN) {
+  if (max_version < ICUDATA_VERSION_MIN) {
     async_safe_write_log(ANDROID_LOG_ERROR, "bionic-icu", "couldn't find an ICU .dat file");
     return false;
   }

libc/malloc_debug/MapData.h

@@ -41,7 +41,7 @@ struct MapEntry {
   MapEntry(uintptr_t start, uintptr_t end, uintptr_t offset, const char* name, size_t name_len)
       : start(start), end(end), offset(offset), name(name, name_len) {}
 
-  MapEntry(uintptr_t pc) : start(pc), end(pc) {}
+  explicit MapEntry(uintptr_t pc) : start(pc), end(pc) {}
 
   uintptr_t start;
   uintptr_t end;

libc/malloc_debug/OptionData.h

@@ -34,7 +34,7 @@ class DebugData;
 
 class OptionData {
  public:
-  OptionData(DebugData* debug) : debug_(debug) {}
+  explicit OptionData(DebugData* debug) : debug_(debug) {}
   ~OptionData() = default;
 
  protected:

libc/malloc_debug/RecordData.h

@@ -66,7 +66,7 @@ class ThreadCompleteEntry : public RecordEntry {
 
 class AllocEntry : public RecordEntry {
  public:
-  AllocEntry(void* pointer);
+  explicit AllocEntry(void* pointer);
   virtual ~AllocEntry() = default;
 
  protected:
@@ -92,7 +92,7 @@ class MallocEntry : public AllocEntry {
 
 class FreeEntry : public AllocEntry {
  public:
-  FreeEntry(void* pointer);
+  explicit FreeEntry(void* pointer);
   virtual ~FreeEntry() = default;
 
   std::string GetString() const override;

libc/malloc_debug/TrackData.h

@@ -46,7 +46,7 @@ class DebugData;
 
 class TrackData : public OptionData {
  public:
-  TrackData(DebugData* debug_data);
+  explicit TrackData(DebugData* debug_data);
   virtual ~TrackData() = default;
 
   void GetList(std::vector<const Header*>* list);

libc/private/CachedProperty.h

@@ -38,7 +38,7 @@
 class CachedProperty {
  public:
   // The lifetime of `property_name` must be greater than that of this CachedProperty.
-  CachedProperty(const char* property_name)
+  explicit CachedProperty(const char* property_name)
     : property_name_(property_name),
       prop_info_(nullptr),
       cached_area_serial_(0),

libc/private/KernelArgumentBlock.h

@@ -32,7 +32,7 @@ struct abort_msg_t;
 // constituents for easy access.
 class KernelArgumentBlock {
  public:
-  KernelArgumentBlock(void* raw_args) {
+  explicit KernelArgumentBlock(void* raw_args) {
     uintptr_t* args = reinterpret_cast<uintptr_t*>(raw_args);
     argc = static_cast<int>(*args);
     argv = reinterpret_cast<char**>(args + 1);

libc/system_properties/include/system_properties/system_properties.h

@@ -48,7 +48,7 @@ class SystemProperties {
   SystemProperties() {
   }
   // Special constructor for testing that also zero initializes the important members.
-  SystemProperties(bool initialized) : initialized_(initialized) {
+  explicit SystemProperties(bool initialized) : initialized_(initialized) {
   }
 
   DISALLOW_COPY_AND_ASSIGN(SystemProperties);

linker/linker.cpp

@@ -3631,7 +3631,7 @@ std::vector<android_namespace_t*> init_default_namespaces(const char* executable
   std::string error_msg;
 
   std::string ld_config_vndk = kLdConfigFilePath;
-  size_t insert_pos = ld_config_vndk.find_last_of(".");
+  size_t insert_pos = ld_config_vndk.find_last_of('.');
   if (insert_pos == std::string::npos) {
     insert_pos = ld_config_vndk.length();
   }

tests/TemporaryFile.h

@@ -22,7 +22,7 @@
 template <typename T = int (*)(char*)>
 class GenericTemporaryFile {
  public:
-  GenericTemporaryFile(T mk_fn = mkstemp) : mk_fn(mk_fn) {
+  explicit GenericTemporaryFile(T mk_fn = mkstemp) : mk_fn(mk_fn) {
     // Since we might be running on the host or the target, and if we're
     // running on the host we might be running under bionic or glibc,
     // let's just try both possible temporary directories and take the
@@ -33,7 +33,7 @@ class GenericTemporaryFile {
     }
   }
 
-  GenericTemporaryFile(const char* dirpath, T mk_fn = mkstemp) : mk_fn(mk_fn) {
+  explicit GenericTemporaryFile(const char* dirpath, T mk_fn = mkstemp) : mk_fn(mk_fn) {
     init(dirpath);
   }
 

tests/dlext_test.cpp

@@ -425,7 +425,7 @@ protected:
   }
 
   void CreateRelroFile(const char* lib, const char* relro_file) {
-    int relro_fd = open(relro_file, O_RDWR | O_TRUNC);
+    int relro_fd = open(relro_file, O_RDWR | O_TRUNC | O_CLOEXEC);
     ASSERT_NOERROR(relro_fd);
 
     pid_t pid = fork();
@@ -447,7 +447,7 @@ protected:
     AssertChildExited(pid, 0);
 
     // reopen file for reading so it can be used
-    relro_fd = open(relro_file, O_RDONLY);
+    relro_fd = open(relro_file, O_RDONLY | O_CLOEXEC);
     ASSERT_NOERROR(relro_fd);
     extinfo_.flags |= ANDROID_DLEXT_USE_RELRO;
     extinfo_.relro_fd = relro_fd;

tests/gtest_main.cpp

@@ -90,7 +90,6 @@ void ColoredPrintf(GTestColor color, const char* fmt, ...);
 }  // namespace testing
 
 using testing::internal::GTestColor;
-using testing::internal::COLOR_DEFAULT;
 using testing::internal::COLOR_RED;
 using testing::internal::COLOR_GREEN;
 using testing::internal::COLOR_YELLOW;
@@ -274,8 +273,7 @@ static int64_t NanoTime() {
 }
 
 static bool EnumerateTests(int argc, char** argv, std::vector<TestCase>& testcase_list) {
-  std::vector<const char*> args;
-  for (int i = 0; i < argc; ++i) args.push_back(argv[i]);
+  std::vector<const char*> args(argv, argv + argc);
   args.push_back("--gtest_list_tests");
   args.push_back(nullptr);
 
@@ -543,7 +541,7 @@ void OnTestIterationEndXmlPrint(const std::string& xml_output_filename,
                                 const std::vector<TestCase>& testcase_list,
                                 time_t epoch_iteration_start_time,
                                 int64_t elapsed_time_ns) {
-  FILE* fp = fopen(xml_output_filename.c_str(), "w");
+  FILE* fp = fopen(xml_output_filename.c_str(), "we");
   if (fp == NULL) {
     fprintf(stderr, "failed to open '%s': %s\n", xml_output_filename.c_str(), strerror(errno));
     exit(1);
@@ -1185,10 +1183,7 @@ int main(int argc, char** argv, char** envp) {
   g_argc = argc;
   g_argv = argv;
   g_envp = envp;
-  std::vector<char*> arg_list;
-  for (int i = 0; i < argc; ++i) {
-    arg_list.push_back(argv[i]);
-  }
+  std::vector<char*> arg_list(argv, argv + argc);
 
   IsolationTestOptions options;
   if (PickOptions(arg_list, options) == false) {

tests/utils.h

@@ -180,10 +180,10 @@ class ExecTestHelper {
     return const_cast<char**>(env_.data());
   }
 
-  void SetArgs(const std::vector<const char*> args) {
+  void SetArgs(const std::vector<const char*>& args) {
     args_ = args;
   }
-  void SetEnv(const std::vector<const char*> env) {
+  void SetEnv(const std::vector<const char*>& env) {
     env_ = env;
   }
 

tools/versioner/src/DeclarationDatabase.cpp

@@ -102,21 +102,18 @@ class Visitor : public RecursiveASTVisitor<Visitor> {
       return true;
     }
 
-    DeclarationType declaration_type;
     std::string declaration_name = getDeclName(named_decl);
     bool is_extern = named_decl->getFormalLinkage() == ExternalLinkage;
     bool is_definition = false;
     bool no_guard = false;
 
     if (auto function_decl = dyn_cast<FunctionDecl>(decl)) {
-      declaration_type = DeclarationType::function;
       is_definition = function_decl->isThisDeclarationADefinition();
     } else if (auto var_decl = dyn_cast<VarDecl>(decl)) {
       if (!var_decl->isFileVarDecl()) {
         return true;
       }
 
-      declaration_type = DeclarationType::variable;
       switch (var_decl->isThisDeclarationADefinition()) {
         case VarDecl::DeclarationOnly:
           is_definition = false;

tools/versioner/src/versioner.cpp

@@ -496,7 +496,6 @@ extern "C" const char* __asan_default_options() {
 
 int main(int argc, char** argv) {
   std::string cwd = getWorkingDir() + "/";
-  bool default_args = true;
   std::string platform_dir;
   std::set<Arch> selected_architectures;
   std::set<int> selected_levels;
@@ -507,7 +506,6 @@ int main(int argc, char** argv) {
 
   int c;
   while ((c = getopt(argc, argv, "a:r:p:so:fdj:vhFi")) != -1) {
-    default_args = false;
     switch (c) {
       case 'a': {
         char* end;