From aa449835dc6c17cff588241026fd1d266e4eb8a1 Mon Sep 17 00:00:00 2001
From: Mikhail Naganov <mnaganov@google.com>
Date: Wed, 2 Aug 2023 12:24:15 -0700
Subject: [PATCH] audio: Address comments from an internal review

Bug: 286914845
Test: atest VtsHalAudioCoreTargetTest
Change-Id: I2f636c77f67fdd8eeac70dd304848bf7f76db4e5
(cherry picked from commit 0faf3394254467f71d27dffc6cdba29a0bdf3e3e)
Merged-In: I2f636c77f67fdd8eeac70dd304848bf7f76db4e5
---
 audio/aidl/default/StreamSwitcher.cpp                 | 11 +++++++----
 audio/aidl/default/alsa/Mixer.h                       |  2 +-
 audio/aidl/default/include/core-impl/StreamSwitcher.h |  2 +-
 3 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/audio/aidl/default/StreamSwitcher.cpp b/audio/aidl/default/StreamSwitcher.cpp
index ed4cc8afb2..8ba15a83e6 100644
--- a/audio/aidl/default/StreamSwitcher.cpp
+++ b/audio/aidl/default/StreamSwitcher.cpp
@@ -126,11 +126,10 @@ ndk::ScopedAStatus StreamSwitcher::removeEffect(const std::shared_ptr<IEffect>&
         LOG(ERROR) << __func__ << ": stream was closed";
         return ndk::ScopedAStatus::fromExceptionCode(EX_ILLEGAL_STATE);
     }
-    for (auto it = mEffects.begin(); it != mEffects.end();) {
+    for (auto it = mEffects.begin(); it != mEffects.end(); ++it) {
         if ((*it)->asBinder() == in_effect->asBinder()) {
-            it = mEffects.erase(it);
-        } else {
-            ++it;
+            mEffects.erase(it);
+            break;
         }
     }
     return !mIsStubStream ? mStream->removeEffect(in_effect) : ndk::ScopedAStatus::ok();
@@ -201,6 +200,10 @@ ndk::ScopedAStatus StreamSwitcher::setConnectedDevices(const std::vector<AudioDe
         }
         // The delegate is null because StreamSwitcher handles IStreamCommon methods by itself.
         if (ndk::ScopedAStatus status = mStream->initInstance(nullptr); !status.isOk()) {
+            if (mIsStubStream) {
+                LOG(FATAL) << __func__
+                           << ": failed to initialize stub stream: " << status.getDescription();
+            }
             // Need to close the current failed stream, and report an error.
             // Since we can't operate without a stream implementation, put a stub in.
             RETURN_STATUS_IF_ERROR(closeCurrentStream(false /*validateStreamState*/));
diff --git a/audio/aidl/default/alsa/Mixer.h b/audio/aidl/default/alsa/Mixer.h
index 78728c296e..8fba1e0753 100644
--- a/audio/aidl/default/alsa/Mixer.h
+++ b/audio/aidl/default/alsa/Mixer.h
@@ -72,7 +72,7 @@ class Mixer {
     std::mutex mMixerAccess;
     // The mixer object is owned by ALSA and will be released when the mixer is closed.
     struct mixer* const mMixer;
-    // `mMixerControls` will only be initialized in constructor. After that, it wil only be
+    // `mMixerControls` will only be initialized in constructor. After that, it will only be
     // read but not be modified. Each mixer_ctl object is owned by ALSA, it's life span is
     // the same as of the mixer itself.
     const Controls mMixerControls;
diff --git a/audio/aidl/default/include/core-impl/StreamSwitcher.h b/audio/aidl/default/include/core-impl/StreamSwitcher.h
index 3086ef98ae..5764ad6018 100644
--- a/audio/aidl/default/include/core-impl/StreamSwitcher.h
+++ b/audio/aidl/default/include/core-impl/StreamSwitcher.h
@@ -20,7 +20,7 @@
 
 namespace aidl::android::hardware::audio::core {
 
-// 'StreamSwitcher' is implementation of 'StreamCommonInterface' which allows
+// 'StreamSwitcher' is an implementation of 'StreamCommonInterface' which allows
 // dynamically switching the underlying stream implementation based on currently
 // connected devices. This is achieved by replacing inheritance from
 // 'StreamCommonImpl' with owning an instance of it. StreamSwitcher must be