commit a1e8e23b61f4371d07bba0cfb02f959d97727fbd
author Benjamin Schwartz <bsschwar@google.com> Wed Mar 10 14:35:04 2021 -0800
committer Benjamin Schwartz <bsschwar@google.com> Thu Mar 11 11:53:52 2021 -0800
tree 602c3867a06242805e394aad00550956c8e5bf76
parent ef533c3a83887e357ba9afc71db1576900428ddb

powerstats: Return exception codes on error

Bug: 182406354
Test: Presubmit
Change-Id: I59ed60855d76fa51df38cb5dd97b5bc7d7e15057

powerstats/PowerStatsAidl.cpp

diff --git a/powerstats/PowerStatsAidl.cpp b/powerstats/PowerStatsAidl.cpp
index 8b0519d..3afd290 100644
--- a/powerstats/PowerStatsAidl.cpp
+++ b/powerstats/PowerStatsAidl.cpp
@@ -72,15 +72,12 @@
         return getStateResidency(v, _aidl_return);
     }
 
-    binder_status_t err = STATUS_OK;
-
     std::unordered_map<std::string, std::vector<StateResidency>> stateResidencies;
 
     for (const int32_t id : in_powerEntityIds) {
-        // skip any invalid ids
+        // check for invalid ids
         if (id < 0 || id >= mPowerEntityInfos.size()) {
-            err = STATUS_BAD_VALUE;
-            continue;
+            return ndk::ScopedAStatus(AStatus_fromExceptionCode(EX_ILLEGAL_ARGUMENT));
         }
 
         // Check to see if we already have data for the given id
@@ -98,15 +95,12 @@
             };
             _aidl_return->emplace_back(res);
         } else {
-            // Failed to retrieve results for the given id.
-
-            // Set error code to STATUS_FAILED_TRANSACTION but don't overwrite it
-            // if there is already a higher priority error code
-            err = (err == STATUS_OK) ? STATUS_FAILED_TRANSACTION : err;
+            // Failed to get results for the given id.
+            LOG(ERROR) << "Failed to get results for " << powerEntityName;
         }
     }
 
-    return ndk::ScopedAStatus::fromStatus(err);
+    return ndk::ScopedAStatus::ok();
 }
 
 void PowerStats::addEnergyConsumer(std::shared_ptr<IEnergyConsumer> p) {
@@ -141,13 +135,10 @@
         return getEnergyConsumed(v, _aidl_return);
     }
 
-    binder_status_t err = STATUS_OK;
-
     for (const auto id : in_energyConsumerIds) {
-        // skip any invalid ids
+        // check for invalid ids
         if (id < 0 || id >= mEnergyConsumers.size()) {
-            err = STATUS_BAD_VALUE;
-            continue;
+            return ndk::ScopedAStatus(AStatus_fromExceptionCode(EX_ILLEGAL_ARGUMENT));
         }
 
         auto resopt = mEnergyConsumers[id]->getEnergyConsumed();
@@ -156,15 +147,12 @@
             res.id = id;
             _aidl_return->emplace_back(res);
         } else {
-            // Failed to retrieve results for the given id.
-
-            // Set error code to STATUS_FAILED_TRANSACTION but don't overwrite it
-            // if there is already a higher priority error code
-            err = (err == STATUS_OK) ? STATUS_FAILED_TRANSACTION : err;
+            // Failed to get results for the given id.
+            LOG(ERROR) << "Failed to get results for " << mEnergyConsumerInfos[id].name;
         }
     }
 
-    return ndk::ScopedAStatus::fromStatus(err);
+    return ndk::ScopedAStatus::ok();
 }
 
 void PowerStats::setEnergyMeterDataProvider(std::unique_ptr<IEnergyMeterDataProvider> p) {

powerstats/dataproviders/IioEnergyMeterDataProvider.cpp

diff --git a/powerstats/dataproviders/IioEnergyMeterDataProvider.cpp b/powerstats/dataproviders/IioEnergyMeterDataProvider.cpp
index 30caef2..249a30e 100644
--- a/powerstats/dataproviders/IioEnergyMeterDataProvider.cpp
+++ b/powerstats/dataproviders/IioEnergyMeterDataProvider.cpp
@@ -201,11 +201,11 @@
 ndk::ScopedAStatus IioEnergyMeterDataProvider::readEnergyMeter(
         const std::vector<int32_t> &in_channelIds, std::vector<EnergyMeasurement> *_aidl_return) {
     std::scoped_lock lock(mLock);
-    binder_status_t ret = STATUS_OK;
+
     for (const auto &devicePath : mDevicePaths) {
         if (parseEnergyValue(devicePath.first) < 0) {
             LOG(ERROR) << "Error in parsing " << devicePath.first;
-            return ndk::ScopedAStatus::fromStatus(STATUS_FAILED_TRANSACTION);
+            return ndk::ScopedAStatus::ok();
         }
     }
 
@@ -213,21 +213,24 @@
         *_aidl_return = mReading;
     } else {
         _aidl_return->reserve(in_channelIds.size());
-        for (const auto &channelId : in_channelIds) {
-            if (channelId < mChannelInfos.size()) {
-                _aidl_return->emplace_back(mReading[channelId]);
-            } else {
-                ret = STATUS_BAD_VALUE;
+        for (const auto &id : in_channelIds) {
+            // check for invalid ids
+            if (id < 0 || id >= mChannelInfos.size()) {
+                return ndk::ScopedAStatus(AStatus_fromExceptionCode(EX_ILLEGAL_ARGUMENT));
             }
+
+            _aidl_return->emplace_back(mReading[id]);
         }
     }
-    return ndk::ScopedAStatus::fromStatus(ret);
+
+    return ndk::ScopedAStatus::ok();
 }
 
 ndk::ScopedAStatus IioEnergyMeterDataProvider::getEnergyMeterInfo(
         std::vector<Channel> *_aidl_return) {
     std::scoped_lock lk(mLock);
     *_aidl_return = mChannelInfos;
+
     return ndk::ScopedAStatus::ok();
 }