summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Jiakai Zhang <jiakaiz@google.com> 2025-02-11 10:14:38 -0800
committer Jiakai Zhang <jiakaiz@google.com> 2025-02-13 02:26:42 -0800
commit9a36de50228fdff9ed53e1dd9545eb446ed09943 (patch)
tree3a88c7187ee79d42291ab924063391e0204e7a6d
parent8ad405adff23e156823bdb6813858e981ad57df6 (diff)
Fix unnecessary warnings on embedded profile not found.
Before this change, artd checks the error message returned from ZipArchive::Find against a hardcoded string to tell if the entry is not found, to decide whether it should log. https://r.android.com/3426849 broke this unreliable approach, causing artd to log warnings in the case of the embedded profile being not found, which is supposed to be expected. This CL fixes it by adding a new method `ZipArchive::FindOrNull` that does not treat entry not found as an error. Bug: 317513933 Change-Id: Ie3c3590d0eeabe9f8a72538534091e8019f8250a Test: Presubmit
-rw-r--r--artd/artd.cc9
-rw-r--r--libartbase/base/zip_archive.cc16
-rw-r--r--libartbase/base/zip_archive.h6
-rw-r--r--libartbase/base/zip_archive_test.cc17
4 files changed, 41 insertions, 7 deletions
diff --git a/artd/artd.cc b/artd/artd.cc
index f7be4cdcc4..d781750306 100644
--- a/artd/artd.cc
+++ b/artd/artd.cc
@@ -424,14 +424,11 @@ Result<File> ExtractEmbeddedProfileToFd(const std::string& dex_path) {
return Error() << error_msg;
}
constexpr const char* kEmbeddedProfileEntry = "assets/art-profile/baseline.prof";
- std::unique_ptr<ZipEntry> zip_entry(zip_archive->Find(kEmbeddedProfileEntry, &error_msg));
+ std::unique_ptr<ZipEntry> zip_entry(zip_archive->FindOrNull(kEmbeddedProfileEntry, &error_msg));
size_t size;
if (zip_entry == nullptr || (size = zip_entry->GetUncompressedLength()) == 0) {
- // From system/libziparchive/zip_error.cpp.
- constexpr const char* kEntryNotFound = "Entry not found";
- if (error_msg != kEntryNotFound) {
- LOG(WARNING) << ART_FORMAT(
- "Failed to find zip entry '{}' in '{}': {}", kEmbeddedProfileEntry, dex_path, error_msg);
+ if (!error_msg.empty()) {
+ LOG(WARNING) << error_msg;
}
// The dex file doesn't necessarily contain a profile. This is expected.
return File();
diff --git a/libartbase/base/zip_archive.cc b/libartbase/base/zip_archive.cc
index f717ada6a3..fef23e9105 100644
--- a/libartbase/base/zip_archive.cc
+++ b/libartbase/base/zip_archive.cc
@@ -292,13 +292,27 @@ ZipArchive* ZipArchive::OpenFromFdInternal(int fd,
}
ZipEntry* ZipArchive::Find(const char* name, std::string* error_msg) const {
+ return FindImpl(name, /*allow_entry_not_found=*/false, error_msg);
+}
+
+ZipEntry* ZipArchive::FindOrNull(const char* name, std::string* error_msg) const {
+ return FindImpl(name, /*allow_entry_not_found=*/true, error_msg);
+}
+
+ZipEntry* ZipArchive::FindImpl(const char* name,
+ bool allow_entry_not_found,
+ std::string* error_msg) const {
DCHECK(name != nullptr);
// Resist the urge to delete the space. <: is a bigraph sequence.
std::unique_ptr< ::ZipEntry> zip_entry(new ::ZipEntry);
const int32_t error = FindEntry(handle_, name, zip_entry.get());
if (error != 0) {
- *error_msg = StringPrintf("Failed to find entry '%s': %s", name, ErrorCodeString(error));
+ // From system/libziparchive/zip_error.cpp.
+ constexpr std::string_view kEntryNotFound = "Entry not found";
+ if (!allow_entry_not_found || ErrorCodeString(error) != kEntryNotFound) {
+ *error_msg = StringPrintf("Failed to find entry '%s': %s", name, ErrorCodeString(error));
+ }
return nullptr;
}
diff --git a/libartbase/base/zip_archive.h b/libartbase/base/zip_archive.h
index e740c9f0f0..741a0a1655 100644
--- a/libartbase/base/zip_archive.h
+++ b/libartbase/base/zip_archive.h
@@ -100,6 +100,10 @@ class ZipArchive {
ZipEntry* Find(const char* name, std::string* error_msg) const;
+ // Same as Find, but doesn't return an error message if the entry is not found. The callers
+ // should expect that the returned pointer is null while the error message is empty.
+ ZipEntry* FindOrNull(const char* name, std::string* error_msg) const;
+
~ZipArchive();
private:
@@ -110,6 +114,8 @@ class ZipArchive {
explicit ZipArchive(ZipArchiveHandle handle) : handle_(handle) {}
+ ZipEntry* FindImpl(const char* name, bool allow_entry_not_found, std::string* error_msg) const;
+
friend class ZipEntry;
ZipArchiveHandle handle_;
diff --git a/libartbase/base/zip_archive_test.cc b/libartbase/base/zip_archive_test.cc
index 969cf1297c..f4053abc8f 100644
--- a/libartbase/base/zip_archive_test.cc
+++ b/libartbase/base/zip_archive_test.cc
@@ -64,4 +64,21 @@ TEST_F(ZipArchiveTest, FindAndExtract) {
EXPECT_EQ(zip_entry->GetCrc32(), computed_crc);
}
+TEST_F(ZipArchiveTest, FindEntryNotFound) {
+ std::string error_msg;
+ std::unique_ptr<ZipArchive> zip_archive(
+ ZipArchive::Open(GetLibCoreDexFileNames()[0].c_str(), &error_msg));
+ ASSERT_TRUE(zip_archive.get() != nullptr) << error_msg;
+ ASSERT_TRUE(error_msg.empty());
+
+ std::unique_ptr<ZipEntry> zip_entry(zip_archive->Find("non-existent-entry", &error_msg));
+ ASSERT_EQ(zip_entry, nullptr);
+ ASSERT_FALSE(error_msg.empty());
+ error_msg = "";
+
+ std::unique_ptr<ZipEntry> zip_entry_2(zip_archive->FindOrNull("non-existent-entry", &error_msg));
+ ASSERT_EQ(zip_entry, nullptr);
+ ASSERT_TRUE(error_msg.empty());
+}
+
} // namespace art