From e8f4b14301e697e0cc54754b1837ce106f08e304 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Fri, 19 Oct 2018 16:09:39 -0700 Subject: [PATCH] Add a simple MappedFile to libbase and switch fastboot and libziparchive over. This allows us to remove libziparchive's dependency on libutils. Bug: http://b/79112958 Test: ran libbase and libziparchive tests, ran fastboot manually Change-Id: I95c651976dad222863e5b8c37d4514b778f5dce7 --- base/Android.bp | 2 + base/include/android-base/mapped_file.h | 74 +++++++++++++++++++ base/mapped_file.cpp | 71 ++++++++++++++++++ base/mapped_file_test.cpp | 40 ++++++++++ fastboot/fastboot.cpp | 4 +- fastboot/fastboot_driver.cpp | 9 +-- libziparchive/Android.bp | 8 -- .../include/ziparchive/zip_archive.h | 7 +- libziparchive/include/ziparchive/zip_writer.h | 6 +- libziparchive/zip_archive.cc | 42 ++++------- libziparchive/zip_archive_private.h | 15 ++-- libziparchive/zip_archive_test.cc | 9 +-- libziparchive/zip_writer.cc | 6 +- 13 files changed, 224 insertions(+), 69 deletions(-) create mode 100644 base/include/android-base/mapped_file.h create mode 100644 base/mapped_file.cpp create mode 100644 base/mapped_file_test.cpp diff --git a/base/Android.bp b/base/Android.bp index daa820ab4..741664bb9 100644 --- a/base/Android.bp +++ b/base/Android.bp @@ -47,6 +47,7 @@ cc_defaults { "chrono_utils.cpp", "file.cpp", "logging.cpp", + "mapped_file.cpp", "parsenetaddress.cpp", "properties.cpp", "quick_exit.cpp", @@ -124,6 +125,7 @@ cc_test { "file_test.cpp", "logging_test.cpp", "macros_test.cpp", + "mapped_file_test.cpp", "parsedouble_test.cpp", "parseint_test.cpp", "parsenetaddress_test.cpp", diff --git a/base/include/android-base/mapped_file.h b/base/include/android-base/mapped_file.h new file mode 100644 index 000000000..52d11eda9 --- /dev/null +++ b/base/include/android-base/mapped_file.h @@ -0,0 +1,74 @@ +/* + * Copyright (C) 2018 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. + */ + +#pragma once + +#include "android-base/macros.h" + +#include + +#include + +#if defined(_WIN32) +#include +#define PROT_READ 1 +#define PROT_WRITE 2 +#else +#include +#endif + +namespace android { +namespace base { + +/** + * A region of a file mapped into memory. + */ +class MappedFile { + public: + /** + * Creates a new mapping of the file pointed to by `fd`. Unlike the underlying OS primitives, + * `offset` does not need to be page-aligned. If `PROT_WRITE` is set in `prot`, the mapping + * will be writable, otherwise it will be read-only. Mappings are always `MAP_SHARED`. + */ + static std::unique_ptr FromFd(int fd, off64_t offset, size_t length, int prot); + + /** + * Removes the mapping. + */ + ~MappedFile(); + + char* data() { return base_ + offset_; } + size_t size() { return size_; } + + private: + DISALLOW_IMPLICIT_CONSTRUCTORS(MappedFile); + + char* base_; + size_t size_; + + size_t offset_; + +#if defined(_WIN32) + MappedFile(char* base, size_t size, size_t offset, HANDLE handle) + : base_(base), size_(size), offset_(offset), handle_(handle) {} + HANDLE handle_; +#else + MappedFile(char* base, size_t size, size_t offset) : base_(base), size_(size), offset_(offset) {} +#endif +}; + +} // namespace base +} // namespace android diff --git a/base/mapped_file.cpp b/base/mapped_file.cpp new file mode 100644 index 000000000..f7901afa4 --- /dev/null +++ b/base/mapped_file.cpp @@ -0,0 +1,71 @@ +/* + * Copyright (C) 2018 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. + */ + +#include "android-base/mapped_file.h" + +namespace android { +namespace base { + +static off64_t InitPageSize() { +#if defined(_WIN32) + SYSTEM_INFO si; + GetSystemInfo(&si); + return si.dwAllocationGranularity; +#else + return sysconf(_SC_PAGE_SIZE); +#endif +} + +std::unique_ptr MappedFile::FromFd(int fd, off64_t offset, size_t length, int prot) { + static off64_t page_size = InitPageSize(); + size_t slop = offset % page_size; + off64_t file_offset = offset - slop; + off64_t file_length = length + slop; + +#if defined(_WIN32) + HANDLE handle = + CreateFileMapping(reinterpret_cast(_get_osfhandle(fd)), nullptr, + (prot & PROT_WRITE) ? PAGE_READWRITE : PAGE_READONLY, 0, 0, nullptr); + if (handle == nullptr) return nullptr; + void* base = MapViewOfFile(handle, (prot & PROT_WRITE) ? FILE_MAP_ALL_ACCESS : FILE_MAP_READ, 0, + file_offset, file_length); + if (base == nullptr) { + CloseHandle(handle); + return nullptr; + } + return std::unique_ptr( + new MappedFile{static_cast(base), length, slop, handle}); +#else + void* base = mmap(nullptr, file_length, prot, MAP_SHARED, fd, file_offset); + if (base == MAP_FAILED) return nullptr; + return std::unique_ptr(new MappedFile{static_cast(base), length, slop}); +#endif +} + +MappedFile::~MappedFile() { +#if defined(_WIN32) + if (base_ != nullptr) UnmapViewOfFile(base_); + if (handle_ != nullptr) CloseHandle(handle_); +#else + if (base_ != nullptr) munmap(base_, size_); +#endif + + base_ = nullptr; + offset_ = size_ = 0; +} + +} // namespace base +} // namespace android diff --git a/base/mapped_file_test.cpp b/base/mapped_file_test.cpp new file mode 100644 index 000000000..57fde6f45 --- /dev/null +++ b/base/mapped_file_test.cpp @@ -0,0 +1,40 @@ +/* + * Copyright (C) 2018 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. + */ + +#include "android-base/mapped_file.h" + +#include + +#include +#include +#include + +#include + +#include "android-base/file.h" +#include "android-base/test_utils.h" +#include "android-base/unique_fd.h" + +TEST(mapped_file, smoke) { + TemporaryFile tf; + ASSERT_TRUE(tf.fd != -1); + ASSERT_TRUE(android::base::WriteStringToFd("hello world", tf.fd)); + + auto m = android::base::MappedFile::FromFd(tf.fd, 3, 2, PROT_READ); + ASSERT_EQ(2u, m->size()); + ASSERT_EQ('l', m->data()[0]); + ASSERT_EQ('o', m->data()[1]); +} diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 6b6e659c6..925cac430 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -79,8 +79,8 @@ using android::base::Split; using android::base::Trim; using android::base::unique_fd; -#ifndef O_BINARY -#define O_BINARY 0 +#if defined(_WIN32) +#define O_CLOEXEC O_NOINHERIT #endif static const char* serial = nullptr; diff --git a/fastboot/fastboot_driver.cpp b/fastboot/fastboot_driver.cpp index b1f3bc91b..65a52475d 100644 --- a/fastboot/fastboot_driver.cpp +++ b/fastboot/fastboot_driver.cpp @@ -41,10 +41,10 @@ #include #include +#include #include #include #include -#include #include "constants.h" #include "transport.h" @@ -467,15 +467,14 @@ RetCode FastBootDriver::SendBuffer(int fd, size_t size) { while (remaining) { // Memory map the file - android::FileMap filemap; size_t len = std::min(remaining, MAX_MAP_SIZE); - - if (!filemap.create(NULL, fd, offset, len, true)) { + auto mapping{android::base::MappedFile::FromFd(fd, offset, len, PROT_READ)}; + if (!mapping) { error_ = "Creating filemap failed"; return IO_ERROR; } - if ((ret = SendBuffer(filemap.getDataPtr(), len))) { + if ((ret = SendBuffer(mapping->data(), mapping->size()))) { return ret; } diff --git a/libziparchive/Android.bp b/libziparchive/Android.bp index 3308adf37..608afb7fd 100644 --- a/libziparchive/Android.bp +++ b/libziparchive/Android.bp @@ -84,14 +84,6 @@ cc_library { "libz", ], target: { - android: { - shared_libs: [ - "libutils", - ], - }, - host: { - static_libs: ["libutils"], - }, linux_bionic: { enabled: true, }, diff --git a/libziparchive/include/ziparchive/zip_archive.h b/libziparchive/include/ziparchive/zip_archive.h index 32d790186..3952532b0 100644 --- a/libziparchive/include/ziparchive/zip_archive.h +++ b/libziparchive/include/ziparchive/zip_archive.h @@ -14,17 +14,16 @@ * limitations under the License. */ +#pragma once + /* * Read-only access to Zip archives, with minimal heap allocation. */ -#ifndef LIBZIPARCHIVE_ZIPARCHIVE_H_ -#define LIBZIPARCHIVE_ZIPARCHIVE_H_ #include #include #include #include -#include /* Zip compression methods we support */ enum { @@ -273,5 +272,3 @@ class Reader { int32_t Inflate(const Reader& reader, const uint32_t compressed_length, const uint32_t uncompressed_length, Writer* writer, uint64_t* crc_out); } // namespace zip_archive - -#endif // LIBZIPARCHIVE_ZIPARCHIVE_H_ diff --git a/libziparchive/include/ziparchive/zip_writer.h b/libziparchive/include/ziparchive/zip_writer.h index 0e0caf11d..6e4ca62cf 100644 --- a/libziparchive/include/ziparchive/zip_writer.h +++ b/libziparchive/include/ziparchive/zip_writer.h @@ -14,8 +14,7 @@ * limitations under the License. */ -#ifndef LIBZIPARCHIVE_ZIPWRITER_H_ -#define LIBZIPARCHIVE_ZIPWRITER_H_ +#pragma once #include #include @@ -25,7 +24,6 @@ #include #include "android-base/macros.h" -#include "utils/Compat.h" struct z_stream_s; typedef struct z_stream_s z_stream; @@ -183,5 +181,3 @@ class ZipWriter { std::unique_ptr z_stream_; std::vector buffer_; }; - -#endif /* LIBZIPARCHIVE_ZIPWRITER_H_ */ diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc index 4221ee7d5..9d6d919b1 100644 --- a/libziparchive/zip_archive.cc +++ b/libziparchive/zip_archive.cc @@ -20,7 +20,8 @@ #define LOG_TAG "ziparchive" -#include +#include "ziparchive/zip_archive.h" + #include #include #include @@ -40,12 +41,10 @@ #include #include #include // TEMP_FAILURE_RETRY may or may not be in unistd +#include #include #include #include -#include -#include -#include "ziparchive/zip_archive.h" #include "zlib.h" #include "entry_name_utils-inl.h" @@ -58,12 +57,6 @@ using android::base::get_unaligned; // specified in the local file header and the central directory. static const bool kCrcChecksEnabled = false; -// This is for windows. If we don't open a file in binary mode, weird -// things will happen. -#ifndef O_BINARY -#define O_BINARY 0 -#endif - // The maximum number of bytes to scan backwards for the EOCD start. static const uint32_t kMaxEOCDSearch = kMaxCommentLen + sizeof(EocdRecord); @@ -196,7 +189,7 @@ ZipArchive::ZipArchive(const int fd, bool assume_ownership) close_file(assume_ownership), directory_offset(0), central_directory(), - directory_map(new android::FileMap()), + directory_map(), num_entries(0), hash_table_size(0), hash_table(nullptr) { @@ -212,7 +205,7 @@ ZipArchive::ZipArchive(void* address, size_t length) close_file(false), directory_offset(0), central_directory(), - directory_map(new android::FileMap()), + directory_map(), num_entries(0), hash_table_size(0), hash_table(nullptr) {} @@ -303,8 +296,7 @@ static int32_t MapCentralDirectory0(const char* debug_file_name, ZipArchive* arc * in archive. */ - if (!archive->InitializeCentralDirectory(debug_file_name, - static_cast(eocd->cd_start_offset), + if (!archive->InitializeCentralDirectory(static_cast(eocd->cd_start_offset), static_cast(eocd->cd_size))) { ALOGE("Zip: failed to intialize central directory.\n"); return kMmapFailed; @@ -823,7 +815,7 @@ class MemoryWriter : public zip_archive::Writer { virtual bool Append(uint8_t* buf, size_t buf_size) override { if (bytes_written_ + buf_size > size_) { - ALOGW("Zip: Unexpected size " ZD " (declared) vs " ZD " (actual)", size_, + ALOGW("Zip: Unexpected size %zu (declared) vs %zu (actual)", size_, bytes_written_ + buf_size); return false; } @@ -910,7 +902,7 @@ class FileWriter : public zip_archive::Writer { virtual bool Append(uint8_t* buf, size_t buf_size) override { if (total_bytes_written_ + buf_size > declared_length_) { - ALOGW("Zip: Unexpected size " ZD " (declared) vs " ZD " (actual)", declared_length_, + ALOGW("Zip: Unexpected size %zu (declared) vs %zu (actual)", declared_length_, total_bytes_written_ + buf_size); return false; } @@ -919,7 +911,7 @@ class FileWriter : public zip_archive::Writer { if (result) { total_bytes_written_ += buf_size; } else { - ALOGW("Zip: unable to write " ZD " bytes to file; %s", buf_size, strerror(errno)); + ALOGW("Zip: unable to write %zu bytes to file; %s", buf_size, strerror(errno)); } return result; @@ -1048,7 +1040,7 @@ int32_t Inflate(const Reader& reader, const uint32_t compressed_length, } } while (zerr == Z_OK); - assert(zerr == Z_STREAM_END); /* other errors should've been caught */ + CHECK_EQ(zerr, Z_STREAM_END); /* other errors should've been caught */ // NOTE: zstream.adler is always set to 0, because we're using the -MAX_WBITS // "feature" of zlib to tell it there won't be a zlib file header. zlib @@ -1255,16 +1247,14 @@ void CentralDirectory::Initialize(void* map_base_ptr, off64_t cd_start_offset, s length_ = cd_size; } -bool ZipArchive::InitializeCentralDirectory(const char* debug_file_name, off64_t cd_start_offset, - size_t cd_size) { +bool ZipArchive::InitializeCentralDirectory(off64_t cd_start_offset, size_t cd_size) { if (mapped_zip.HasFd()) { - if (!directory_map->create(debug_file_name, mapped_zip.GetFileDescriptor(), cd_start_offset, - cd_size, true /* read only */)) { - return false; - } + directory_map = android::base::MappedFile::FromFd(mapped_zip.GetFileDescriptor(), + cd_start_offset, cd_size, PROT_READ); + if (!directory_map) return false; - CHECK_EQ(directory_map->getDataLength(), cd_size); - central_directory.Initialize(directory_map->getDataPtr(), 0 /*offset*/, cd_size); + CHECK_EQ(directory_map->size(), cd_size); + central_directory.Initialize(directory_map->data(), 0 /*offset*/, cd_size); } else { if (mapped_zip.GetBasePtr() == nullptr) { ALOGE("Zip: Failed to map central directory, bad mapped_zip base pointer\n"); diff --git a/libziparchive/zip_archive_private.h b/libziparchive/zip_archive_private.h index 83cb11f2b..330a02a07 100644 --- a/libziparchive/zip_archive_private.h +++ b/libziparchive/zip_archive_private.h @@ -14,8 +14,9 @@ * limitations under the License. */ -#ifndef LIBZIPARCHIVE_ZIPARCHIVE_PRIVATE_H_ -#define LIBZIPARCHIVE_ZIPARCHIVE_PRIVATE_H_ +#pragma once + +#include #include #include @@ -24,9 +25,8 @@ #include #include -#include -#include #include "android-base/macros.h" +#include "android-base/mapped_file.h" static const char* kErrorMessages[] = { "Success", @@ -164,7 +164,7 @@ struct ZipArchive { // mapped central directory area off64_t directory_offset; CentralDirectory central_directory; - std::unique_ptr directory_map; + std::unique_ptr directory_map; // number of entries in the Zip archive uint16_t num_entries; @@ -180,8 +180,5 @@ struct ZipArchive { ZipArchive(void* address, size_t length); ~ZipArchive(); - bool InitializeCentralDirectory(const char* debug_file_name, off64_t cd_start_offset, - size_t cd_size); + bool InitializeCentralDirectory(off64_t cd_start_offset, size_t cd_size); }; - -#endif // LIBZIPARCHIVE_ZIPARCHIVE_PRIVATE_H_ diff --git a/libziparchive/zip_archive_test.cc b/libziparchive/zip_archive_test.cc index 23dd5dc84..af9f4c8be 100644 --- a/libziparchive/zip_archive_test.cc +++ b/libziparchive/zip_archive_test.cc @@ -27,10 +27,10 @@ #include #include +#include #include #include #include -#include #include #include @@ -416,11 +416,10 @@ TEST(ziparchive, OpenFromMemory) { ASSERT_EQ(0, fstat(fd, &sb)); // Memory map the file first and open the archive from the memory region. - android::FileMap file_map; - file_map.create(zip_path.c_str(), fd, 0 /*offset*/, sb.st_size, true); + auto file_map{android::base::MappedFile::FromFd(fd, 0, sb.st_size, PROT_READ)}; ZipArchiveHandle handle; - ASSERT_EQ(0, OpenArchiveFromMemory(file_map.getDataPtr(), file_map.getDataLength(), - zip_path.c_str(), &handle)); + ASSERT_EQ(0, + OpenArchiveFromMemory(file_map->data(), file_map->size(), zip_path.c_str(), &handle)); // Assert one entry can be found and extracted correctly. std::string BINARY_PATH("META-INF/com/google/android/update-binary"); diff --git a/libziparchive/zip_writer.cc b/libziparchive/zip_writer.cc index ed1d1350c..981df3add 100644 --- a/libziparchive/zip_writer.cc +++ b/libziparchive/zip_writer.cc @@ -26,8 +26,6 @@ #include #include "android-base/logging.h" -#include "utils/Compat.h" -#include "utils/Log.h" #include "entry_name_utils-inl.h" #include "zip_archive_common.h" @@ -303,10 +301,10 @@ int32_t ZipWriter::PrepareDeflate() { if (zerr != Z_OK) { if (zerr == Z_VERSION_ERROR) { - ALOGE("Installed zlib is not compatible with linked version (%s)", ZLIB_VERSION); + LOG(ERROR) << "Installed zlib is not compatible with linked version (" << ZLIB_VERSION << ")"; return HandleError(kZlibError); } else { - ALOGE("deflateInit2 failed (zerr=%d)", zerr); + LOG(ERROR) << "deflateInit2 failed (zerr=" << zerr << ")"; return HandleError(kZlibError); } }