Use canonical result handling macros in libaudiohal
For staying aligned with the audioserver code, it is
preferred to use the same utility macros and functions
for dealing with Binder transaction errors.
One missing piece was an overload for statusTFromBinderStatus
which takes ndk::ScopedAStatus, added it.
Bug: 205884982
Test: m
Change-Id: I50815e9cde6cd1ab35b79b01621049bfa4228ba3
diff --git a/media/audioaidlconversion/include/media/AidlConversionUtil.h b/media/audioaidlconversion/include/media/AidlConversionUtil.h
index 28c7522..a7a96b8 100644
--- a/media/audioaidlconversion/include/media/AidlConversionUtil.h
+++ b/media/audioaidlconversion/include/media/AidlConversionUtil.h
@@ -25,6 +25,7 @@
#include <error/Result.h>
#if defined(BACKEND_NDK)
+#include <android/binder_auto_utils.h>
#include <android/binder_enums.h>
#include <android/binder_status.h>
@@ -362,6 +363,20 @@
// standard Java exception (fromExceptionCode)
}
+#if defined(BACKEND_NDK)
+static inline ::android::status_t statusTFromBinderStatus(const ::ndk::ScopedAStatus &status) {
+ // What we want to do is to 'return statusTFromBinderStatus(status.get()->get())'
+ // However, since the definition of AStatus is not exposed, we have to do the same
+ // via methods of ScopedAStatus:
+ return status.isOk() ? ::android::OK // check ::android::OK,
+ : status.getServiceSpecificError() // service-side error, not standard Java exception
+ // (fromServiceSpecificError)
+ ?: status.getStatus() // a native binder transaction error (fromStatusT)
+ ?: statusTFromExceptionCode(status.getExceptionCode()); // a service-side error with a
+ // standard Java exception (fromExceptionCode)
+}
+#endif
+
/**
* Return a binder::Status from native service status.
*
@@ -403,4 +418,4 @@
#if defined(BACKEND_NDK)
} // namespace aidl
-#endif
\ No newline at end of file
+#endif
diff --git a/media/libaudiohal/impl/EffectHalAidl.cpp b/media/libaudiohal/impl/EffectHalAidl.cpp
index 31c5ca5..daa68a1 100644
--- a/media/libaudiohal/impl/EffectHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectHalAidl.cpp
@@ -17,9 +17,10 @@
#define LOG_TAG "EffectHalAidl"
//#define LOG_NDEBUG 0
+#include <error/expected_utils.h>
#include <media/AidlConversionCppNdk.h>
#include <media/AidlConversionNdk.h>
-#include <media/audiohal/AudioHalUtils.h>
+#include <media/AidlConversionUtil.h>
#include <media/EffectsFactoryApi.h>
#include <mediautils/TimeCheck.h>
#include <utils/Log.h>
@@ -30,6 +31,7 @@
#include <aidl/android/hardware/audio/effect/IEffect.h>
+using ::aidl::android::aidl_utils::statusTFromBinderStatus;
using ::aidl::android::hardware::audio::effect::CommandId;
using ::aidl::android::hardware::audio::effect::Descriptor;
using ::aidl::android::hardware::audio::effect::IEffect;
@@ -85,7 +87,7 @@
memcpy(&mConfig, pCmdData, cmdSize);
State state;
- RETURN_IF_BINDER_FAIL(mEffect->getState(&state));
+ RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getState(&state)));
// effect not open yet, save settings locally
if (state != State::INIT) {
effect_config_t* legacyConfig = static_cast<effect_config_t*>(pCmdData);
@@ -103,7 +105,7 @@
Parameter::Id id;
id.set<Parameter::Id::commonTag>(Parameter::common);
aidlParam.set<Parameter::common>(aidlCommon);
- RETURN_IF_BINDER_FAIL(mEffect->setParameter(aidlParam));
+ RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->setParameter(aidlParam)));
}
*(int*)pReplyData = 0;
*static_cast<int32_t*>(pReplyData) = OK;
@@ -153,7 +155,8 @@
// open with default effect_config_t (convert to Parameter.Common)
IEffect::OpenEffectReturn ret;
Parameter::Common common;
- RETURN_IF_BINDER_FAIL(mEffect->open(common, std::nullopt, &ret));
+ RETURN_STATUS_IF_ERROR(
+ statusTFromBinderStatus(mEffect->open(common, std::nullopt, &ret)));
return OK;
}
case EFFECT_CMD_SET_CONFIG:
@@ -212,7 +215,7 @@
return BAD_VALUE;
}
Descriptor aidlDesc;
- RETURN_IF_BINDER_FAIL(mEffect->getDescriptor(&aidlDesc));
+ RETURN_STATUS_IF_ERROR(statusTFromBinderStatus(mEffect->getDescriptor(&aidlDesc)));
*pDescriptor = VALUE_OR_RETURN_STATUS(
::aidl::android::aidl2legacy_Descriptor_effect_descriptor(aidlDesc));
diff --git a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
index 0039c86..c6234e4 100644
--- a/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
+++ b/media/libaudiohal/impl/EffectsFactoryHalAidl.cpp
@@ -24,7 +24,6 @@
#include <android/binder_manager.h>
#include <media/AidlConversionCppNdk.h>
#include <media/AidlConversionNdk.h>
-#include <media/audiohal/AudioHalUtils.h>
#include <utils/Log.h>
#include "EffectBufferHalAidl.h"
@@ -55,7 +54,7 @@
{
std::lock_guard lg(mLock);
- RETURN_IF_NOT_OK(queryEffectList_l());
+ RETURN_STATUS_IF_ERROR(queryEffectList_l());
*pNumEffects = mDescList->size();
}
ALOGI("%s %d", __func__, *pNumEffects);
@@ -68,7 +67,7 @@
}
std::lock_guard lg(mLock);
- RETURN_IF_NOT_OK(queryEffectList_l());
+ RETURN_STATUS_IF_ERROR(queryEffectList_l());
auto listSize = mDescList->size();
if (index >= listSize) {
@@ -174,7 +173,7 @@
return BAD_VALUE;
}
if (!mDescList) {
- RETURN_IF_NOT_OK(queryEffectList_l());
+ RETURN_STATUS_IF_ERROR(queryEffectList_l());
}
auto matchIt = std::find_if(mDescList->begin(), mDescList->end(),
@@ -195,7 +194,7 @@
return BAD_VALUE;
}
if (!mDescList) {
- RETURN_IF_NOT_OK(queryEffectList_l());
+ RETURN_STATUS_IF_ERROR(queryEffectList_l());
}
std::vector<Descriptor> result;
std::copy_if(mDescList->begin(), mDescList->end(), std::back_inserter(result),
diff --git a/media/libaudiohal/include/media/audiohal/AudioHalUtils.h b/media/libaudiohal/include/media/audiohal/AudioHalUtils.h
deleted file mode 100644
index 4862cba..0000000
--- a/media/libaudiohal/include/media/audiohal/AudioHalUtils.h
+++ /dev/null
@@ -1,35 +0,0 @@
-/*
- * Copyright (C) 2022 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#pragma once
-
-#define RETURN_IF_BINDER_FAIL(expr) \
- do { \
- const ::ndk::ScopedAStatus _temp_status_ = (expr); \
- if (!_temp_status_.isOk()) { \
- ALOGE("%s:%d return with expr %s msg %s", __func__, __LINE__, #expr, \
- _temp_status_.getMessage()); \
- return _temp_status_.getStatus(); \
- } \
- } while (false)
-
-#define RETURN_IF_NOT_OK(statement) \
- do { \
- auto tmp = (statement); \
- if (tmp != OK) { \
- return tmp; \
- } \
- } while (false)