From b511b8aa2179f0c5dd4e5cc180258995e6b41714 Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Mon, 15 May 2023 14:35:24 -0700
Subject: [PATCH] audio: Enable more compile time checks in the default impl

Enable "-Wall, Wextra, Werror, Wthread-safety",
fix discovered issues.

Bug: 205884982
Test: m
Change-Id: I0a8d3095dd24dbb3bc7cf6569c1f71945cd55168
---
 audio/aidl/default/Android.bp                  |  6 ++++++
 audio/aidl/default/Module.cpp                  |  4 ++--
 audio/aidl/default/usb/StreamUsb.cpp           | 11 +++++++----
 audio/aidl/default/usb/UsbAlsaMixerControl.cpp | 15 ++++-----------
 4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/audio/aidl/default/Android.bp b/audio/aidl/default/Android.bp
index c9edae0aa7..31ed044514 100644
--- a/audio/aidl/default/Android.bp
+++ b/audio/aidl/default/Android.bp
@@ -92,6 +92,12 @@ cc_library {
         "audio_policy_configuration_aidl_default",
         "audio_policy_engine_configuration_aidl_default",
     ],
+    cflags: [
+        "-Wall",
+        "-Wextra",
+        "-Werror",
+        "-Wthread-safety",
+    ],
 }
 
 cc_binary {
diff --git a/audio/aidl/default/Module.cpp b/audio/aidl/default/Module.cpp
index 6b417a4331..6885a49d6b 100644
--- a/audio/aidl/default/Module.cpp
+++ b/audio/aidl/default/Module.cpp
@@ -187,7 +187,7 @@ ndk::ScopedAStatus Module::createStreamContext(
         return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
     }
     LOG(DEBUG) << __func__ << ": frame size " << frameSize << " bytes";
-    if (frameSize > kMaximumStreamBufferSizeBytes / in_bufferSizeFrames) {
+    if (frameSize > static_cast<size_t>(kMaximumStreamBufferSizeBytes / in_bufferSizeFrames)) {
         LOG(ERROR) << __func__ << ": buffer size " << in_bufferSizeFrames
                    << " frames is too large, maximum size is "
                    << kMaximumStreamBufferSizeBytes / frameSize;
@@ -281,7 +281,7 @@ ndk::ScopedAStatus Module::findPortIdForNewStream(int32_t in_portConfigId, Audio
                    << " does not correspond to a mix port";
         return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_ARGUMENT);
     }
-    const int32_t maxOpenStreamCount = portIt->ext.get<AudioPortExt::Tag::mix>().maxOpenStreamCount;
+    const size_t maxOpenStreamCount = portIt->ext.get<AudioPortExt::Tag::mix>().maxOpenStreamCount;
     if (maxOpenStreamCount != 0 && mStreams.count(portId) >= maxOpenStreamCount) {
         LOG(ERROR) << __func__ << ": port id " << portId
                    << " has already reached maximum allowed opened stream count: "
diff --git a/audio/aidl/default/usb/StreamUsb.cpp b/audio/aidl/default/usb/StreamUsb.cpp
index 5d1d7febc8..9ac1cc9b77 100644
--- a/audio/aidl/default/usb/StreamUsb.cpp
+++ b/audio/aidl/default/usb/StreamUsb.cpp
@@ -106,10 +106,13 @@ DriverUsb::DriverUsb(const StreamContext& context, bool isInput)
 
 ::android::status_t DriverUsb::transfer(void* buffer, size_t frameCount, size_t* actualFrameCount,
                                         int32_t* latencyMs) {
-    if (!mConfig.has_value() || mConnectedDevices.empty()) {
-        LOG(ERROR) << __func__ << ": failed, has config: " << mConfig.has_value()
-                   << ", has connected devices: " << mConnectedDevices.empty();
-        return ::android::NO_INIT;
+    {
+        std::lock_guard guard(mLock);
+        if (!mConfig.has_value() || mConnectedDevices.empty()) {
+            LOG(ERROR) << __func__ << ": failed, has config: " << mConfig.has_value()
+                       << ", has connected devices: " << mConnectedDevices.empty();
+            return ::android::NO_INIT;
+        }
     }
     if (mIsStandby) {
         if (::android::status_t status = exitStandby(); status != ::android::OK) {
diff --git a/audio/aidl/default/usb/UsbAlsaMixerControl.cpp b/audio/aidl/default/usb/UsbAlsaMixerControl.cpp
index b5337d1642..6c0c24bc09 100644
--- a/audio/aidl/default/usb/UsbAlsaMixerControl.cpp
+++ b/audio/aidl/default/usb/UsbAlsaMixerControl.cpp
@@ -99,16 +99,6 @@ int volumeFloatToInteger(float fValue, int maxValue, int minValue) {
     return minValue + std::ceil((maxValue - minValue) * fValue);
 }
 
-float volumeIntegerToFloat(int iValue, int maxValue, int minValue) {
-    if (iValue > maxValue) {
-        return 1.0f;
-    }
-    if (iValue < minValue) {
-        return 0.0f;
-    }
-    return static_cast<float>(iValue - minValue) / (maxValue - minValue);
-}
-
 }  // namespace
 
 ndk::ScopedAStatus AlsaMixer::setMasterMute(bool muted) {
@@ -146,11 +136,14 @@ ndk::ScopedAStatus AlsaMixer::setVolumes(std::vector<float> volumes) {
         return ndk::ScopedAStatus::fromExceptionCode(EX_UNSUPPORTED_OPERATION);
     }
     const int numValues = it->second->getNumValues();
+    if (numValues < 0) {
+        LOG(FATAL) << __func__ << ": negative number of values: " << numValues;
+    }
     const int maxValue = it->second->getMaxValue();
     const int minValue = it->second->getMinValue();
     std::vector<int> values;
     size_t i = 0;
-    for (; i < numValues && i < values.size(); ++i) {
+    for (; i < static_cast<size_t>(numValues) && i < values.size(); ++i) {
         values.emplace_back(volumeFloatToInteger(volumes[i], maxValue, minValue));
     }
     if (int err = it->second->setArray(values.data(), values.size()); err != 0) {