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
diff --git a/benchmarks/bionic_benchmarks.cpp b/benchmarks/bionic_benchmarks.cpp
index fe7a1b1..d8635cd 100644
--- a/benchmarks/bionic_benchmarks.cpp
+++ b/benchmarks/bionic_benchmarks.cpp
@@ -305,7 +305,7 @@
}
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 @@
}
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 @@
// 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 = "";
}
diff --git a/benchmarks/stdio_benchmark.cpp b/benchmarks/stdio_benchmark.cpp
index 965a3ae..9c81e0f 100644
--- a/benchmarks/stdio_benchmark.cpp
+++ b/benchmarks/stdio_benchmark.cpp
@@ -29,7 +29,7 @@
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);
}
diff --git a/benchmarks/util.h b/benchmarks/util.h
index a546764..3225bc2 100644
--- a/benchmarks/util.h
+++ b/benchmarks/util.h
@@ -29,7 +29,7 @@
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();
diff --git a/libc/bionic/icu.cpp b/libc/bionic/icu.cpp
index 78e551b..c09c9ea 100644
--- a/libc/bionic/icu.cpp
+++ b/libc/bionic/icu.cpp
@@ -68,7 +68,7 @@
}
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;
}
diff --git a/libc/malloc_debug/MapData.h b/libc/malloc_debug/MapData.h
index 0238139..895f78f 100644
--- a/libc/malloc_debug/MapData.h
+++ b/libc/malloc_debug/MapData.h
@@ -41,7 +41,7 @@
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;
diff --git a/libc/malloc_debug/OptionData.h b/libc/malloc_debug/OptionData.h
index 80190f5..5c2d272 100644
--- a/libc/malloc_debug/OptionData.h
+++ b/libc/malloc_debug/OptionData.h
@@ -34,7 +34,7 @@
class OptionData {
public:
- OptionData(DebugData* debug) : debug_(debug) {}
+ explicit OptionData(DebugData* debug) : debug_(debug) {}
~OptionData() = default;
protected:
diff --git a/libc/malloc_debug/RecordData.h b/libc/malloc_debug/RecordData.h
index 6e19923..ccabac2 100644
--- a/libc/malloc_debug/RecordData.h
+++ b/libc/malloc_debug/RecordData.h
@@ -66,7 +66,7 @@
class AllocEntry : public RecordEntry {
public:
- AllocEntry(void* pointer);
+ explicit AllocEntry(void* pointer);
virtual ~AllocEntry() = default;
protected:
@@ -92,7 +92,7 @@
class FreeEntry : public AllocEntry {
public:
- FreeEntry(void* pointer);
+ explicit FreeEntry(void* pointer);
virtual ~FreeEntry() = default;
std::string GetString() const override;
diff --git a/libc/malloc_debug/TrackData.h b/libc/malloc_debug/TrackData.h
index f7486e9..9a649b9 100644
--- a/libc/malloc_debug/TrackData.h
+++ b/libc/malloc_debug/TrackData.h
@@ -46,7 +46,7 @@
class TrackData : public OptionData {
public:
- TrackData(DebugData* debug_data);
+ explicit TrackData(DebugData* debug_data);
virtual ~TrackData() = default;
void GetList(std::vector<const Header*>* list);
diff --git a/libc/private/CachedProperty.h b/libc/private/CachedProperty.h
index 84ead01..bd67d74 100644
--- a/libc/private/CachedProperty.h
+++ b/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),
diff --git a/libc/private/KernelArgumentBlock.h b/libc/private/KernelArgumentBlock.h
index 68d4999..747186c 100644
--- a/libc/private/KernelArgumentBlock.h
+++ b/libc/private/KernelArgumentBlock.h
@@ -32,7 +32,7 @@
// 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);
diff --git a/libc/system_properties/include/system_properties/system_properties.h b/libc/system_properties/include/system_properties/system_properties.h
index c74f875..83ac00c 100644
--- a/libc/system_properties/include/system_properties/system_properties.h
+++ b/libc/system_properties/include/system_properties/system_properties.h
@@ -48,7 +48,7 @@
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);
diff --git a/linker/linker.cpp b/linker/linker.cpp
index 317fdff..63415d8 100644
--- a/linker/linker.cpp
+++ b/linker/linker.cpp
@@ -3631,7 +3631,7 @@
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();
}
diff --git a/tests/TemporaryFile.h b/tests/TemporaryFile.h
index 5c2fe4f..070b71a 100644
--- a/tests/TemporaryFile.h
+++ b/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 @@
}
}
- 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);
}
diff --git a/tests/dlext_test.cpp b/tests/dlext_test.cpp
index 04b83f2..64cfa08 100644
--- a/tests/dlext_test.cpp
+++ b/tests/dlext_test.cpp
@@ -425,7 +425,7 @@
}
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 @@
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;
diff --git a/tests/gtest_main.cpp b/tests/gtest_main.cpp
index 33dbfd9..6b20094 100644
--- a/tests/gtest_main.cpp
+++ b/tests/gtest_main.cpp
@@ -90,7 +90,6 @@
} // 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 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 @@
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 @@
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) {
diff --git a/tests/utils.h b/tests/utils.h
index a5783af..3f4218e 100644
--- a/tests/utils.h
+++ b/tests/utils.h
@@ -180,10 +180,10 @@
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;
}
diff --git a/tools/versioner/src/DeclarationDatabase.cpp b/tools/versioner/src/DeclarationDatabase.cpp
index 19c2f0d..d9cff17 100644
--- a/tools/versioner/src/DeclarationDatabase.cpp
+++ b/tools/versioner/src/DeclarationDatabase.cpp
@@ -102,21 +102,18 @@
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;
diff --git a/tools/versioner/src/versioner.cpp b/tools/versioner/src/versioner.cpp
index efb39bd..af0c067 100644
--- a/tools/versioner/src/versioner.cpp
+++ b/tools/versioner/src/versioner.cpp
@@ -496,7 +496,6 @@
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 c;
while ((c = getopt(argc, argv, "a:r:p:so:fdj:vhFi")) != -1) {
- default_args = false;
switch (c) {
case 'a': {
char* end;