logd: fix various clang-tidy issues
In order of severity:
1) Add a CHECK() that a pointer is not nullptr, where the analyzer
believes this is possible.
2) Add `final` appropriately to functions called from constructors.
3) Add missing cloexec flags.
4) Add missing `noexcept` and other subtle performance warnings
Test: build with clang-tidy
Change-Id: Ifd9a1299a51027a47382926b2224748b5750d6cf
diff --git a/logd/LogBufferElement.cpp b/logd/LogBufferElement.cpp
index ef9f1cf..dd779f9 100644
--- a/logd/LogBufferElement.cpp
+++ b/logd/LogBufferElement.cpp
@@ -61,7 +61,7 @@
}
}
-LogBufferElement::LogBufferElement(LogBufferElement&& elem)
+LogBufferElement::LogBufferElement(LogBufferElement&& elem) noexcept
: uid_(elem.uid_),
pid_(elem.pid_),
tid_(elem.tid_),
@@ -134,7 +134,7 @@
char* retval = nullptr;
char buffer[256];
snprintf(buffer, sizeof(buffer), "/proc/%u/comm", tid);
- int fd = open(buffer, O_RDONLY);
+ int fd = open(buffer, O_RDONLY | O_CLOEXEC);
if (fd >= 0) {
ssize_t ret = read(fd, buffer, sizeof(buffer));
if (ret >= (ssize_t)sizeof(buffer)) {
diff --git a/logd/LogBufferElement.h b/logd/LogBufferElement.h
index fd5d88f..b263ca5 100644
--- a/logd/LogBufferElement.h
+++ b/logd/LogBufferElement.h
@@ -37,7 +37,7 @@
LogBufferElement(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid, pid_t tid,
uint64_t sequence, const char* msg, uint16_t len);
LogBufferElement(const LogBufferElement& elem);
- LogBufferElement(LogBufferElement&& elem);
+ LogBufferElement(LogBufferElement&& elem) noexcept;
~LogBufferElement();
uint32_t GetTag() const;
diff --git a/logd/LogListener.h b/logd/LogListener.h
index c114e38..566af5b 100644
--- a/logd/LogListener.h
+++ b/logd/LogListener.h
@@ -20,7 +20,7 @@
class LogListener {
public:
- LogListener(LogBuffer* buf);
+ explicit LogListener(LogBuffer* buf);
bool StartListener();
private:
diff --git a/logd/LogPermissions.cpp b/logd/LogPermissions.cpp
index 8f02d5a..3fd0ea1 100644
--- a/logd/LogPermissions.cpp
+++ b/logd/LogPermissions.cpp
@@ -89,7 +89,7 @@
//
for (int retry = 3; !(ret = foundGid && foundUid && foundLog) && retry;
--retry) {
- FILE* file = fopen(filename, "r");
+ FILE* file = fopen(filename, "re");
if (!file) {
continue;
}
diff --git a/logd/LogReader.cpp b/logd/LogReader.cpp
index f71133d..6c97693 100644
--- a/logd/LogReader.cpp
+++ b/logd/LogReader.cpp
@@ -127,7 +127,7 @@
if (cp) {
logMask = 0;
cp += sizeof(_logIds) - 1;
- while (*cp && *cp != '\0') {
+ while (*cp != '\0') {
int val = 0;
while (isdigit(*cp)) {
val = val * 10 + *cp - '0';
diff --git a/logd/LogReaderThread.cpp b/logd/LogReaderThread.cpp
index dc8582d..4a8be01 100644
--- a/logd/LogReaderThread.cpp
+++ b/logd/LogReaderThread.cpp
@@ -25,8 +25,6 @@
#include "LogBuffer.h"
#include "LogReaderList.h"
-using namespace std::placeholders;
-
LogReaderThread::LogReaderThread(LogBuffer* log_buffer, LogReaderList* reader_list,
std::unique_ptr<LogWriter> writer, bool non_block,
unsigned long tail, LogMask log_mask, pid_t pid,
diff --git a/logd/LogStatistics.cpp b/logd/LogStatistics.cpp
index d49982a..3cd8fde 100644
--- a/logd/LogStatistics.cpp
+++ b/logd/LogStatistics.cpp
@@ -88,7 +88,7 @@
} else {
char buffer[512];
snprintf(buffer, sizeof(buffer), "/proc/%u/cmdline", pid);
- int fd = open(buffer, O_RDONLY);
+ int fd = open(buffer, O_RDONLY | O_CLOEXEC);
if (fd >= 0) {
ssize_t ret = read(fd, buffer, sizeof(buffer));
if (ret > 0) {
@@ -944,7 +944,7 @@
uid_t pidToUid(pid_t pid) {
char buffer[512];
snprintf(buffer, sizeof(buffer), "/proc/%u/status", pid);
- FILE* fp = fopen(buffer, "r");
+ FILE* fp = fopen(buffer, "re");
if (fp) {
while (fgets(buffer, sizeof(buffer), fp)) {
int uid = AID_LOGD;
diff --git a/logd/LogStatistics.h b/logd/LogStatistics.h
index 200c228..5f55802 100644
--- a/logd/LogStatistics.h
+++ b/logd/LogStatistics.h
@@ -128,9 +128,9 @@
if (++index < (ssize_t)len) {
size_t num = len - index - 1;
if (num) {
- memmove(&out_keys[index + 1], &out_keys[index], num * sizeof(out_keys[0]));
+ memmove(&out_keys[index + 1], &out_keys[index], num * sizeof(const TKey*));
memmove(&out_entries[index + 1], &out_entries[index],
- num * sizeof(out_entries[0]));
+ num * sizeof(const TEntry*));
}
out_keys[index] = &key;
out_entries[index] = &entry;
@@ -477,15 +477,15 @@
tagNameTable.sizeOf() +
(pidTable.size() * sizeof(pidTable_t::iterator)) +
(tagTable.size() * sizeof(tagTable_t::iterator));
- for (auto it : pidTable) {
+ for (const auto& it : pidTable) {
const char* name = it.second.name();
if (name) size += strlen(name) + 1;
}
- for (auto it : tidTable) {
+ for (const auto& it : tidTable) {
const char* name = it.second.name();
if (name) size += strlen(name) + 1;
}
- for (auto it : tagNameTable) {
+ for (const auto& it : tagNameTable) {
size += sizeof(std::string);
size_t len = it.first.size();
// Account for short string optimization: if the string's length is <= 22 bytes for 64
@@ -505,7 +505,7 @@
}
public:
- LogStatistics(bool enable_statistics);
+ explicit LogStatistics(bool enable_statistics);
void AddTotal(log_id_t log_id, uint16_t size) EXCLUDES(lock_);
void Add(const LogStatisticsElement& entry) EXCLUDES(lock_);
diff --git a/logd/SerializedFlushToState.cpp b/logd/SerializedFlushToState.cpp
index 17ecb6d..6e2e8b0 100644
--- a/logd/SerializedFlushToState.cpp
+++ b/logd/SerializedFlushToState.cpp
@@ -60,7 +60,7 @@
}
log_position.read_offset = read_offset;
- log_positions_[log_id].emplace(std::move(log_position));
+ log_positions_[log_id].emplace(log_position);
}
void SerializedFlushToState::AddMinHeapEntry(log_id_t log_id) {
@@ -149,7 +149,7 @@
min_heap_.pop();
}
for (auto&& element : old_elements) {
- min_heap_.emplace(std::move(element));
+ min_heap_.emplace(element);
}
// Finally set logs_needed_from_next_position_, so CheckForNewLogs() will re-create the
diff --git a/logd/SerializedLogBuffer.cpp b/logd/SerializedLogBuffer.cpp
index 8bda7cf..70b800f 100644
--- a/logd/SerializedLogBuffer.cpp
+++ b/logd/SerializedLogBuffer.cpp
@@ -208,6 +208,7 @@
}
// Otherwise we are stuck due to a reader, so mitigate it.
+ CHECK(oldest != nullptr);
KickReader(oldest, log_id, bytes_to_free);
return false;
}
diff --git a/logd/SerializedLogBuffer.h b/logd/SerializedLogBuffer.h
index 27334ba..346f51f 100644
--- a/logd/SerializedLogBuffer.h
+++ b/logd/SerializedLogBuffer.h
@@ -33,7 +33,7 @@
#include "SerializedLogEntry.h"
#include "rwlock.h"
-class SerializedLogBuffer : public LogBuffer {
+class SerializedLogBuffer final : public LogBuffer {
public:
SerializedLogBuffer(LogReaderList* reader_list, LogTags* tags, LogStatistics* stats);
~SerializedLogBuffer();
diff --git a/logd/SerializedLogChunk.h b/logd/SerializedLogChunk.h
index 75cb936..a8ac8cd 100644
--- a/logd/SerializedLogChunk.h
+++ b/logd/SerializedLogChunk.h
@@ -25,7 +25,7 @@
class SerializedLogChunk {
public:
- SerializedLogChunk(size_t size) : contents_(size) {}
+ explicit SerializedLogChunk(size_t size) : contents_(size) {}
~SerializedLogChunk();
void Compress();
diff --git a/logd/SimpleLogBuffer.h b/logd/SimpleLogBuffer.h
index 2172507..9f7d699 100644
--- a/logd/SimpleLogBuffer.h
+++ b/logd/SimpleLogBuffer.h
@@ -31,7 +31,7 @@
public:
SimpleLogBuffer(LogReaderList* reader_list, LogTags* tags, LogStatistics* stats);
~SimpleLogBuffer();
- void Init() override;
+ void Init() override final;
int Log(log_id_t log_id, log_time realtime, uid_t uid, pid_t pid, pid_t tid, const char* msg,
uint16_t len) override;
@@ -42,7 +42,7 @@
bool Clear(log_id_t id, uid_t uid) override;
unsigned long GetSize(log_id_t id) override;
- int SetSize(log_id_t id, unsigned long size) override;
+ int SetSize(log_id_t id, unsigned long size) override final;
uint64_t sequence() const override { return sequence_.load(std::memory_order_relaxed); }
diff --git a/logd/rwlock.h b/logd/rwlock.h
index 2b27ff1..c37721e 100644
--- a/logd/rwlock.h
+++ b/logd/rwlock.h
@@ -43,7 +43,7 @@
class SCOPED_CAPABILITY SharedLock {
public:
- SharedLock(RwLock& lock) ACQUIRE_SHARED(lock) : lock_(lock) { lock_.lock_shared(); }
+ explicit SharedLock(RwLock& lock) ACQUIRE_SHARED(lock) : lock_(lock) { lock_.lock_shared(); }
~SharedLock() RELEASE() { lock_.unlock(); }
void lock_shared() ACQUIRE_SHARED() { lock_.lock_shared(); }