AudioFlinger: Update utils for clang tidy
Test: ALLOW_LOCAL_TIDY_TRUE=1 mm -j .
Bug: 286457446
Change-Id: Ifce1035205077c4d429a32249e8f2e6c87d55b7b
diff --git a/services/audioflinger/afutils/Android.bp b/services/audioflinger/afutils/Android.bp
index 4309bf5..5eac519 100644
--- a/services/audioflinger/afutils/Android.bp
+++ b/services/audioflinger/afutils/Android.bp
@@ -7,11 +7,108 @@
default_applicable_licenses: ["frameworks_av_services_audioflinger_license"],
}
+audioflinger_utils_tidy_errors = [
+ // https://clang.llvm.org/extra/clang-tidy/checks/list.html
+ // For many categories, the checks are too many to specify individually.
+ // Feel free to disable as needed - as warnings are generally ignored,
+ // we treat warnings as errors.
+ "android-*",
+ "bugprone-*",
+ "cert-*",
+ "clang-analyzer-security*",
+ "google-*",
+ "misc-*",
+ //"modernize-*", // explicitly list the modernize as they can be subjective.
+ "modernize-avoid-bind",
+ //"modernize-avoid-c-arrays", // std::array<> can be verbose
+ "modernize-concat-nested-namespaces",
+ //"modernize-deprecated-headers", // C headers still ok even if there is C++ equivalent.
+ "modernize-deprecated-ios-base-aliases",
+ "modernize-loop-convert",
+ "modernize-make-shared",
+ "modernize-make-unique",
+ // "modernize-pass-by-value",
+ "modernize-raw-string-literal",
+ "modernize-redundant-void-arg",
+ "modernize-replace-auto-ptr",
+ "modernize-replace-random-shuffle",
+ "modernize-return-braced-init-list",
+ "modernize-shrink-to-fit",
+ "modernize-unary-static-assert",
+ // "modernize-use-auto", // found in MediaMetricsService.h, debatable - auto can obscure type
+ "modernize-use-bool-literals",
+ "modernize-use-default-member-init",
+ "modernize-use-emplace",
+ "modernize-use-equals-default",
+ "modernize-use-equals-delete",
+ // "modernize-use-nodiscard",
+ "modernize-use-noexcept",
+ "modernize-use-nullptr",
+ "modernize-use-override",
+ //"modernize-use-trailing-return-type", // not necessarily more readable
+ "modernize-use-transparent-functors",
+ "modernize-use-uncaught-exceptions",
+ "modernize-use-using",
+ "performance-*",
+
+ // Remove some pedantic stylistic requirements.
+ "-google-readability-casting", // C++ casts not always necessary and may be verbose
+ "-google-readability-todo", // do not require TODO(info)
+
+ "-bugprone-unhandled-self-assignment",
+ "-bugprone-suspicious-string-compare",
+ "-cert-oop54-cpp", // found in TransactionLog.h
+ "-bugprone-narrowing-conversions", // b/182410845
+
+ // TODO(b/275642749) Reenable these warnings
+ "-misc-non-private-member-variables-in-classes",
+]
+
+// Eventually use common tidy defaults
+cc_defaults {
+ name: "audioflinger_utils_flags_defaults",
+ // https://clang.llvm.org/docs/UsersManual.html#command-line-options
+ // https://clang.llvm.org/docs/DiagnosticsReference.html
+ cflags: [
+ "-Wall",
+ "-Wdeprecated",
+ "-Werror",
+ "-Werror=implicit-fallthrough",
+ "-Werror=sometimes-uninitialized",
+ "-Werror=conditional-uninitialized",
+ "-Wextra",
+
+ // suppress some warning chatter.
+ "-Wno-deprecated-copy-with-dtor",
+ "-Wno-deprecated-copy-with-user-provided-dtor",
+
+ "-Wredundant-decls",
+ "-Wshadow",
+ "-Wstrict-aliasing",
+ "-fstrict-aliasing",
+ "-Wthread-safety",
+ //"-Wthread-safety-negative", // experimental - looks broken in R.
+ "-Wunreachable-code",
+ "-Wunreachable-code-break",
+ "-Wunreachable-code-return",
+ "-Wunused",
+ "-Wused-but-marked-unused",
+ "-D_LIBCPP_ENABLE_THREAD_SAFETY_ANNOTATIONS",
+ ],
+ // https://clang.llvm.org/extra/clang-tidy/
+ tidy: true,
+ tidy_checks: audioflinger_utils_tidy_errors,
+ tidy_checks_as_errors: audioflinger_utils_tidy_errors,
+ tidy_flags: [
+ "-format-style=file",
+ ],
+}
+
cc_library {
name: "libaudioflinger_utils",
defaults: [
- "audioflinger_flags_defaults",
+ "audioflinger_utils_flags_defaults",
],
srcs: [
diff --git a/services/audioflinger/afutils/AudioWatchdog.cpp b/services/audioflinger/afutils/AudioWatchdog.cpp
index 877e776..3116584 100644
--- a/services/audioflinger/afutils/AudioWatchdog.cpp
+++ b/services/audioflinger/afutils/AudioWatchdog.cpp
@@ -41,7 +41,7 @@
bool AudioWatchdog::threadLoop()
{
{
- AutoMutex _l(mMyLock);
+ const AutoMutex _l(mMyLock);
if (mPaused) {
mMyCond.wait(mMyLock);
// ignore previous timestamp after resume()
@@ -116,13 +116,13 @@
void AudioWatchdog::pause()
{
- AutoMutex _l(mMyLock);
+ const AutoMutex _l(mMyLock);
mPaused = true;
}
void AudioWatchdog::resume()
{
- AutoMutex _l(mMyLock);
+ const AutoMutex _l(mMyLock);
if (mPaused) {
mPaused = false;
mMyCond.signal();
diff --git a/services/audioflinger/afutils/AudioWatchdog.h b/services/audioflinger/afutils/AudioWatchdog.h
index 7b69fc6..217761a 100644
--- a/services/audioflinger/afutils/AudioWatchdog.h
+++ b/services/audioflinger/afutils/AudioWatchdog.h
@@ -19,8 +19,7 @@
// as soon as possible when there appears to be a CPU shortage
// (b) monitor the other threads [not yet implemented]
-#ifndef AUDIO_WATCHDOG_H
-#define AUDIO_WATCHDOG_H
+#pragma once
#include <time.h>
#include <utils/Thread.h>
@@ -30,33 +29,27 @@
// Keeps a cache of AudioWatchdog statistics that can be logged by dumpsys.
// The usual caveats about atomicity of information apply.
struct AudioWatchdogDump {
- AudioWatchdogDump() : mUnderruns(0), mLogs(0), mMostRecent(0) { }
- /*virtual*/ ~AudioWatchdogDump() { }
- uint32_t mUnderruns; // total number of underruns
- uint32_t mLogs; // total number of log messages
- time_t mMostRecent; // time of most recent log
+ uint32_t mUnderruns = 0; // total number of underruns
+ uint32_t mLogs = 0; // total number of log messages
+ time_t mMostRecent = 0; // time of most recent log
void dump(int fd); // should only be called on a stable copy, not the original
};
class AudioWatchdog : public Thread {
public:
- explicit AudioWatchdog(unsigned periodMs = 50) : Thread(false /*canCallJava*/), mPaused(false),
- mPeriodNs(periodMs * 1000000), mMaxCycleNs(mPeriodNs * 2),
- // mOldTs
- // mLogTs initialized below
- mOldTsValid(false), mUnderruns(0), mLogs(0), mDump(&mDummyDump)
+ explicit AudioWatchdog(unsigned periodMs = 50) : Thread(false /*canCallJava*/),
+ mPeriodNs(periodMs * 1000000), mMaxCycleNs(mPeriodNs * 2)
{
-#define MIN_TIME_BETWEEN_LOGS_SEC 60
+ constexpr int32_t MIN_TIME_BETWEEN_LOGS_SEC = 60;
// force an immediate log on first underrun
mLogTs.tv_sec = MIN_TIME_BETWEEN_LOGS_SEC;
mLogTs.tv_nsec = 0;
}
- virtual ~AudioWatchdog() { }
// Do not call Thread::requestExitAndWait() without first calling requestExit().
// Thread::requestExitAndWait() is not virtual, and the implementation doesn't do enough.
- virtual void requestExit();
+ void requestExit() override;
// FIXME merge API and implementation with AudioTrackThread
void pause(); // suspend thread from execution at next loop boundary
@@ -66,23 +59,22 @@
void setDump(AudioWatchdogDump* dump);
private:
- virtual bool threadLoop();
+ bool threadLoop() override;
Mutex mMyLock; // Thread::mLock is private
Condition mMyCond; // Thread::mThreadExitedCondition is private
- bool mPaused; // whether thread is currently paused
+ bool mPaused = false; // whether thread is currently paused
uint32_t mPeriodNs; // nominal period
uint32_t mMaxCycleNs; // maximum allowed time of one cycle before declaring underrun
struct timespec mOldTs; // monotonic time when threadLoop last ran
- struct timespec mLogTs; // time since last log
- bool mOldTsValid; // whether mOldTs is valid
- uint32_t mUnderruns; // total number of underruns
- uint32_t mLogs; // total number of logs
- AudioWatchdogDump* mDump; // where to store the dump, always non-NULL
+ struct timespec mLogTs; // time since last log (initialized in ctor).
+ bool mOldTsValid = false; // whether mOldTs is valid
+ uint32_t mUnderruns = 0; // total number of underruns
+ uint32_t mLogs = 0; // total number of logs
+ AudioWatchdogDump* mDump{&mDummyDump}; // where to store the dump, always non-NULL
AudioWatchdogDump mDummyDump; // default area for dump in case setDump() is not called
};
} // namespace android
-#endif // AUDIO_WATCHDOG_H
diff --git a/services/audioflinger/afutils/BufLog.cpp b/services/audioflinger/afutils/BufLog.cpp
index 5f6aca0..47efe00 100644
--- a/services/audioflinger/afutils/BufLog.cpp
+++ b/services/audioflinger/afutils/BufLog.cpp
@@ -28,12 +28,14 @@
#define MIN(a, b) ((a) < (b) ? (a) : (b))
+namespace android {
+
// ------------------------------
// BufLogSingleton
// ------------------------------
pthread_once_t onceControl = PTHREAD_ONCE_INIT;
-BufLog *BufLogSingleton::mInstance = NULL;
+BufLog *BufLogSingleton::mInstance = nullptr;
void BufLogSingleton::initOnce() {
mInstance = new BufLog();
@@ -49,7 +51,7 @@
}
bool BufLogSingleton::instanceExists() {
- return mInstance != NULL;
+ return mInstance != nullptr;
}
// ------------------------------
@@ -61,43 +63,42 @@
}
BufLog::~BufLog() {
- android::Mutex::Autolock autoLock(mLock);
+ const AutoMutex autoLock(mLock);
- for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) {
+ for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) { // NOLINT(modernize-loop-convert)
BufLogStream *pBLStream = mStreams[id];
- if (pBLStream != NULL) {
+ if (pBLStream != nullptr) {
delete pBLStream ;
- mStreams[id] = NULL;
+ mStreams[id] = nullptr;
}
}
}
size_t BufLog::write(int streamid, const char *tag, int format, int channels,
int samplingRate, size_t maxBytes, const void *buf, size_t size) {
- unsigned int id = streamid % BUFLOG_MAXSTREAMS;
- android::Mutex::Autolock autoLock(mLock);
+ const unsigned int id = streamid % BUFLOG_MAXSTREAMS;
+ const AutoMutex autoLock(mLock);
BufLogStream *pBLStream = mStreams[id];
- if (pBLStream == NULL) {
+ if (pBLStream == nullptr) {
pBLStream = mStreams[id] = new BufLogStream(id, tag, format, channels,
samplingRate, maxBytes);
- ALOG_ASSERT(pBLStream != NULL, "BufLogStream Failed to be created");
}
return pBLStream->write(buf, size);
}
void BufLog::reset() {
- android::Mutex::Autolock autoLock(mLock);
+ const AutoMutex autoLock(mLock);
ALOGV("Resetting all BufLogs");
int count = 0;
- for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) {
+ for (unsigned int id = 0; id < BUFLOG_MAXSTREAMS; id++) { // NOLINT(modernize-loop-convert)
BufLogStream *pBLStream = mStreams[id];
- if (pBLStream != NULL) {
+ if (pBLStream != nullptr) {
delete pBLStream;
- mStreams[id] = NULL;
+ mStreams[id] = nullptr;
count++;
}
}
@@ -117,7 +118,7 @@
mSamplingRate(samplingRate), mMaxBytes(maxBytes) {
mByteCount = 0;
mPaused = false;
- if (tag != NULL) {
+ if (tag != nullptr) {
(void)audio_utils_strlcpy(mTag, tag);
} else {
mTag[0] = 0;
@@ -129,7 +130,7 @@
//timestamp
char timeStr[16]; //size 16: format %Y%m%d%H%M%S 14 chars + string null terminator
struct timeval tv;
- gettimeofday(&tv, NULL);
+ gettimeofday(&tv, nullptr);
struct tm tm;
localtime_r(&tv.tv_sec, &tm);
strftime(timeStr, sizeof(timeStr), "%Y%m%d%H%M%S", &tm);
@@ -139,7 +140,7 @@
ALOGV("data output: %s", logPath);
mFile = fopen(logPath, "wb");
- if (mFile != NULL) {
+ if (mFile != nullptr) {
ALOGV("Success creating file at: %p", mFile);
} else {
ALOGE("Error: could not create file BufLogStream %s", strerror(errno));
@@ -148,24 +149,24 @@
void BufLogStream::closeStream_l() {
ALOGV("Closing BufLogStream id:%d tag:%s", mId, mTag);
- if (mFile != NULL) {
+ if (mFile != nullptr) {
fclose(mFile);
- mFile = NULL;
+ mFile = nullptr;
}
}
BufLogStream::~BufLogStream() {
ALOGV("Destroying BufLogStream id:%d tag:%s", mId, mTag);
- android::Mutex::Autolock autoLock(mLock);
+ const AutoMutex autoLock(mLock);
closeStream_l();
}
size_t BufLogStream::write(const void *buf, size_t size) {
size_t bytes = 0;
- if (!mPaused && mFile != NULL) {
- if (size > 0 && buf != NULL) {
- android::Mutex::Autolock autoLock(mLock);
+ if (!mPaused && mFile != nullptr) {
+ if (size > 0 && buf != nullptr) {
+ const AutoMutex autoLock(mLock);
if (mMaxBytes > 0) {
size = MIN(size, mMaxBytes - mByteCount);
}
@@ -185,12 +186,14 @@
}
bool BufLogStream::setPause(bool pause) {
- bool old = mPaused;
+ const bool old = mPaused;
mPaused = pause;
return old;
}
void BufLogStream::finalize() {
- android::Mutex::Autolock autoLock(mLock);
+ const AutoMutex autoLock(mLock);
closeStream_l();
}
+
+} // namespace android
diff --git a/services/audioflinger/afutils/BufLog.h b/services/audioflinger/afutils/BufLog.h
index 1b402f4..0bb16d6 100644
--- a/services/audioflinger/afutils/BufLog.h
+++ b/services/audioflinger/afutils/BufLog.h
@@ -14,8 +14,7 @@
* limitations under the License.
*/
-#ifndef ANDROID_AUDIO_BUFLOG_H
-#define ANDROID_AUDIO_BUFLOG_H
+#pragma once
/*
* BUFLOG creates up to BUFLOG_MAXSTREAMS simultaneous streams [0:15] of audio buffer data
@@ -99,7 +98,6 @@
BufLogSingleton::instance()->reset(); } } while (0)
#endif
-
#include <stdint.h>
#include <stdio.h>
#include <sys/types.h>
@@ -110,6 +108,8 @@
#define BUFLOG_BASE_PATH "/data/misc/audioserver"
#define BUFLOG_MAX_PATH_SIZE 300
+namespace android {
+
class BufLogStream {
public:
BufLogStream(unsigned int id,
@@ -196,4 +196,4 @@
static BufLog *mInstance;
};
-#endif //ANDROID_AUDIO_BUFLOG_H
+} // namespace android
diff --git a/services/audioflinger/afutils/NBAIO_Tee.cpp b/services/audioflinger/afutils/NBAIO_Tee.cpp
index 53083d5..d0682b5 100644
--- a/services/audioflinger/afutils/NBAIO_Tee.cpp
+++ b/services/audioflinger/afutils/NBAIO_Tee.cpp
@@ -123,7 +123,7 @@
std::string generateFilename(const std::string &suffix) const {
char fileTime[sizeof("YYYYmmdd_HHMMSS_\0")];
struct timeval tv;
- gettimeofday(&tv, NULL);
+ gettimeofday(&tv, nullptr /* struct timezone */);
struct tm tm;
localtime_r(&tv.tv_sec, &tm);
LOG_ALWAYS_FATAL_IF(strftime(fileTime, sizeof(fileTime), "%Y%m%d_%H%M%S_", &tm) == 0,
@@ -230,14 +230,16 @@
Pipe *pipe = new Pipe(frames, format);
size_t numCounterOffers = 0;
const NBAIO_Format offers[1] = {format};
- ssize_t index = pipe->negotiate(offers, 1, NULL, numCounterOffers);
+ ssize_t index = pipe->negotiate(
+ offers, 1 /* numOffers */, nullptr /* counterOffers */, numCounterOffers);
if (index != 0) {
ALOGW("pipe failure to negotiate: %zd", index);
goto exit;
}
PipeReader *pipeReader = new PipeReader(*pipe);
numCounterOffers = 0;
- index = pipeReader->negotiate(offers, 1, NULL, numCounterOffers);
+ index = pipeReader->negotiate(
+ offers, 1 /* numOffers */, nullptr /* counterOffers */, numCounterOffers);
if (index != 0) {
ALOGW("pipeReader failure to negotiate: %zd", index);
goto exit;
diff --git a/services/audioflinger/afutils/NBAIO_Tee.h b/services/audioflinger/afutils/NBAIO_Tee.h
index fed8cc8..918cca2 100644
--- a/services/audioflinger/afutils/NBAIO_Tee.h
+++ b/services/audioflinger/afutils/NBAIO_Tee.h
@@ -15,8 +15,8 @@
*/
// Enabled with TEE_SINK in Configuration.h
-#ifndef ANDROID_NBAIO_TEE_H
-#define ANDROID_NBAIO_TEE_H
+
+#pragma once
#ifdef TEE_SINK
@@ -320,7 +320,4 @@
const std::shared_ptr<NBAIO_TeeImpl> mTee;
}; // NBAIO_Tee
-} // namespace android
-
#endif // TEE_SINK
-#endif // !ANDROID_NBAIO_TEE_H
diff --git a/services/audioflinger/afutils/TypedLogger.h b/services/audioflinger/afutils/TypedLogger.h
index f34a50c..8c2d239 100644
--- a/services/audioflinger/afutils/TypedLogger.h
+++ b/services/audioflinger/afutils/TypedLogger.h
@@ -15,8 +15,7 @@
* limitations under the License.
*/
-#ifndef ANDROID_TYPED_LOGGER_H
-#define ANDROID_TYPED_LOGGER_H
+#pragma once
// This is the client API for the typed logger.
@@ -136,6 +135,5 @@
NBLog::Writer *getThreadWriter();
void setThreadWriter(NBLog::Writer *writer);
-} // namespace android::aflog
-#endif // ANDROID_TYPED_LOGGER_H
+} // namespace android::aflog
