Log an ignore non-fatal errors in deleteRuntimeArtifacts.
This is to comform with the javadoc on this method, which says "Throws
fatal errors. Logs and ignores non-fatal errors." Also, the binder
caller does not expect non-fatal errors and does not handle them.
Test: m test-art-host-gtest-art_artd_tests
Change-Id: If8419e7c3439be35719a09bd14e83342ae4c2442
diff --git a/artd/artd.cc b/artd/artd.cc
index 42eb450..078afd9 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -510,17 +510,24 @@
} // namespace
-#define OR_RETURN_ERROR(func, expr) \
- ({ \
- decltype(expr)&& tmp = (expr); \
- if (!tmp.ok()) { \
- return (func)(tmp.error().message()); \
- } \
- std::move(tmp).value(); \
+#define OR_RETURN_ERROR(func, expr) \
+ ({ \
+ decltype(expr)&& __or_return_error_tmp = (expr); \
+ if (!__or_return_error_tmp.ok()) { \
+ return (func)(__or_return_error_tmp.error().message()); \
+ } \
+ std::move(__or_return_error_tmp).value(); \
})
#define OR_RETURN_FATAL(expr) OR_RETURN_ERROR(Fatal, expr)
#define OR_RETURN_NON_FATAL(expr) OR_RETURN_ERROR(NonFatal, expr)
+#define OR_LOG_AND_RETURN_OK(expr) \
+ OR_RETURN_ERROR( \
+ [](const std::string& message) { \
+ LOG(ERROR) << message; \
+ return ScopedAStatus::ok(); \
+ }, \
+ expr)
ScopedAStatus Artd::isAlive(bool* _aidl_return) {
*_aidl_return = true;
@@ -1281,8 +1288,9 @@
ScopedAStatus Artd::deleteRuntimeArtifacts(const RuntimeArtifactsPath& in_runtimeArtifactsPath,
int64_t* _aidl_return) {
OR_RETURN_FATAL(ValidateRuntimeArtifactsPath(in_runtimeArtifactsPath));
- std::string android_data = OR_RETURN_NON_FATAL(GetAndroidDataOrError());
- std::string android_expand = OR_RETURN_NON_FATAL(GetAndroidExpandOrError());
+ *_aidl_return = 0;
+ std::string android_data = OR_LOG_AND_RETURN_OK(GetAndroidDataOrError());
+ std::string android_expand = OR_LOG_AND_RETURN_OK(GetAndroidExpandOrError());
for (const std::string& file :
ListRuntimeArtifactsFiles(android_data, android_expand, in_runtimeArtifactsPath)) {
*_aidl_return += GetSizeAndDeleteFile(file);
@@ -1309,14 +1317,10 @@
int64_t* _aidl_return) {
OR_RETURN_FATAL(ValidateRuntimeArtifactsPath(in_runtimeArtifactsPath));
*_aidl_return = 0;
- Result<std::string> android_data = GetAndroidDataOrError();
- Result<std::string> android_expand = GetAndroidExpandOrError();
- if (!android_data.ok() || !android_expand.ok()) {
- LOG(ERROR) << "Failed to get the path to ANDROID_DATA or ANDROID_EXPAND";
- return ScopedAStatus::ok();
- }
- for (const std::string& file : ListRuntimeArtifactsFiles(
- android_data.value(), android_expand.value(), in_runtimeArtifactsPath)) {
+ std::string android_data = OR_LOG_AND_RETURN_OK(GetAndroidDataOrError());
+ std::string android_expand = OR_LOG_AND_RETURN_OK(GetAndroidExpandOrError());
+ for (const std::string& file :
+ ListRuntimeArtifactsFiles(android_data, android_expand, in_runtimeArtifactsPath)) {
*_aidl_return += GetSize(file).value_or(0);
}
return ScopedAStatus::ok();
diff --git a/artd/artd_test.cc b/artd/artd_test.cc
index 79e6858..6e6dad4 100644
--- a/artd/artd_test.cc
+++ b/artd/artd_test.cc
@@ -2285,6 +2285,25 @@
}
}
+TEST_F(ArtdTest, deleteRuntimeArtifactsAndroidDataNotExist) {
+ // Will be cleaned up by `android_data_env_`
+ setenv("ANDROID_DATA", "/non-existing", /*replace=*/1);
+
+ auto scoped_set_logger = ScopedSetLogger(mock_logger_.AsStdFunction());
+ EXPECT_CALL(mock_logger_,
+ Call(_, _, _, _, _, HasSubstr("Failed to find directory /non-existing")));
+
+ int64_t aidl_return;
+ ASSERT_TRUE(
+ artd_
+ ->deleteRuntimeArtifacts(
+ {.packageName = "com.android.foo", .dexPath = "/a/b/base.apk", .isa = "arm64"},
+ &aidl_return)
+ .isOk());
+
+ EXPECT_EQ(aidl_return, 0);
+}
+
TEST_F(ArtdTest, deleteRuntimeArtifactsSpecialChars) {
std::vector<std::string> removed_files;
std::vector<std::string> kept_files;