From 1c5704d08c303f47362af9d912b97da3323e09d9 Mon Sep 17 00:00:00 2001 From: Pavel Maltsev Date: Mon, 20 Mar 2017 14:58:59 -0700 Subject: [PATCH 1/2] Handle hidl transaction errors in ConfigStore Test: surface flinger not failing, android now bootsup Bug: b/36445794 Change-Id: I64cc404bec71f5e4eea2e0034f07b86fb60a3e32 Merged-In: I22fa7aab9fa92bc04333aaa0eef45891ebeba8e7 (cherry picked from commit 076b792ade608a7fdf4e600a42550ad17296a39e) --- configstore/utils/include/configstore/Utils.h | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/configstore/utils/include/configstore/Utils.h b/configstore/utils/include/configstore/Utils.h index 98ccae904f..42bf002415 100644 --- a/configstore/utils/include/configstore/Utils.h +++ b/configstore/utils/include/configstore/Utils.h @@ -20,6 +20,10 @@ #include #include +#pragma push_macro("LOG_TAG") +#undef LOG_TAG +#define LOG_TAG "ConfigStoreUtil" + namespace android { namespace hardware { namespace configstore { @@ -39,9 +43,11 @@ decltype(V::value) get(const decltype(V::value) &defValue) { // fallback to the default value ret.specified = false; } else { - (*configs.*func)([&ret](V v) { - ret = v; - }); + auto status = (*configs.*func)([&ret](V v) { ret = v; }); + if (!status.isOk()) { + ALOGE("HIDL call failed. %s", status.description().c_str()); + ret.specified = false; + } } return ret; @@ -91,4 +97,6 @@ std::string getString(const std::string &defValue) { } // namespace hardware } // namespace android +#pragma pop_macro("LOG_TAG") + #endif // ANDROID_HARDWARE_CONFIGSTORE_UTILS_H From a8959849ba40bf0ade8f6520e2983bda914dd9d8 Mon Sep 17 00:00:00 2001 From: Jaesoo Lee Date: Fri, 24 Mar 2017 14:08:24 +0900 Subject: [PATCH 2/2] print log message for values retrieved from configstore This CL adds a code for printing log messages in clients-side utility library functions (getXXX) for configstore so that the developer can figure out which configuration values are actually retrieved from the configstore. Bug: 36275627 Test: Built and check the log message appears Change-Id: I7d5d80ca7a0317816c71016e013f9e73ad23ee08 Merged-In: I2c0895f8afbbb2947b62164acaf62a491c451dc0 (cherry picked from commit 0dc72ecbb798971d6f394d7f4dbc9d1f4006da5f) --- configstore/utils/Android.bp | 10 ++++- configstore/utils/ConfigStoreUtils.cpp | 40 +++++++++++++++++ configstore/utils/include/configstore/Utils.h | 45 +++++++++++++++---- 3 files changed, 85 insertions(+), 10 deletions(-) create mode 100644 configstore/utils/ConfigStoreUtils.cpp diff --git a/configstore/utils/Android.bp b/configstore/utils/Android.bp index aa420d19e0..2c8aad6b4b 100644 --- a/configstore/utils/Android.bp +++ b/configstore/utils/Android.bp @@ -14,14 +14,22 @@ // limitations under the License. // -cc_library_headers { +cc_library_shared { name: "android.hardware.configstore-utils", defaults: ["hidl_defaults"], + + srcs: [ "ConfigStoreUtils.cpp" ], + export_include_dirs: ["include"], + shared_libs: [ + "android.hardware.configstore@1.0", + "libbase", "libhidlbase" ], export_shared_lib_headers: [ + "android.hardware.configstore@1.0", + "libbase", "libhidlbase" ], } diff --git a/configstore/utils/ConfigStoreUtils.cpp b/configstore/utils/ConfigStoreUtils.cpp new file mode 100644 index 0000000000..5a1fb42e27 --- /dev/null +++ b/configstore/utils/ConfigStoreUtils.cpp @@ -0,0 +1,40 @@ +// +// Copyright (C) 2017 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. +// + +#define LOG_TAG "ConfigStore" + +#include +#include + +namespace android { +namespace hardware { +namespace details { + +bool wouldLogInfo() { + return WOULD_LOG(INFO); +} + +void logAlwaysInfo(const std::string& message) { + LOG(INFO) << message; +} + +void logAlwaysError(const std::string& message) { + LOG(ERROR) << message; +} + +} // namespace details +} // namespace hardware +} // namespace android diff --git a/configstore/utils/include/configstore/Utils.h b/configstore/utils/include/configstore/Utils.h index 42bf002415..b054534194 100644 --- a/configstore/utils/include/configstore/Utils.h +++ b/configstore/utils/include/configstore/Utils.h @@ -17,24 +17,31 @@ #ifndef ANDROID_HARDWARE_CONFIGSTORE_UTILS_H #define ANDROID_HARDWARE_CONFIGSTORE_UTILS_H +#include #include -#include -#pragma push_macro("LOG_TAG") -#undef LOG_TAG -#define LOG_TAG "ConfigStoreUtil" +#include namespace android { namespace hardware { + +namespace details { +// Templated classes can use the below method +// to avoid creating dependencies on liblog. +bool wouldLogInfo(); +void logAlwaysInfo(const std::string& message); +void logAlwaysError(const std::string& message); +} // namespace details + namespace configstore { +using namespace android::hardware::configstore::V1_0; // arguments V: type for the value (i.e., OptionalXXX) // I: interface class name // func: member function pointer -using namespace V1_0; - template (I::* func) (std::function)> decltype(V::value) get(const decltype(V::value) &defValue) { + using namespace android::hardware::details; auto getHelper = []()->V { V ret; sp configs = I::getService(); @@ -45,7 +52,11 @@ decltype(V::value) get(const decltype(V::value) &defValue) { } else { auto status = (*configs.*func)([&ret](V v) { ret = v; }); if (!status.isOk()) { - ALOGE("HIDL call failed. %s", status.description().c_str()); + std::ostringstream oss; + oss << "HIDL call failed for retrieving a config item from " + "configstore : " + << status.description().c_str(); + logAlwaysError(oss.str()); ret.specified = false; } } @@ -54,6 +65,24 @@ decltype(V::value) get(const decltype(V::value) &defValue) { }; static V cachedValue = getHelper(); + if (wouldLogInfo()) { + std::string iname = __PRETTY_FUNCTION__; + // func name starts with "func = " in __PRETTY_FUNCTION__ + auto pos = iname.find("func = "); + if (pos != std::string::npos) { + iname = iname.substr(pos + sizeof("func = ")); + iname.pop_back(); // remove trailing ']' + } else { + iname += " (unknown)"; + } + + std::ostringstream oss; + oss << iname << " retrieved: " + << (cachedValue.specified ? cachedValue.value : defValue) + << (cachedValue.specified ? "" : " (default)"); + logAlwaysInfo(oss.str()); + } + return cachedValue.specified ? cachedValue.value : defValue; } @@ -97,6 +126,4 @@ std::string getString(const std::string &defValue) { } // namespace hardware } // namespace android -#pragma pop_macro("LOG_TAG") - #endif // ANDROID_HARDWARE_CONFIGSTORE_UTILS_H