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
This commit is contained in:
Tom Cherry 2019-07-08 15:09:36 -07:00
parent 0e04ce98ef
commit 247ffbf314
16 changed files with 27 additions and 29 deletions

View file

@ -469,7 +469,7 @@ class ScopedLogSeverity {
} // 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.
//

View file

@ -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 {

View file

@ -88,7 +88,8 @@ void ActionManager::ExecuteOneCommand() {
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());
}
}
}

View file

@ -243,7 +243,7 @@ static Result<void> do_ifup(const BuiltinArguments& args) {
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> do_write(const BuiltinArguments& args) {
}
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";
}

View file

@ -78,7 +78,7 @@ void FreeRamdisk(DIR* dir, dev_t dev) {
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 @@ int FirstStageMain(int argc, char** argv) {
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);

View file

@ -69,7 +69,7 @@ Result<PersistentProperties> LoadLegacyPersistentProperties() {
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;

View file

@ -667,9 +667,9 @@ static void LoadProperties(char* data, const char* filter, const char* filename,
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;
}
}

View file

@ -126,7 +126,7 @@ Service::Service(const std::string& name, Subcontext* subcontext_for_restart_com
: 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),

View file

@ -69,9 +69,8 @@ class Service {
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 @@ class Service {
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_; }

View file

@ -30,7 +30,7 @@ namespace init {
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 @@ TEST(service, pod_initialized) {
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 @@ TEST(service, pod_initialized) {
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());

View file

@ -120,22 +120,19 @@ Result<void> SetUpPidNamespace(const char* name) {
}
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

View file

@ -30,7 +30,7 @@ namespace android {
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;
};

View file

@ -246,7 +246,7 @@ void Subcontext::Fork() {
// 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";
}

View file

@ -46,6 +46,7 @@ void RunTest(const std::string& data, const std::vector<std::vector<std::string>
return;
case T_NEWLINE:
tokens.emplace_back(std::move(current_line));
current_line.clear();
break;
case T_TEXT:
current_line.emplace_back(state.text);

View file

@ -131,7 +131,7 @@ ListenerAction UeventListener::RegenerateUeventsForDir(DIR* d,
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 @@ ListenerAction UeventListener::RegenerateUeventsForDir(DIR* d,
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);

View file

@ -450,7 +450,7 @@ static void InitAborter(const char* abort_message) {
// 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);