MediaMetrics: Add and fix clang-tidy
Test: adb shell dumpsys media.metrics
Test: atest mediametrics_tests
Bug: 149850236
Change-Id: Ia02fad2cbefdbfe3deba3897526a21dd88270c27
diff --git a/services/mediametrics/Android.bp b/services/mediametrics/Android.bp
index c87fbd9..645d151 100644
--- a/services/mediametrics/Android.bp
+++ b/services/mediametrics/Android.bp
@@ -1,8 +1,94 @@
// Media Statistics service
//
+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", // 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)
+]
+
+cc_defaults {
+ name: "mediametrics_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",
+ "-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",
+ ],
+ // https://clang.llvm.org/extra/clang-tidy/
+ tidy: true,
+ tidy_checks: tidy_errors,
+ tidy_checks_as_errors: tidy_errors,
+ tidy_flags: [
+ "-format-style='file'",
+ "--header-filter='frameworks/av/services/mediametrics/'",
+ ],
+}
+
cc_binary {
name: "mediametrics",
+ defaults: [
+ "mediametrics_flags_defaults",
+ ],
srcs: [
"main_mediametrics.cpp",
@@ -22,17 +108,13 @@
init_rc: [
"mediametrics.rc",
],
-
- cflags: [
- "-Wall",
- "-Werror",
- "-Wextra",
- "-Wthread-safety",
- ],
}
cc_library_shared {
name: "libmediametricsservice",
+ defaults: [
+ "mediametrics_flags_defaults",
+ ],
srcs: [
"AudioAnalytics.cpp",
@@ -73,11 +155,4 @@
include_dirs: [
"system/media/audio_utils/include",
],
-
- cflags: [
- "-Wall",
- "-Werror",
- "-Wextra",
- "-Wthread-safety",
- ],
}
diff --git a/services/mediametrics/AudioAnalytics.cpp b/services/mediametrics/AudioAnalytics.cpp
index 6138d32..31ad234 100644
--- a/services/mediametrics/AudioAnalytics.cpp
+++ b/services/mediametrics/AudioAnalytics.cpp
@@ -594,4 +594,4 @@
#endif
}
-} // namespace android
+} // namespace android::mediametrics
diff --git a/services/mediametrics/AudioPowerUsage.cpp b/services/mediametrics/AudioPowerUsage.cpp
index b1615bd..b1648d9 100644
--- a/services/mediametrics/AudioPowerUsage.cpp
+++ b/services/mediametrics/AudioPowerUsage.cpp
@@ -160,7 +160,7 @@
return true; //ignore unknown device
}
- for (auto item : mItems) {
+ for (const auto& item : mItems) {
int32_t item_type = 0, item_device = 0;
double item_volume = 0.;
int64_t item_duration_ns = 0;
@@ -259,8 +259,8 @@
const int64_t endCallNs = item->getTimestamp();
const int64_t durationNs = endCallNs - mDeviceTimeNs;
if (durationNs > 0) {
- mDeviceVolume = (mDeviceVolume * (mVolumeTimeNs - mDeviceTimeNs) +
- mVoiceVolume * (endCallNs - mVolumeTimeNs)) / durationNs;
+ mDeviceVolume = (mDeviceVolume * double(mVolumeTimeNs - mDeviceTimeNs) +
+ mVoiceVolume * double(endCallNs - mVolumeTimeNs)) / durationNs;
saveAsItem_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
}
} else if (mode == "AUDIO_MODE_IN_CALL") { // entering call mode
@@ -287,8 +287,8 @@
const int64_t timeNs = item->getTimestamp();
const int64_t durationNs = timeNs - mDeviceTimeNs;
if (durationNs > 0) {
- mDeviceVolume = (mDeviceVolume * (mVolumeTimeNs - mDeviceTimeNs) +
- mVoiceVolume * (timeNs - mVolumeTimeNs)) / durationNs;
+ mDeviceVolume = (mDeviceVolume * double(mVolumeTimeNs - mDeviceTimeNs) +
+ mVoiceVolume * double(timeNs - mVolumeTimeNs)) / durationNs;
mVolumeTimeNs = timeNs;
}
}
@@ -318,8 +318,8 @@
const int64_t endDeviceNs = item->getTimestamp();
const int64_t durationNs = endDeviceNs - mDeviceTimeNs;
if (durationNs > 0) {
- mDeviceVolume = (mDeviceVolume * (mVolumeTimeNs - mDeviceTimeNs) +
- mVoiceVolume * (endDeviceNs - mVolumeTimeNs)) / durationNs;
+ mDeviceVolume = (mDeviceVolume * double(mVolumeTimeNs - mDeviceTimeNs) +
+ mVoiceVolume * double(endDeviceNs - mVolumeTimeNs)) / durationNs;
saveAsItem_l(mPrimaryDevice, durationNs, VOICE_CALL_TYPE, mDeviceVolume);
}
// reset statistics
@@ -391,4 +391,4 @@
return { ss.str(), slot };
}
-} // namespace android
+} // namespace android::mediametrics
diff --git a/services/mediametrics/MediaMetricsService.cpp b/services/mediametrics/MediaMetricsService.cpp
index 3b3dc3e..584bd13 100644
--- a/services/mediametrics/MediaMetricsService.cpp
+++ b/services/mediametrics/MediaMetricsService.cpp
@@ -32,7 +32,8 @@
namespace android {
-using namespace mediametrics;
+using mediametrics::Item;
+using mediametrics::startsWith;
// individual records kept in memory: age or count
// age: <= 28 hours (1 1/6 days)
@@ -63,7 +64,7 @@
bool MediaMetricsService::useUidForPackage(
const std::string& package, const std::string& installer)
{
- if (strchr(package.c_str(), '.') == NULL) {
+ if (strchr(package.c_str(), '.') == nullptr) {
return false; // not of form 'com.whatever...'; assume internal and ok
} else if (strncmp(package.c_str(), "android.", 8) == 0) {
return false; // android.* packages are assumed fine
@@ -203,9 +204,9 @@
// Overwrite package name and version if the caller was untrusted or empty
if (!isTrusted || item->getPkgName().empty()) {
- const uid_t uid = item->getUid();
+ const uid_t uidItem = item->getUid();
const auto [ pkgName, version ] =
- MediaMetricsService::getSanitizedPackageNameAndVersionCode(uid);
+ MediaMetricsService::getSanitizedPackageNameAndVersionCode(uidItem);
item->setPkgName(pkgName);
item->setPkgVersionCode(version);
}
@@ -320,7 +321,7 @@
String8 value(args[i]);
char *endp;
const char *p = value.string();
- long long sec = strtoll(p, &endp, 10);
+ const auto sec = (int64_t)strtoll(p, &endp, 10);
if (endp == p || *endp != '\0' || sec == 0) {
sinceNs = 0;
} else if (sec < 0) {
diff --git a/services/mediametrics/TimeMachine.h b/services/mediametrics/TimeMachine.h
index c82778b..00a44a4 100644
--- a/services/mediametrics/TimeMachine.h
+++ b/services/mediametrics/TimeMachine.h
@@ -18,6 +18,7 @@
#include <any>
#include <map>
+#include <mutex>
#include <sstream>
#include <string>
#include <variant>
@@ -81,6 +82,8 @@
, mCreationTime(time)
, mLastModificationTime(time)
{
+ (void)mCreationTime; // suppress unused warning.
+
// allowUid allows an untrusted client with a matching uid to set properties
// in this key.
// If allowUid == (uid_t)-1, no untrusted client may set properties in the key.
@@ -209,7 +212,7 @@
const std::string mKey;
const uid_t mAllowUid;
- const int64_t mCreationTime __unused;
+ const int64_t mCreationTime;
int64_t mLastModificationTime;
std::map<std::string /* property */, PropertyHistory> mPropertyMap;
@@ -442,7 +445,7 @@
++it) {
if (ll <= 0) break;
if (prefix != nullptr && !startsWith(it->first, prefix)) break;
- std::lock_guard lock(getLockForKey(it->first));
+ std::lock_guard lock2(getLockForKey(it->first));
auto [s, l] = it->second->dump(ll, sinceNs);
ss << s;
ll -= l;
diff --git a/services/mediametrics/iface_statsd.cpp b/services/mediametrics/iface_statsd.cpp
index 3a1eea7..6e51f72 100644
--- a/services/mediametrics/iface_statsd.cpp
+++ b/services/mediametrics/iface_statsd.cpp
@@ -71,7 +71,7 @@
// give me a record, i'll look at the type and upload appropriately
bool dump2Statsd(const std::shared_ptr<const mediametrics::Item>& item) {
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// get the key
std::string key = item->getKey();
diff --git a/services/mediametrics/main_mediametrics.cpp b/services/mediametrics/main_mediametrics.cpp
index 6992c32..3a66538 100644
--- a/services/mediametrics/main_mediametrics.cpp
+++ b/services/mediametrics/main_mediametrics.cpp
@@ -25,9 +25,9 @@
#include <binder/ProcessState.h>
#include <mediautils/LimitProcessMemory.h>
-int main(int argc __unused, char **argv __unused)
+int main(int argc __unused, char **argv)
{
- using namespace android;
+ using namespace android; // NOLINT (clang-tidy)
limitProcessMemory(
"media.metrics.maxmem", /* property that defines limit */
@@ -39,7 +39,8 @@
// to match the service name
// we're replacing "/system/bin/mediametrics" with "media.metrics"
// we add a ".", but discard the path components: we finish with a shorter string
- strcpy(argv[0], MediaMetricsService::kServiceName);
+ const size_t origSize = strlen(argv[0]) + 1; // include null termination.
+ strlcpy(argv[0], MediaMetricsService::kServiceName, origSize);
defaultServiceManager()->addService(
String16(MediaMetricsService::kServiceName), new MediaMetricsService());
diff --git a/services/mediametrics/statsd_audiopolicy.cpp b/services/mediametrics/statsd_audiopolicy.cpp
index 634c801..393c6ae 100644
--- a/services/mediametrics/statsd_audiopolicy.cpp
+++ b/services/mediametrics/statsd_audiopolicy.cpp
@@ -39,7 +39,7 @@
bool statsd_audiopolicy(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -122,4 +122,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_audiorecord.cpp b/services/mediametrics/statsd_audiorecord.cpp
index 69d1661..43feda1 100644
--- a/services/mediametrics/statsd_audiorecord.cpp
+++ b/services/mediametrics/statsd_audiorecord.cpp
@@ -39,7 +39,7 @@
bool statsd_audiorecord(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -155,4 +155,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_audiothread.cpp b/services/mediametrics/statsd_audiothread.cpp
index 300151b..e867f5b 100644
--- a/services/mediametrics/statsd_audiothread.cpp
+++ b/services/mediametrics/statsd_audiothread.cpp
@@ -39,7 +39,7 @@
bool statsd_audiothread(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -204,4 +204,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_audiotrack.cpp b/services/mediametrics/statsd_audiotrack.cpp
index 397cdf3..ee5b9b2 100644
--- a/services/mediametrics/statsd_audiotrack.cpp
+++ b/services/mediametrics/statsd_audiotrack.cpp
@@ -39,7 +39,7 @@
bool statsd_audiotrack(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -146,4 +146,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_codec.cpp b/services/mediametrics/statsd_codec.cpp
index f5fa57e..7a38c7c 100644
--- a/services/mediametrics/statsd_codec.cpp
+++ b/services/mediametrics/statsd_codec.cpp
@@ -39,7 +39,7 @@
bool statsd_codec(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -188,4 +188,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_drm.cpp b/services/mediametrics/statsd_drm.cpp
index 4f2e861..89d6f8f 100644
--- a/services/mediametrics/statsd_drm.cpp
+++ b/services/mediametrics/statsd_drm.cpp
@@ -43,22 +43,22 @@
// mediadrm
bool statsd_mediadrm(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
std::string pkgName = item->getPkgName();
int64_t pkgVersionCode = item->getPkgVersionCode();
int64_t mediaApexVersion = 0;
- char *vendor = NULL;
+ char *vendor = nullptr;
(void) item->getCString("vendor", &vendor);
- char *description = NULL;
+ char *description = nullptr;
(void) item->getCString("description", &description);
- char *serialized_metrics = NULL;
+ char *serialized_metrics = nullptr;
(void) item->getCString("serialized_metrics", &serialized_metrics);
if (enabled_statsd) {
- android::util::BytesField bf_serialized(serialized_metrics ? serialized_metrics : NULL,
+ android::util::BytesField bf_serialized(serialized_metrics ? serialized_metrics : nullptr,
serialized_metrics ? strlen(serialized_metrics)
: 0);
android::util::stats_write(android::util::MEDIAMETRICS_MEDIADRM_REPORTED,
@@ -80,18 +80,18 @@
// widevineCDM
bool statsd_widevineCDM(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
std::string pkgName = item->getPkgName();
int64_t pkgVersionCode = item->getPkgVersionCode();
int64_t mediaApexVersion = 0;
- char *serialized_metrics = NULL;
+ char *serialized_metrics = nullptr;
(void) item->getCString("serialized_metrics", &serialized_metrics);
if (enabled_statsd) {
- android::util::BytesField bf_serialized(serialized_metrics ? serialized_metrics : NULL,
+ android::util::BytesField bf_serialized(serialized_metrics ? serialized_metrics : nullptr,
serialized_metrics ? strlen(serialized_metrics)
: 0);
android::util::stats_write(android::util::MEDIAMETRICS_DRM_WIDEVINE_REPORTED,
@@ -111,7 +111,7 @@
bool statsd_drmmanager(const mediametrics::Item *item)
{
using namespace std::string_literals;
- if (item == NULL) return false;
+ if (item == nullptr) return false;
if (!enabled_statsd) {
ALOGV("NOT sending: drmmanager data");
@@ -123,13 +123,13 @@
int64_t pkgVersionCode = item->getPkgVersionCode();
int64_t mediaApexVersion = 0;
- char *plugin_id = NULL;
+ char *plugin_id = nullptr;
(void) item->getCString("plugin_id", &plugin_id);
- char *description = NULL;
+ char *description = nullptr;
(void) item->getCString("description", &description);
int32_t method_id = -1;
(void) item->getInt32("method_id", &method_id);
- char *mime_types = NULL;
+ char *mime_types = nullptr;
(void) item->getCString("mime_types", &mime_types);
// Corresponds to the 13 APIs tracked in the MediametricsDrmManagerReported statsd proto
diff --git a/services/mediametrics/statsd_extractor.cpp b/services/mediametrics/statsd_extractor.cpp
index 8574358..3d5739f 100644
--- a/services/mediametrics/statsd_extractor.cpp
+++ b/services/mediametrics/statsd_extractor.cpp
@@ -39,7 +39,7 @@
bool statsd_extractor(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -91,4 +91,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_nuplayer.cpp b/services/mediametrics/statsd_nuplayer.cpp
index df7e59f..488bdcb 100644
--- a/services/mediametrics/statsd_nuplayer.cpp
+++ b/services/mediametrics/statsd_nuplayer.cpp
@@ -43,7 +43,7 @@
*/
bool statsd_nuplayer(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -167,4 +167,4 @@
return true;
}
-};
+} // namespace android
diff --git a/services/mediametrics/statsd_recorder.cpp b/services/mediametrics/statsd_recorder.cpp
index 4de1746..6d5fca0 100644
--- a/services/mediametrics/statsd_recorder.cpp
+++ b/services/mediametrics/statsd_recorder.cpp
@@ -39,7 +39,7 @@
bool statsd_recorder(const mediametrics::Item *item)
{
- if (item == NULL) return false;
+ if (item == nullptr) return false;
// these go into the statsd wrapper
const nsecs_t timestamp = MediaMetricsService::roundTime(item->getTimestamp());
@@ -186,4 +186,4 @@
return true;
}
-};
+} // namespace android