diff options
author | 2025-02-11 10:14:38 -0800 | |
---|---|---|
committer | 2025-02-13 02:26:42 -0800 | |
commit | 9a36de50228fdff9ed53e1dd9545eb446ed09943 (patch) | |
tree | 3a88c7187ee79d42291ab924063391e0204e7a6d | |
parent | 8ad405adff23e156823bdb6813858e981ad57df6 (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.cc | 9 | ||||
-rw-r--r-- | libartbase/base/zip_archive.cc | 16 | ||||
-rw-r--r-- | libartbase/base/zip_archive.h | 6 | ||||
-rw-r--r-- | libartbase/base/zip_archive_test.cc | 17 |
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 |