Fix a few clang-tidy issues and add NOLINT for others
android-base:
* Add NOLINT for expanding namespace std for std::string* ostream
overload
libdm:
* Fix missing parentesis around macro parameters
init:
* Fix missing CLOEXEC usage and add NOLINT for the intended
usages.
* Fix missing parentesis around macro parameters
* Fix erase() / remove_if() idiom
* Correctly specific unsigned char when intended
* 'namespace flags' should be signed, since 'flags' it signed for
clone()
* Add clear to property restore vector<string> to empty after move
* Explicit comparison against 0 for strcmp
Test: build
Change-Id: I8c31dafda2c43ebc5aa50124cbbd6e23ed2c4101
diff --git a/base/include/android-base/logging.h b/base/include/android-base/logging.h
index f94cc25..ab6476c 100644
--- a/base/include/android-base/logging.h
+++ b/base/include/android-base/logging.h
@@ -469,7 +469,7 @@
} // namespace base
} // namespace android
-namespace std {
+namespace std { // NOLINT(cert-dcl58-cpp)
// Emit a warning of ostream<< with std::string*. The intention was most likely to print *string.
//
diff --git a/fs_mgr/libdm/include/libdm/dm.h b/fs_mgr/libdm/include/libdm/dm.h
index afcb090..08376c0 100644
--- a/fs_mgr/libdm/include/libdm/dm.h
+++ b/fs_mgr/libdm/include/libdm/dm.h
@@ -38,7 +38,7 @@
#define DM_VERSION2 (0)
#define DM_ALIGN_MASK (7)
-#define DM_ALIGN(x) ((x + DM_ALIGN_MASK) & ~DM_ALIGN_MASK)
+#define DM_ALIGN(x) (((x) + DM_ALIGN_MASK) & ~DM_ALIGN_MASK)
namespace android {
namespace dm {
diff --git a/init/action_manager.cpp b/init/action_manager.cpp
index 9de4085..985b8ad 100644
--- a/init/action_manager.cpp
+++ b/init/action_manager.cpp
@@ -88,7 +88,8 @@
current_command_ = 0;
if (action->oneshot()) {
auto eraser = [&action](std::unique_ptr<Action>& a) { return a.get() == action; };
- actions_.erase(std::remove_if(actions_.begin(), actions_.end(), eraser));
+ actions_.erase(std::remove_if(actions_.begin(), actions_.end(), eraser),
+ actions_.end());
}
}
}
diff --git a/init/builtins.cpp b/init/builtins.cpp
index 55b0248..ba2c7ac 100644
--- a/init/builtins.cpp
+++ b/init/builtins.cpp
@@ -243,7 +243,7 @@
strlcpy(ifr.ifr_name, args[1].c_str(), IFNAMSIZ);
- unique_fd s(TEMP_FAILURE_RETRY(socket(AF_INET, SOCK_DGRAM, 0)));
+ unique_fd s(TEMP_FAILURE_RETRY(socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0)));
if (s < 0) return ErrnoError() << "opening socket failed";
if (ioctl(s, SIOCGIFFLAGS, &ifr) < 0) {
@@ -784,7 +784,7 @@
}
static Result<void> readahead_file(const std::string& filename, bool fully) {
- android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(filename.c_str(), O_RDONLY)));
+ android::base::unique_fd fd(TEMP_FAILURE_RETRY(open(filename.c_str(), O_RDONLY | O_CLOEXEC)));
if (fd == -1) {
return ErrnoError() << "Error opening file";
}
diff --git a/init/first_stage_init.cpp b/init/first_stage_init.cpp
index 36eece8..b60c450 100644
--- a/init/first_stage_init.cpp
+++ b/init/first_stage_init.cpp
@@ -78,7 +78,7 @@
if (S_ISDIR(info.st_mode)) {
is_dir = true;
- auto fd = openat(dfd, de->d_name, O_RDONLY | O_DIRECTORY);
+ auto fd = openat(dfd, de->d_name, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
if (fd >= 0) {
auto subdir =
std::unique_ptr<DIR, decltype(&closedir)>{fdopendir(fd), closedir};
@@ -152,7 +152,7 @@
std::vector<std::pair<std::string, int>> errors;
#define CHECKCALL(x) \
- if (x != 0) errors.emplace_back(#x " failed", errno);
+ if ((x) != 0) errors.emplace_back(#x " failed", errno);
// Clear the umask.
umask(0);
diff --git a/init/persistent_properties.cpp b/init/persistent_properties.cpp
index 73787b9..baa9ad4 100644
--- a/init/persistent_properties.cpp
+++ b/init/persistent_properties.cpp
@@ -69,7 +69,7 @@
continue;
}
- unique_fd fd(openat(dirfd(dir.get()), entry->d_name, O_RDONLY | O_NOFOLLOW));
+ unique_fd fd(openat(dirfd(dir.get()), entry->d_name, O_RDONLY | O_NOFOLLOW | O_CLOEXEC));
if (fd == -1) {
PLOG(ERROR) << "Unable to open persistent property file \"" << entry->d_name << "\"";
continue;
diff --git a/init/property_service.cpp b/init/property_service.cpp
index 1520c9f..b89914f 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -667,9 +667,9 @@
if (flen > 0) {
if (filter[flen - 1] == '*') {
- if (strncmp(key, filter, flen - 1)) continue;
+ if (strncmp(key, filter, flen - 1) != 0) continue;
} else {
- if (strcmp(key, filter)) continue;
+ if (strcmp(key, filter) != 0) continue;
}
}
diff --git a/init/service.cpp b/init/service.cpp
index cd08f3d..f95b675 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -126,7 +126,7 @@
: Service(name, 0, 0, 0, {}, 0, "", subcontext_for_restart_commands, args) {}
Service::Service(const std::string& name, unsigned flags, uid_t uid, gid_t gid,
- const std::vector<gid_t>& supp_gids, unsigned namespace_flags,
+ const std::vector<gid_t>& supp_gids, int namespace_flags,
const std::string& seclabel, Subcontext* subcontext_for_restart_commands,
const std::vector<std::string>& args)
: name_(name),
diff --git a/init/service.h b/init/service.h
index 78f94ce..cc35a8d 100644
--- a/init/service.h
+++ b/init/service.h
@@ -69,9 +69,8 @@
const std::vector<std::string>& args);
Service(const std::string& name, unsigned flags, uid_t uid, gid_t gid,
- const std::vector<gid_t>& supp_gids, unsigned namespace_flags,
- const std::string& seclabel, Subcontext* subcontext_for_restart_commands,
- const std::vector<std::string>& args);
+ const std::vector<gid_t>& supp_gids, int namespace_flags, const std::string& seclabel,
+ Subcontext* subcontext_for_restart_commands, const std::vector<std::string>& args);
static std::unique_ptr<Service> MakeTemporaryOneshotService(const std::vector<std::string>& args);
@@ -109,7 +108,7 @@
int crash_count() const { return crash_count_; }
uid_t uid() const { return proc_attr_.uid; }
gid_t gid() const { return proc_attr_.gid; }
- unsigned namespace_flags() const { return namespaces_.flags; }
+ int namespace_flags() const { return namespaces_.flags; }
const std::vector<gid_t>& supp_gids() const { return proc_attr_.supp_gids; }
const std::string& seclabel() const { return seclabel_; }
const std::vector<int>& keycodes() const { return keycodes_; }
diff --git a/init/service_test.cpp b/init/service_test.cpp
index 4bfaa6b..6a34acc 100644
--- a/init/service_test.cpp
+++ b/init/service_test.cpp
@@ -30,7 +30,7 @@
TEST(service, pod_initialized) {
constexpr auto memory_size = sizeof(Service);
- alignas(alignof(Service)) char old_memory[memory_size];
+ alignas(alignof(Service)) unsigned char old_memory[memory_size];
for (std::size_t i = 0; i < memory_size; ++i) {
old_memory[i] = 0xFF;
@@ -45,7 +45,7 @@
EXPECT_EQ(0, service_in_old_memory->crash_count());
EXPECT_EQ(0U, service_in_old_memory->uid());
EXPECT_EQ(0U, service_in_old_memory->gid());
- EXPECT_EQ(0U, service_in_old_memory->namespace_flags());
+ EXPECT_EQ(0, service_in_old_memory->namespace_flags());
EXPECT_EQ(IoSchedClass_NONE, service_in_old_memory->ioprio_class());
EXPECT_EQ(0, service_in_old_memory->ioprio_pri());
EXPECT_EQ(0, service_in_old_memory->priority());
@@ -64,7 +64,7 @@
EXPECT_EQ(0, service_in_old_memory2->crash_count());
EXPECT_EQ(0U, service_in_old_memory2->uid());
EXPECT_EQ(0U, service_in_old_memory2->gid());
- EXPECT_EQ(0U, service_in_old_memory2->namespace_flags());
+ EXPECT_EQ(0, service_in_old_memory2->namespace_flags());
EXPECT_EQ(IoSchedClass_NONE, service_in_old_memory2->ioprio_class());
EXPECT_EQ(0, service_in_old_memory2->ioprio_pri());
EXPECT_EQ(0, service_in_old_memory2->priority());
diff --git a/init/service_utils.cpp b/init/service_utils.cpp
index f88ea97..34aa837 100644
--- a/init/service_utils.cpp
+++ b/init/service_utils.cpp
@@ -120,22 +120,19 @@
}
void ZapStdio() {
- int fd;
- fd = open("/dev/null", O_RDWR);
+ auto fd = unique_fd{open("/dev/null", O_RDWR | O_CLOEXEC)};
dup2(fd, 0);
dup2(fd, 1);
dup2(fd, 2);
- close(fd);
}
void OpenConsole(const std::string& console) {
- int fd = open(console.c_str(), O_RDWR);
- if (fd == -1) fd = open("/dev/null", O_RDWR);
+ auto fd = unique_fd{open(console.c_str(), O_RDWR | O_CLOEXEC)};
+ if (fd == -1) fd.reset(open("/dev/null", O_RDWR | O_CLOEXEC));
ioctl(fd, TIOCSCTTY, 0);
dup2(fd, 0);
dup2(fd, 1);
dup2(fd, 2);
- close(fd);
}
} // namespace
diff --git a/init/service_utils.h b/init/service_utils.h
index c26b123..365cb29 100644
--- a/init/service_utils.h
+++ b/init/service_utils.h
@@ -30,7 +30,7 @@
namespace init {
struct NamespaceInfo {
- unsigned flags;
+ int flags;
// Pair of namespace type, path to name.
std::vector<std::pair<int, std::string>> namespaces_to_enter;
};
diff --git a/init/subcontext.cpp b/init/subcontext.cpp
index 02ed507..2f9541b 100644
--- a/init/subcontext.cpp
+++ b/init/subcontext.cpp
@@ -246,7 +246,7 @@
// We explicitly do not use O_CLOEXEC here, such that we can reference this FD by number
// in the subcontext process after we exec.
- int child_fd = dup(subcontext_socket);
+ int child_fd = dup(subcontext_socket); // NOLINT(android-cloexec-dup)
if (child_fd < 0) {
PLOG(FATAL) << "Could not dup child_fd";
}
diff --git a/init/tokenizer_test.cpp b/init/tokenizer_test.cpp
index acfc7c7..6b31683 100644
--- a/init/tokenizer_test.cpp
+++ b/init/tokenizer_test.cpp
@@ -46,6 +46,7 @@
return;
case T_NEWLINE:
tokens.emplace_back(std::move(current_line));
+ current_line.clear();
break;
case T_TEXT:
current_line.emplace_back(state.text);
diff --git a/init/uevent_listener.cpp b/init/uevent_listener.cpp
index 62cd2be..ac633776 100644
--- a/init/uevent_listener.cpp
+++ b/init/uevent_listener.cpp
@@ -131,7 +131,7 @@
const ListenerCallback& callback) const {
int dfd = dirfd(d);
- int fd = openat(dfd, "uevent", O_WRONLY);
+ int fd = openat(dfd, "uevent", O_WRONLY | O_CLOEXEC);
if (fd >= 0) {
write(fd, "add\n", 4);
close(fd);
@@ -146,7 +146,7 @@
while ((de = readdir(d)) != nullptr) {
if (de->d_type != DT_DIR || de->d_name[0] == '.') continue;
- fd = openat(dfd, de->d_name, O_RDONLY | O_DIRECTORY);
+ fd = openat(dfd, de->d_name, O_RDONLY | O_DIRECTORY | O_CLOEXEC);
if (fd < 0) continue;
std::unique_ptr<DIR, decltype(&closedir)> d2(fdopendir(fd), closedir);
diff --git a/init/util.cpp b/init/util.cpp
index 2b34242..058a111 100644
--- a/init/util.cpp
+++ b/init/util.cpp
@@ -450,7 +450,7 @@
// SetStdioToDevNull() must be called again in second stage init.
void SetStdioToDevNull(char** argv) {
// Make stdin/stdout/stderr all point to /dev/null.
- int fd = open("/dev/null", O_RDWR);
+ int fd = open("/dev/null", O_RDWR); // NOLINT(android-cloexec-open)
if (fd == -1) {
int saved_errno = errno;
android::base::InitLogging(argv, &android::base::KernelLogger, InitAborter);
