From 93b5bf261ca984dcebdb538b7691fe47576069eb Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Thu, 25 Oct 2018 10:39:01 -0700 Subject: Refactor the code to check the metadata The two functions check_wipe_package() and check_newer_ab_build() were using the same flow; and checked the same device properties against the metadata file in the package. These properties include: ota_type, pre-device, and serial number. Therefore, we can consolidate the checks to a single function; and continue to check the fingerprint and timestamp only for AB updates. This change also addresses the need to accept multiple serial number in the wipe package. Bug: 118401208 Test: unit tests pass Change-Id: Ia6bc48fb6effcae059a2ff2cf71764b4136b4c00 --- install.cpp | 226 +++++++++++++++++-------------- install.h | 18 ++- recovery.cpp | 33 ++--- tests/component/install_test.cpp | 280 ++++++++++++++++++++++++++++++++++++--- 4 files changed, 410 insertions(+), 147 deletions(-) diff --git a/install.cpp b/install.cpp index 42d264157..e1e8ee879 100644 --- a/install.cpp +++ b/install.cpp @@ -32,9 +32,7 @@ #include #include #include -#include #include -#include #include #include @@ -47,7 +45,6 @@ #include #include #include -#include #include "common.h" #include "otautil/error_code.h" @@ -67,18 +64,7 @@ static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; static std::condition_variable finish_log_temperature; -// This function parses and returns the build.version.incremental -static std::string parse_build_number(const std::string& str) { - size_t pos = str.find('='); - if (pos != std::string::npos) { - return android::base::Trim(str.substr(pos+1)); - } - - LOG(ERROR) << "Failed to parse build number in " << str; - return ""; -} - -bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) { +bool ReadMetadataFromPackage(ZipArchiveHandle zip, std::map* metadata) { CHECK(metadata != nullptr); static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata"; @@ -90,101 +76,79 @@ bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) { } uint32_t length = entry.uncompressed_length; - metadata->resize(length, '\0'); - int32_t err = ExtractToMemory(zip, &entry, reinterpret_cast(&(*metadata)[0]), length); + std::string metadata_string(length, '\0'); + int32_t err = + ExtractToMemory(zip, &entry, reinterpret_cast(&metadata_string[0]), length); if (err != 0) { LOG(ERROR) << "Failed to extract " << METADATA_PATH << ": " << ErrorCodeString(err); return false; } - return true; -} -// Read the build.version.incremental of src/tgt from the metadata and log it to last_install. -static void read_source_target_build(ZipArchiveHandle zip, std::vector* log_buffer) { - std::string metadata; - if (!read_metadata_from_package(zip, &metadata)) { - return; - } - // Examples of the pre-build and post-build strings in metadata: - // pre-build-incremental=2943039 - // post-build-incremental=2951741 - std::vector lines = android::base::Split(metadata, "\n"); - for (const std::string& line : lines) { - std::string str = android::base::Trim(line); - if (android::base::StartsWith(str, "pre-build-incremental")) { - std::string source_build = parse_build_number(str); - if (!source_build.empty()) { - log_buffer->push_back("source_build: " + source_build); - } - } else if (android::base::StartsWith(str, "post-build-incremental")) { - std::string target_build = parse_build_number(str); - if (!target_build.empty()) { - log_buffer->push_back("target_build: " + target_build); - } - } - } -} - -// Parses the metadata of the OTA package in |zip| and checks whether we are allowed to accept this -// A/B package. Downgrading is not allowed unless explicitly enabled in the package and only for -// incremental packages. -static int check_newer_ab_build(ZipArchiveHandle zip) { - std::string metadata_str; - if (!read_metadata_from_package(zip, &metadata_str)) { - return INSTALL_CORRUPT; - } - std::map metadata; - for (const std::string& line : android::base::Split(metadata_str, "\n")) { + for (const std::string& line : android::base::Split(metadata_string, "\n")) { size_t eq = line.find('='); if (eq != std::string::npos) { - metadata[line.substr(0, eq)] = line.substr(eq + 1); + metadata->emplace(android::base::Trim(line.substr(0, eq)), + android::base::Trim(line.substr(eq + 1))); } } - std::string value = android::base::GetProperty("ro.product.device", ""); - const std::string& pkg_device = metadata["pre-device"]; - if (pkg_device != value || pkg_device.empty()) { - LOG(ERROR) << "Package is for product " << pkg_device << " but expected " << value; - return INSTALL_ERROR; + return true; +} + +// Gets the value for the given key in |metadata|. Returns an emtpy string if the key isn't +// present. +static std::string get_value(const std::map& metadata, + const std::string& key) { + const auto& it = metadata.find(key); + return (it == metadata.end()) ? "" : it->second; +} + +static std::string OtaTypeToString(OtaType type) { + switch (type) { + case OtaType::AB: + return "AB"; + case OtaType::BLOCK: + return "BLOCK"; + case OtaType::BRICK: + return "BRICK"; } +} - // We allow the package to not have any serialno; and we also allow it to carry multiple serial - // numbers split by "|"; e.g. serialno=serialno1|serialno2|serialno3 ... We will fail the - // verification if the device's serialno doesn't match any of these carried numbers. - value = android::base::GetProperty("ro.serialno", ""); - const std::string& pkg_serial_no = metadata["serialno"]; - if (!pkg_serial_no.empty()) { - bool match = false; - for (const std::string& number : android::base::Split(pkg_serial_no, "|")) { - if (value == android::base::Trim(number)) { - match = true; - break; - } - } - if (!match) { - LOG(ERROR) << "Package is for serial " << pkg_serial_no; - return INSTALL_ERROR; - } +// Read the build.version.incremental of src/tgt from the metadata and log it to last_install. +static void ReadSourceTargetBuild(const std::map& metadata, + std::vector* log_buffer) { + // Examples of the pre-build and post-build strings in metadata: + // pre-build-incremental=2943039 + // post-build-incremental=2951741 + auto source_build = get_value(metadata, "pre-build-incremental"); + if (!source_build.empty()) { + log_buffer->push_back("source_build: " + source_build); } - if (metadata["ota-type"] != "AB") { - LOG(ERROR) << "Package is not A/B"; - return INSTALL_ERROR; + auto target_build = get_value(metadata, "post-build-incremental"); + if (!target_build.empty()) { + log_buffer->push_back("target_build: " + target_build); } +} +// Checks the build version, fingerprint and timestamp in the metadata of the A/B package. +// Downgrading is not allowed unless explicitly enabled in the package and only for +// incremental packages. +static int CheckAbSpecificMetadata(const std::map& metadata) { // Incremental updates should match the current build. - value = android::base::GetProperty("ro.build.version.incremental", ""); - const std::string& pkg_pre_build = metadata["pre-build-incremental"]; - if (!pkg_pre_build.empty() && pkg_pre_build != value) { - LOG(ERROR) << "Package is for source build " << pkg_pre_build << " but expected " << value; + auto device_pre_build = android::base::GetProperty("ro.build.version.incremental", ""); + auto pkg_pre_build = get_value(metadata, "pre-build-incremental"); + if (!pkg_pre_build.empty() && pkg_pre_build != device_pre_build) { + LOG(ERROR) << "Package is for source build " << pkg_pre_build << " but expected " + << device_pre_build; return INSTALL_ERROR; } - value = android::base::GetProperty("ro.build.fingerprint", ""); - const std::string& pkg_pre_build_fingerprint = metadata["pre-build"]; - if (!pkg_pre_build_fingerprint.empty() && pkg_pre_build_fingerprint != value) { + auto device_fingerprint = android::base::GetProperty("ro.build.fingerprint", ""); + auto pkg_pre_build_fingerprint = get_value(metadata, "pre-build"); + if (!pkg_pre_build_fingerprint.empty() && pkg_pre_build_fingerprint != device_fingerprint) { LOG(ERROR) << "Package is for source build " << pkg_pre_build_fingerprint << " but expected " - << value; + << device_fingerprint; return INSTALL_ERROR; } @@ -194,10 +158,11 @@ static int check_newer_ab_build(ZipArchiveHandle zip) { int64_t pkg_post_timestamp = 0; // We allow to full update to the same version we are running, in case there // is a problem with the current copy of that version. - if (metadata["post-timestamp"].empty() || - !android::base::ParseInt(metadata["post-timestamp"].c_str(), &pkg_post_timestamp) || + auto pkg_post_timestamp_string = get_value(metadata, "post-timestamp"); + if (pkg_post_timestamp_string.empty() || + !android::base::ParseInt(pkg_post_timestamp_string, &pkg_post_timestamp) || pkg_post_timestamp < build_timestamp) { - if (metadata["ota-downgrade"] != "yes") { + if (get_value(metadata, "ota-downgrade") != "yes") { LOG(ERROR) << "Update package is older than the current build, expected a build " "newer than timestamp " << build_timestamp << " but package has timestamp " << pkg_post_timestamp @@ -213,13 +178,55 @@ static int check_newer_ab_build(ZipArchiveHandle zip) { return 0; } +int CheckPackageMetadata(const std::map& metadata, OtaType ota_type) { + auto package_ota_type = get_value(metadata, "ota-type"); + auto expected_ota_type = OtaTypeToString(ota_type); + if (ota_type != OtaType::AB && ota_type != OtaType::BRICK) { + LOG(INFO) << "Skip package metadata check for ota type " << expected_ota_type; + return 0; + } + + if (package_ota_type != expected_ota_type) { + LOG(ERROR) << "Unexpected ota package type, expects " << expected_ota_type << ", actual " + << package_ota_type; + return INSTALL_ERROR; + } + + auto device = android::base::GetProperty("ro.product.device", ""); + auto pkg_device = get_value(metadata, "pre-device"); + if (pkg_device != device || pkg_device.empty()) { + LOG(ERROR) << "Package is for product " << pkg_device << " but expected " << device; + return INSTALL_ERROR; + } + + // We allow the package to not have any serialno; and we also allow it to carry multiple serial + // numbers split by "|"; e.g. serialno=serialno1|serialno2|serialno3 ... We will fail the + // verification if the device's serialno doesn't match any of these carried numbers. + auto pkg_serial_no = get_value(metadata, "serialno"); + if (!pkg_serial_no.empty()) { + auto device_serial_no = android::base::GetProperty("ro.serialno", ""); + bool serial_number_match = false; + for (const auto& number : android::base::Split(pkg_serial_no, "|")) { + if (device_serial_no == android::base::Trim(number)) { + serial_number_match = true; + } + } + if (!serial_number_match) { + LOG(ERROR) << "Package is for serial " << pkg_serial_no; + return INSTALL_ERROR; + } + } + + if (ota_type == OtaType::AB) { + return CheckAbSpecificMetadata(metadata); + } + + return 0; +} + int SetUpAbUpdateCommands(const std::string& package, ZipArchiveHandle zip, int status_fd, std::vector* cmd) { CHECK(cmd != nullptr); - int ret = check_newer_ab_build(zip); - if (ret != 0) { - return ret; - } // For A/B updates we extract the payload properties to a buffer and obtain the RAW payload offset // in the zip file. @@ -311,20 +318,33 @@ static void log_max_temperature(int* max_temperature, const std::atomic& l static int try_update_binary(const std::string& package, ZipArchiveHandle zip, bool* wipe_cache, std::vector* log_buffer, int retry_count, int* max_temperature) { - read_source_target_build(zip, log_buffer); + std::map metadata; + if (!ReadMetadataFromPackage(zip, &metadata)) { + LOG(ERROR) << "Failed to parse metadata in the zip file"; + return INSTALL_CORRUPT; + } + + bool is_ab = android::base::GetBoolProperty("ro.build.ab_update", false); + // Verifies against the metadata in the package first. + if (int check_status = is_ab ? CheckPackageMetadata(metadata, OtaType::AB) : 0; + check_status != 0) { + log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure)); + return check_status; + } + + ReadSourceTargetBuild(metadata, log_buffer); int pipefd[2]; pipe(pipefd); - - bool is_ab = android::base::GetBoolProperty("ro.build.ab_update", false); std::vector args; - int ret = is_ab ? SetUpAbUpdateCommands(package, zip, pipefd[1], &args) - : SetUpNonAbUpdateCommands(package, zip, retry_count, pipefd[1], &args); - if (ret) { + if (int update_status = + is_ab ? SetUpAbUpdateCommands(package, zip, pipefd[1], &args) + : SetUpNonAbUpdateCommands(package, zip, retry_count, pipefd[1], &args); + update_status != 0) { close(pipefd[0]); close(pipefd[1]); log_buffer->push_back(android::base::StringPrintf("error: %d", kUpdateBinaryCommandFailure)); - return ret; + return update_status; } // When executing the update binary contained in the package, the diff --git a/install.h b/install.h index 1d3d0cd27..c6db1d1d9 100644 --- a/install.h +++ b/install.h @@ -19,6 +19,7 @@ #include +#include #include #include @@ -33,6 +34,12 @@ enum InstallResult { INSTALL_KEY_INTERRUPTED }; +enum class OtaType { + AB, + BLOCK, + BRICK, +}; + // Installs the given update package. If INSTALL_SUCCESS is returned and *wipe_cache is true on // exit, caller should wipe the cache partition. int install_package(const std::string& package, bool* wipe_cache, bool needs_mount, @@ -42,12 +49,17 @@ int install_package(const std::string& package, bool* wipe_cache, bool needs_mou // otherwise return false. bool verify_package(const unsigned char* package_data, size_t package_size); -// Read meta data file of the package, write its content in the string pointed by meta_data. -// Return true if succeed, otherwise return false. -bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata); +// Reads meta data file of the package; parses each line in the format "key=value"; and writes the +// result to |metadata|. Return true if succeed, otherwise return false. +bool ReadMetadataFromPackage(ZipArchiveHandle zip, std::map* metadata); // Verifies the compatibility info in a Treble-compatible package. Returns true directly if the // entry doesn't exist. bool verify_package_compatibility(ZipArchiveHandle package_zip); +// Checks if the the metadata in the OTA package has expected values. Returns 0 on success. +// Mandatory checks: ota-type, pre-device and serial number(if presents) +// AB OTA specific checks: pre-build version, fingerprint, timestamp. +int CheckPackageMetadata(const std::map& metadata, OtaType ota_type); + #endif // RECOVERY_INSTALL_H_ diff --git a/recovery.cpp b/recovery.cpp index d7bc6fd2f..e17526a95 100644 --- a/recovery.cpp +++ b/recovery.cpp @@ -520,34 +520,17 @@ static bool check_wipe_package(size_t wipe_package_size) { LOG(ERROR) << "Can't open wipe package : " << ErrorCodeString(err); return false; } - std::string metadata; - if (!read_metadata_from_package(zip, &metadata)) { - CloseArchive(zip); - return false; + + std::map metadata; + if (!ReadMetadataFromPackage(zip, &metadata)) { + LOG(ERROR) << "Failed to parse metadata in the zip file"; + return false; } + + int result = CheckPackageMetadata(metadata, OtaType::BRICK); CloseArchive(zip); - // Check metadata - std::vector lines = android::base::Split(metadata, "\n"); - bool ota_type_matched = false; - bool device_type_matched = false; - bool has_serial_number = false; - bool serial_number_matched = false; - for (const auto& line : lines) { - if (line == "ota-type=BRICK") { - ota_type_matched = true; - } else if (android::base::StartsWith(line, "pre-device=")) { - std::string device_type = line.substr(strlen("pre-device=")); - std::string real_device_type = android::base::GetProperty("ro.build.product", ""); - device_type_matched = (device_type == real_device_type); - } else if (android::base::StartsWith(line, "serialno=")) { - std::string serial_no = line.substr(strlen("serialno=")); - std::string real_serial_no = android::base::GetProperty("ro.serialno", ""); - has_serial_number = true; - serial_number_matched = (serial_no == real_serial_no); - } - } - return ota_type_matched && device_type_matched && (!has_serial_number || serial_number_matched); + return result == 0; } // Wipes the current A/B device, with a secure wipe of all the partitions in RECOVERY_WIPE. diff --git a/tests/component/install_test.cpp b/tests/component/install_test.cpp index 6583c96f0..5c6d58472 100644 --- a/tests/component/install_test.cpp +++ b/tests/component/install_test.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include @@ -74,15 +75,15 @@ TEST(InstallTest, verify_package_compatibility_invalid_entry) { TEST(InstallTest, read_metadata_from_package_smoke) { TemporaryFile temp_file; - const std::string content("abcdefg"); + const std::string content("abc=defg"); BuildZipArchive({ { "META-INF/com/android/metadata", content } }, temp_file.release(), kCompressStored); ZipArchiveHandle zip; ASSERT_EQ(0, OpenArchive(temp_file.path, &zip)); - std::string metadata; - ASSERT_TRUE(read_metadata_from_package(zip, &metadata)); - ASSERT_EQ(content, metadata); + std::map metadata; + ASSERT_TRUE(ReadMetadataFromPackage(zip, &metadata)); + ASSERT_EQ("defg", metadata["abc"]); CloseArchive(zip); TemporaryFile temp_file2; @@ -91,8 +92,8 @@ TEST(InstallTest, read_metadata_from_package_smoke) { ASSERT_EQ(0, OpenArchive(temp_file2.path, &zip)); metadata.clear(); - ASSERT_TRUE(read_metadata_from_package(zip, &metadata)); - ASSERT_EQ(content, metadata); + ASSERT_TRUE(ReadMetadataFromPackage(zip, &metadata)); + ASSERT_EQ("defg", metadata["abc"]); CloseArchive(zip); } @@ -102,8 +103,8 @@ TEST(InstallTest, read_metadata_from_package_no_entry) { ZipArchiveHandle zip; ASSERT_EQ(0, OpenArchive(temp_file.path, &zip)); - std::string metadata; - ASSERT_FALSE(read_metadata_from_package(zip, &metadata)); + std::map metadata; + ASSERT_FALSE(ReadMetadataFromPackage(zip, &metadata)); CloseArchive(zip); } @@ -235,11 +236,11 @@ static void VerifyAbUpdateCommands(const std::string& serialno, bool success = t if (!serialno.empty()) { meta.push_back("serialno=" + serialno); } - std::string metadata = android::base::Join(meta, "\n"); + std::string metadata_string = android::base::Join(meta, "\n"); BuildZipArchive({ { "payload.bin", "" }, { "payload_properties.txt", properties }, - { "META-INF/com/android/metadata", metadata } }, + { "META-INF/com/android/metadata", metadata_string } }, temp_file.release(), kCompressStored); ZipArchiveHandle zip; @@ -247,10 +248,15 @@ static void VerifyAbUpdateCommands(const std::string& serialno, bool success = t ZipString payload_name("payload.bin"); ZipEntry payload_entry; ASSERT_EQ(0, FindEntry(zip, payload_name, &payload_entry)); - int status_fd = 10; - std::string package = "/path/to/update.zip"; - std::vector cmd; + + std::map metadata; + ASSERT_TRUE(ReadMetadataFromPackage(zip, &metadata)); if (success) { + ASSERT_EQ(0, CheckPackageMetadata(metadata, OtaType::AB)); + + int status_fd = 10; + std::string package = "/path/to/update.zip"; + std::vector cmd; ASSERT_EQ(0, SetUpAbUpdateCommands(package, zip, status_fd, &cmd)); ASSERT_EQ(5U, cmd.size()); ASSERT_EQ("/system/bin/update_engine_sideload", cmd[0]); @@ -259,7 +265,7 @@ static void VerifyAbUpdateCommands(const std::string& serialno, bool success = t ASSERT_EQ("--headers=" + properties, cmd[3]); ASSERT_EQ("--status_fd=" + std::to_string(status_fd), cmd[4]); } else { - ASSERT_EQ(INSTALL_ERROR, SetUpAbUpdateCommands(package, zip, status_fd, &cmd)); + ASSERT_EQ(INSTALL_ERROR, CheckPackageMetadata(metadata, OtaType::AB)); } CloseArchive(zip); } @@ -282,8 +288,12 @@ TEST(InstallTest, SetUpAbUpdateCommands_MissingPayloadPropertiesTxt) { }, "\n"); - BuildZipArchive({ { "payload.bin", "" }, { "META-INF/com/android/metadata", metadata } }, - temp_file.release(), kCompressStored); + BuildZipArchive( + { + { "payload.bin", "" }, + { "META-INF/com/android/metadata", metadata }, + }, + temp_file.release(), kCompressStored); ZipArchiveHandle zip; ASSERT_EQ(0, OpenArchive(temp_file.path, &zip)); @@ -322,3 +332,241 @@ TEST(InstallTest, SetUpAbUpdateCommands_MultipleSerialnos) { // String with the matching serialno should pass the verification. VerifyAbUpdateCommands(long_serialno); } + +static void test_check_package_metadata(const std::string& metadata_string, OtaType ota_type, + int exptected_result) { + TemporaryFile temp_file; + BuildZipArchive( + { + { "META-INF/com/android/metadata", metadata_string }, + }, + temp_file.release(), kCompressStored); + + ZipArchiveHandle zip; + ASSERT_EQ(0, OpenArchive(temp_file.path, &zip)); + + std::map metadata; + ASSERT_TRUE(ReadMetadataFromPackage(zip, &metadata)); + ASSERT_EQ(exptected_result, CheckPackageMetadata(metadata, ota_type)); + CloseArchive(zip); +} + +TEST(InstallTest, CheckPackageMetadata_ota_type) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + // ota-type must be present + std::string metadata = android::base::Join( + std::vector{ + "pre-device=" + device, + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); + + // Checks if ota-type matches + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, 0); + + test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR); +} + +TEST(InstallTest, CheckPackageMetadata_device_type) { + // device type can not be empty + std::string metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR); + + // device type mismatches + metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=dummy_device_type", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR); +} + +TEST(InstallTest, CheckPackageMetadata_serial_number_smoke) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + // Serial number doesn't need to exist + std::string metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=" + device, + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, 0); + + // Serial number mismatches + metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=" + device, + "serialno=dummy_serial", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR); + + std::string serialno = android::base::GetProperty("ro.serialno", ""); + ASSERT_NE("", serialno); + metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=" + device, + "serialno=" + serialno, + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, 0); +} + +TEST(InstallTest, CheckPackageMetadata_multiple_serial_number) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + std::string serialno = android::base::GetProperty("ro.serialno", ""); + ASSERT_NE("", serialno); + + std::vector serial_numbers; + // Creates a dummy serial number string. + for (size_t c = 'a'; c <= 'z'; c++) { + serial_numbers.emplace_back(c, serialno.size()); + } + + // No matched serialno found. + std::string metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=" + device, + "serialno=" + android::base::Join(serial_numbers, '|'), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, INSTALL_ERROR); + + serial_numbers.emplace_back(serialno); + std::shuffle(serial_numbers.begin(), serial_numbers.end(), std::default_random_engine()); + metadata = android::base::Join( + std::vector{ + "ota-type=BRICK", + "pre-device=" + device, + "serialno=" + android::base::Join(serial_numbers, '|'), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::BRICK, 0); +} + +TEST(InstallTest, CheckPackageMetadata_ab_build_version) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + std::string build_version = android::base::GetProperty("ro.build.version.incremental", ""); + ASSERT_NE("", build_version); + + std::string metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "pre-build-incremental=" + build_version, + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, 0); + + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "pre-build-incremental=dummy_build", + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); +} + +TEST(InstallTest, CheckPackageMetadata_ab_fingerprint) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + std::string finger_print = android::base::GetProperty("ro.build.fingerprint", ""); + ASSERT_NE("", finger_print); + + std::string metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "pre-build=" + finger_print, + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, 0); + + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "pre-build=dummy_build_fingerprint", + "post-timestamp=" + std::to_string(std::numeric_limits::max()), + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); +} + +TEST(InstallTest, CheckPackageMetadata_ab_post_timestamp) { + std::string device = android::base::GetProperty("ro.product.device", ""); + ASSERT_NE("", device); + + // post timestamp is required for upgrade. + std::string metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); + + // post timestamp should be larger than the timestamp on device. + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "post-timestamp=0", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); + + // fingerprint is required for downgrade + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "post-timestamp=0", + "ota-downgrade=yes", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, INSTALL_ERROR); + + std::string finger_print = android::base::GetProperty("ro.build.fingerprint", ""); + ASSERT_NE("", finger_print); + + metadata = android::base::Join( + std::vector{ + "ota-type=AB", + "pre-device=" + device, + "post-timestamp=0", + "pre-build=" + finger_print, + "ota-downgrade=yes", + }, + "\n"); + test_check_package_metadata(metadata, OtaType::AB, 0); +} -- cgit v1.2.3 From 44820ac1e31ffa029ab5baa71238a11b6db3e6cc Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 30 Oct 2018 23:34:50 -0700 Subject: minui: Add a protected GRSurface ctor. This prepares for the removal of the default and copy ctors, by making GRSurface::Create() as the only way to get GRSurface instances. Test: mmma -j bootable/recovery Test: Run recovery_unit_test on marlin. Change-Id: I0c34c3f3967e252deb020907c83acbac8a8f36b9 --- minui/include/minui/minui.h | 13 ++-- minui/resources.cpp | 137 +++++++++++++++++------------------------- tests/unit/minui_test.cpp | 2 +- tests/unit/screen_ui_test.cpp | 41 ++++++------- 4 files changed, 86 insertions(+), 107 deletions(-) diff --git a/minui/include/minui/minui.h b/minui/include/minui/minui.h index e9bd1c4f1..66d992b93 100644 --- a/minui/include/minui/minui.h +++ b/minui/include/minui/minui.h @@ -33,10 +33,11 @@ class GRSurface { GRSurface() = default; virtual ~GRSurface(); - // Creates and returns a GRSurface instance for the given data_size. The starting address of the - // surface data is aligned to SURFACE_DATA_ALIGNMENT. Returns the created GRSurface instance (in - // std::unique_ptr), or nullptr on error. - static std::unique_ptr Create(size_t data_size); + // Creates and returns a GRSurface instance that's sufficient for storing an image of the given + // size. The starting address of the surface data is aligned to SURFACE_DATA_ALIGNMENT. Returns + // the created GRSurface instance (in std::unique_ptr), or nullptr on error. + static std::unique_ptr Create(int width, int height, int row_bytes, int pixel_bytes, + size_t data_size); virtual uint8_t* data() { return data_; @@ -51,6 +52,10 @@ class GRSurface { int row_bytes; int pixel_bytes; + protected: + GRSurface(int width, int height, int row_bytes, int pixel_bytes) + : width(width), height(height), row_bytes(row_bytes), pixel_bytes(pixel_bytes) {} + private: uint8_t* data_{ nullptr }; }; diff --git a/minui/resources.cpp b/minui/resources.cpp index c01c1868e..9c9af0242 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -39,9 +39,11 @@ static std::string g_resource_dir{ "/res/images" }; -std::unique_ptr GRSurface::Create(size_t data_size) { +std::unique_ptr GRSurface::Create(int width, int height, int row_bytes, int pixel_bytes, + size_t data_size) { static constexpr size_t kSurfaceDataAlignment = 8; - std::unique_ptr result = std::make_unique(); + // Cannot use std::make_unique to access non-public ctor. + auto result = std::unique_ptr(new GRSurface(width, height, row_bytes, pixel_bytes)); size_t aligned_size = (data_size + kSurfaceDataAlignment - 1) / kSurfaceDataAlignment * kSurfaceDataAlignment; result->data_ = static_cast(aligned_alloc(kSurfaceDataAlignment, aligned_size)); @@ -68,7 +70,7 @@ PngHandler::PngHandler(const std::string& name) { return; } - unsigned char header[8]; + uint8_t header[8]; size_t bytesRead = fread(header, 1, sizeof(header), png_fp_.get()); if (bytesRead != sizeof(header)) { error_code_ = -2; @@ -131,70 +133,49 @@ PngHandler::~PngHandler() { } } -// "display" surfaces are transformed into the framebuffer's required -// pixel format (currently only RGBX is supported) at load time, so -// gr_blit() can be nothing more than a memcpy() for each row. The -// next two functions are the only ones that know anything about the -// framebuffer pixel format; they need to be modified if the -// framebuffer format changes (but nothing else should). - -// Allocates and returns a GRSurface* sufficient for storing an image of the indicated size in the -// framebuffer pixel format. -static std::unique_ptr init_display_surface(png_uint_32 width, png_uint_32 height) { - std::unique_ptr surface = GRSurface::Create(width * height * 4); - if (!surface) return nullptr; - - surface->width = width; - surface->height = height; - surface->row_bytes = width * 4; - surface->pixel_bytes = 4; - - return surface; -} +// "display" surfaces are transformed into the framebuffer's required pixel format (currently only +// RGBX is supported) at load time, so gr_blit() can be nothing more than a memcpy() for each row. -// Copy 'input_row' to 'output_row', transforming it to the -// framebuffer pixel format. The input format depends on the value of -// 'channels': +// Copies 'input_row' to 'output_row', transforming it to the framebuffer pixel format. The input +// format depends on the value of 'channels': // // 1 - input is 8-bit grayscale // 3 - input is 24-bit RGB // 4 - input is 32-bit RGBA/RGBX // // 'width' is the number of pixels in the row. -static void transform_rgb_to_draw(unsigned char* input_row, - unsigned char* output_row, - int channels, int width) { - int x; - unsigned char* ip = input_row; - unsigned char* op = output_row; - - switch (channels) { - case 1: - // expand gray level to RGBX - for (x = 0; x < width; ++x) { - *op++ = *ip; - *op++ = *ip; - *op++ = *ip; - *op++ = 0xff; - ip++; - } - break; - - case 3: - // expand RGBA to RGBX - for (x = 0; x < width; ++x) { - *op++ = *ip++; - *op++ = *ip++; - *op++ = *ip++; - *op++ = 0xff; - } - break; - - case 4: - // copy RGBA to RGBX - memcpy(output_row, input_row, width*4); - break; - } +static void TransformRgbToDraw(const uint8_t* input_row, uint8_t* output_row, int channels, + int width) { + const uint8_t* ip = input_row; + uint8_t* op = output_row; + + switch (channels) { + case 1: + // expand gray level to RGBX + for (int x = 0; x < width; ++x) { + *op++ = *ip; + *op++ = *ip; + *op++ = *ip; + *op++ = 0xff; + ip++; + } + break; + + case 3: + // expand RGBA to RGBX + for (int x = 0; x < width; ++x) { + *op++ = *ip++; + *op++ = *ip++; + *op++ = *ip++; + *op++ = 0xff; + } + break; + + case 4: + // copy RGBA to RGBX + memcpy(output_row, input_row, width * 4); + break; + } } int res_create_display_surface(const char* name, GRSurface** pSurface) { @@ -207,7 +188,7 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - std::unique_ptr surface = init_display_surface(width, height); + auto surface = GRSurface::Create(width, height, width * 4, 4, width * height * 4); if (!surface) { return -8; } @@ -218,10 +199,10 @@ int res_create_display_surface(const char* name, GRSurface** pSurface) { } for (png_uint_32 y = 0; y < height; ++y) { - std::vector p_row(width * 4); + std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); - transform_rgb_to_draw(p_row.data(), surface->data() + y * surface->row_bytes, - png_handler.channels(), width); + TransformRgbToDraw(p_row.data(), surface->data() + y * surface->row_bytes, + png_handler.channels(), width); } *pSurface = surface.release(); @@ -277,7 +258,9 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, goto exit; } for (int i = 0; i < *frames; ++i) { - auto created_surface = init_display_surface(width, height / *frames); + auto height_per_frame = height / *frames; + auto created_surface = + GRSurface::Create(width, height_per_frame, width * 4, 4, width * height_per_frame); if (!created_surface) { result = -8; goto exit; @@ -290,11 +273,11 @@ int res_create_multi_display_surface(const char* name, int* frames, int* fps, } for (png_uint_32 y = 0; y < height; ++y) { - std::vector p_row(width * 4); + std::vector p_row(width * 4); png_read_row(png_ptr, p_row.data(), nullptr); int frame = y % *frames; - unsigned char* out_row = surface[frame]->data() + (y / *frames) * surface[frame]->row_bytes; - transform_rgb_to_draw(p_row.data(), out_row, png_handler.channels(), width); + uint8_t* out_row = surface[frame]->data() + (y / *frames) * surface[frame]->row_bytes; + TransformRgbToDraw(p_row.data(), out_row, png_handler.channels(), width); } *pSurface = surface; @@ -325,14 +308,10 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { png_uint_32 width = png_handler.width(); png_uint_32 height = png_handler.height(); - std::unique_ptr surface = GRSurface::Create(width * height); + auto surface = GRSurface::Create(width, height, width, 1, width * height); if (!surface) { return -8; } - surface->width = width; - surface->height = height; - surface->row_bytes = width; - surface->pixel_bytes = 1; PixelFormat pixel_format = gr_pixel_format(); if (pixel_format == PixelFormat::ABGR || pixel_format == PixelFormat::BGRA) { @@ -340,7 +319,7 @@ int res_create_alpha_surface(const char* name, GRSurface** pSurface) { } for (png_uint_32 y = 0; y < height; ++y) { - unsigned char* p_row = surface->data() + y * surface->row_bytes; + uint8_t* p_row = surface->data() + y * surface->row_bytes; png_read_row(png_ptr, p_row, nullptr); } @@ -389,7 +368,7 @@ std::vector get_locales_in_png(const std::string& png_name) { } std::vector result; - std::vector row(png_handler.width()); + std::vector row(png_handler.width()); for (png_uint_32 y = 0; y < png_handler.height(); ++y) { png_read_row(png_handler.png_ptr(), row.data(), nullptr); int h = (row[3] << 8) | row[2]; @@ -425,7 +404,7 @@ int res_create_localized_alpha_surface(const char* name, png_uint_32 height = png_handler.height(); for (png_uint_32 y = 0; y < height; ++y) { - std::vector row(width); + std::vector row(width); png_read_row(png_ptr, row.data(), nullptr); int w = (row[1] << 8) | row[0]; int h = (row[3] << 8) | row[2]; @@ -435,14 +414,10 @@ int res_create_localized_alpha_surface(const char* name, if (y + 1 + h >= height || matches_locale(loc, locale)) { printf(" %20s: %s (%d x %d @ %d)\n", name, loc, w, h, y); - std::unique_ptr surface = GRSurface::Create(w * h); + auto surface = GRSurface::Create(w, h, w, 1, w * h); if (!surface) { return -8; } - surface->width = w; - surface->height = h; - surface->row_bytes = w; - surface->pixel_bytes = 1; for (int i = 0; i < h; ++i, ++y) { png_read_row(png_ptr, row.data(), nullptr); diff --git a/tests/unit/minui_test.cpp b/tests/unit/minui_test.cpp index cad6a3d79..b188b5992 100644 --- a/tests/unit/minui_test.cpp +++ b/tests/unit/minui_test.cpp @@ -25,7 +25,7 @@ TEST(GRSurfaceTest, Create_aligned) { static constexpr size_t kSurfaceDataAlignment = 8; for (size_t data_size = 100; data_size < 128; data_size++) { - std::unique_ptr surface(GRSurface::Create(data_size)); + auto surface = GRSurface::Create(10, 1, 10, 1, data_size); ASSERT_TRUE(surface); ASSERT_EQ(0, reinterpret_cast(surface->data()) % kSurfaceDataAlignment); } diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp index 0014e45f1..2b76944b1 100644 --- a/tests/unit/screen_ui_test.cpp +++ b/tests/unit/screen_ui_test.cpp @@ -69,17 +69,6 @@ class ScreenUITest : public testing::Test { MockDrawFunctions draw_funcs_; }; -// TODO(xunchang) Create a constructor. -static GRSurface CreateFakeGRSurface(int width, int height, int row_bytes, int pixel_bytes) { - GRSurface fake_surface; - fake_surface.width = width; - fake_surface.height = height; - fake_surface.row_bytes = row_bytes; - fake_surface.pixel_bytes = pixel_bytes; - - return fake_surface; -} - TEST_F(ScreenUITest, StartPhoneMenuSmoke) { TextMenu menu(false, 10, 20, HEADERS, ITEMS, 0, 20, draw_funcs_); ASSERT_FALSE(menu.scrollable()); @@ -241,9 +230,14 @@ TEST_F(ScreenUITest, WearMenuSelectItemsOverflow) { } TEST_F(ScreenUITest, GraphicMenuSelection) { - auto fake_surface = CreateFakeGRSurface(50, 50, 50, 1); - std::vector items = { &fake_surface, &fake_surface, &fake_surface }; - GraphicMenu menu(&fake_surface, items, 0, draw_funcs_); + auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); + std::vector items = { + item.get(), + item.get(), + item.get(), + }; + GraphicMenu menu(header.get(), items, 0, draw_funcs_); ASSERT_EQ(0, menu.selection()); @@ -263,18 +257,23 @@ TEST_F(ScreenUITest, GraphicMenuSelection) { } TEST_F(ScreenUITest, GraphicMenuValidate) { - auto fake_surface = CreateFakeGRSurface(50, 50, 50, 1); - std::vector items = { &fake_surface, &fake_surface, &fake_surface }; + auto header = GRSurface::Create(50, 50, 50, 1, 50 * 50); + auto item = GRSurface::Create(50, 50, 50, 1, 50 * 50); + std::vector items = { + item.get(), + item.get(), + item.get(), + }; - ASSERT_TRUE(GraphicMenu::Validate(200, 200, &fake_surface, items)); + ASSERT_TRUE(GraphicMenu::Validate(200, 200, header.get(), items)); // Menu exceeds the horizontal boundary. - auto wide_surface = CreateFakeGRSurface(300, 50, 300, 1); - ASSERT_FALSE(GraphicMenu::Validate(299, 200, &wide_surface, items)); + auto wide_surface = GRSurface::Create(300, 50, 300, 1, 300 * 50); + ASSERT_FALSE(GraphicMenu::Validate(299, 200, wide_surface.get(), items)); // Menu exceeds the vertical boundary. - items.push_back(&fake_surface); - ASSERT_FALSE(GraphicMenu::Validate(200, 249, &fake_surface, items)); + items.push_back(item.get()); + ASSERT_FALSE(GraphicMenu::Validate(200, 249, header.get(), items)); } static constexpr int kMagicAction = 101; -- cgit v1.2.3 From 4a22b28bea62852288e481f8e901939df94f87af Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 31 Oct 2018 00:00:31 -0700 Subject: minui: Refactor GRSurfaceFbdev. - Adds Create() that returns a GRSurfaceFbdev instance. - Moves away from using the copy ctor (precisely assignment operator) of GRSurfaceFbdev. - Moves the GRSurfaceFbdev deallocation code into GRSurfaceFbdev's dtor. - Manages MinuiBackendFbdev::gr_framebuffer with std::unique_ptr. Test: mmma -j bootable/recovery Test: `Run graphics test` on taimen. Change-Id: I8e67cda7bc3a2feec0790124d035caa36fb58a89 --- minui/graphics_fbdev.cpp | 68 +++++++++++++++++++++++------------------------- minui/graphics_fbdev.h | 29 +++++++++++++++------ 2 files changed, 53 insertions(+), 44 deletions(-) diff --git a/minui/graphics_fbdev.cpp b/minui/graphics_fbdev.cpp index f958d62d4..4da5613af 100644 --- a/minui/graphics_fbdev.cpp +++ b/minui/graphics_fbdev.cpp @@ -26,21 +26,27 @@ #include #include +#include + #include "minui/minui.h" -MinuiBackendFbdev::MinuiBackendFbdev() : gr_draw(nullptr), fb_fd(-1) {} +std::unique_ptr GRSurfaceFbdev::Create(int width, int height, int row_bytes, + int pixel_bytes) { + // Cannot use std::make_unique to access non-public ctor. + return std::unique_ptr(new GRSurfaceFbdev(width, height, row_bytes, pixel_bytes)); +} void MinuiBackendFbdev::Blank(bool blank) { int ret = ioctl(fb_fd, FBIOBLANK, blank ? FB_BLANK_POWERDOWN : FB_BLANK_UNBLANK); if (ret < 0) perror("ioctl(): blank"); } -void MinuiBackendFbdev::SetDisplayedFramebuffer(unsigned n) { +void MinuiBackendFbdev::SetDisplayedFramebuffer(size_t n) { if (n > 1 || !double_buffered) return; - vi.yres_virtual = gr_framebuffer[0].height * 2; - vi.yoffset = n * gr_framebuffer[0].height; - vi.bits_per_pixel = gr_framebuffer[0].pixel_bytes * 8; + vi.yres_virtual = gr_framebuffer[0]->height * 2; + vi.yoffset = n * gr_framebuffer[0]->height; + vi.bits_per_pixel = gr_framebuffer[0]->pixel_bytes * 8; if (ioctl(fb_fd, FBIOPUT_VSCREENINFO, &vi) < 0) { perror("active fb swap failed"); } @@ -96,35 +102,31 @@ GRSurface* MinuiBackendFbdev::Init() { memset(bits, 0, fi.smem_len); - gr_framebuffer[0].width = vi.xres; - gr_framebuffer[0].height = vi.yres; - gr_framebuffer[0].row_bytes = fi.line_length; - gr_framebuffer[0].pixel_bytes = vi.bits_per_pixel / 8; - gr_framebuffer[0].buffer_ = static_cast(bits); - memset(gr_framebuffer[0].buffer_, 0, gr_framebuffer[0].height * gr_framebuffer[0].row_bytes); + gr_framebuffer[0] = + GRSurfaceFbdev::Create(vi.xres, vi.yres, fi.line_length, vi.bits_per_pixel / 8); + gr_framebuffer[0]->buffer_ = static_cast(bits); + memset(gr_framebuffer[0]->buffer_, 0, gr_framebuffer[0]->height * gr_framebuffer[0]->row_bytes); + + gr_framebuffer[1] = + GRSurfaceFbdev::Create(gr_framebuffer[0]->width, gr_framebuffer[0]->height, + gr_framebuffer[0]->row_bytes, gr_framebuffer[0]->pixel_bytes); /* check if we can use double buffering */ if (vi.yres * fi.line_length * 2 <= fi.smem_len) { double_buffered = true; - gr_framebuffer[1] = gr_framebuffer[0]; - gr_framebuffer[1].buffer_ = - gr_framebuffer[0].buffer_ + gr_framebuffer[0].height * gr_framebuffer[0].row_bytes; - - gr_draw = gr_framebuffer + 1; - + gr_framebuffer[1]->buffer_ = + gr_framebuffer[0]->buffer_ + gr_framebuffer[0]->height * gr_framebuffer[0]->row_bytes; } else { double_buffered = false; - // Without double-buffering, we allocate RAM for a buffer to - // draw in, and then "flipping" the buffer consists of a - // memcpy from the buffer we allocated to the framebuffer. - - gr_draw = new GRSurfaceFbdev; - *gr_draw = gr_framebuffer[0]; - gr_draw->buffer_ = new uint8_t[gr_draw->height * gr_draw->row_bytes]; + // Without double-buffering, we allocate RAM for a buffer to draw in, and then "flipping" the + // buffer consists of a memcpy from the buffer we allocated to the framebuffer. + memory_buffer.resize(gr_framebuffer[1]->height * gr_framebuffer[1]->row_bytes); + gr_framebuffer[1]->buffer_ = memory_buffer.data(); } + gr_draw = gr_framebuffer[1].get(); memset(gr_draw->buffer_, 0, gr_draw->height * gr_draw->row_bytes); fb_fd = fd; SetDisplayedFramebuffer(0); @@ -139,25 +141,19 @@ GRSurface* MinuiBackendFbdev::Init() { GRSurface* MinuiBackendFbdev::Flip() { if (double_buffered) { - // Change gr_draw to point to the buffer currently displayed, - // then flip the driver so we're displaying the other buffer - // instead. - gr_draw = gr_framebuffer + displayed_buffer; + // Change gr_draw to point to the buffer currently displayed, then flip the driver so we're + // displaying the other buffer instead. + gr_draw = gr_framebuffer[displayed_buffer].get(); SetDisplayedFramebuffer(1 - displayed_buffer); } else { // Copy from the in-memory surface to the framebuffer. - memcpy(gr_framebuffer[0].buffer_, gr_draw->buffer_, gr_draw->height * gr_draw->row_bytes); + memcpy(gr_framebuffer[0]->buffer_, gr_draw->buffer_, gr_draw->height * gr_draw->row_bytes); } return gr_draw; } MinuiBackendFbdev::~MinuiBackendFbdev() { - close(fb_fd); - fb_fd = -1; - - if (!double_buffered && gr_draw) { - delete[] gr_draw->buffer_; - delete gr_draw; + if (fb_fd != -1) { + close(fb_fd); } - gr_draw = nullptr; } diff --git a/minui/graphics_fbdev.h b/minui/graphics_fbdev.h index be813dccb..934e584d7 100644 --- a/minui/graphics_fbdev.h +++ b/minui/graphics_fbdev.h @@ -19,37 +19,50 @@ #include #include +#include +#include + #include "graphics.h" #include "minui/minui.h" class GRSurfaceFbdev : public GRSurface { public: + // Creates and returns a GRSurfaceFbdev instance, or nullptr on error. + static std::unique_ptr Create(int width, int height, int row_bytes, + int pixel_bytes); + uint8_t* data() override { return buffer_; } + protected: + using GRSurface::GRSurface; + private: friend class MinuiBackendFbdev; // Points to the start of the buffer: either the mmap'd framebuffer or one allocated in-memory. - uint8_t* buffer_; + uint8_t* buffer_{ nullptr }; }; class MinuiBackendFbdev : public MinuiBackend { public: + MinuiBackendFbdev() = default; + ~MinuiBackendFbdev() override; + GRSurface* Init() override; GRSurface* Flip() override; void Blank(bool) override; - ~MinuiBackendFbdev() override; - MinuiBackendFbdev(); private: - void SetDisplayedFramebuffer(unsigned n); + void SetDisplayedFramebuffer(size_t n); - GRSurfaceFbdev gr_framebuffer[2]; + std::unique_ptr gr_framebuffer[2]; + // Points to the current surface (i.e. one of the two gr_framebuffer's). + GRSurfaceFbdev* gr_draw{ nullptr }; bool double_buffered; - GRSurfaceFbdev* gr_draw; - int displayed_buffer; + std::vector memory_buffer; + size_t displayed_buffer{ 0 }; fb_var_screeninfo vi; - int fb_fd; + int fb_fd{ -1 }; }; -- cgit v1.2.3 From 1b18cf56e21d038fcb03d09a13c90545cb1b6501 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 31 Oct 2018 00:12:11 -0700 Subject: minui: Refactor GRSurfaceAdf. Test: mmma -j bootable/recovery Change-Id: I14514017aace4b7043a9db1f5a93ec130a6f89c4 --- minui/graphics_adf.cpp | 89 +++++++++++++++++++++++++------------------------- minui/graphics_adf.h | 32 +++++++++++------- 2 files changed, 65 insertions(+), 56 deletions(-) diff --git a/minui/graphics_adf.cpp b/minui/graphics_adf.cpp index 6fc193f74..9eea497d6 100644 --- a/minui/graphics_adf.cpp +++ b/minui/graphics_adf.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -28,51 +29,60 @@ #include "minui/minui.h" -MinuiBackendAdf::MinuiBackendAdf() - : intf_fd(-1), dev(), current_surface(0), n_surfaces(0), surfaces() {} +GRSurfaceAdf::~GRSurfaceAdf() { + if (mmapped_buffer_) { + munmap(mmapped_buffer_, pitch * height); + } + if (fence_fd != -1) { + close(fence_fd); + } + if (fd != -1) { + close(fd); + } +} -int MinuiBackendAdf::SurfaceInit(const drm_mode_modeinfo* mode, GRSurfaceAdf* surf) { - *surf = {}; - surf->fence_fd = -1; - surf->fd = adf_interface_simple_buffer_alloc(intf_fd, mode->hdisplay, mode->vdisplay, format, - &surf->offset, &surf->pitch); - if (surf->fd < 0) { - return surf->fd; +std::unique_ptr GRSurfaceAdf::Create(int intf_fd, const drm_mode_modeinfo* mode, + __u32 format, int* err) { + __u32 offset; + __u32 pitch; + auto fd = adf_interface_simple_buffer_alloc(intf_fd, mode->hdisplay, mode->vdisplay, format, + &offset, &pitch); + + if (fd < 0) { + *err = fd; + return nullptr; } - surf->width = mode->hdisplay; - surf->height = mode->vdisplay; - surf->row_bytes = surf->pitch; - surf->pixel_bytes = (format == DRM_FORMAT_RGB565) ? 2 : 4; + std::unique_ptr surf = std::unique_ptr( + new GRSurfaceAdf(mode->hdisplay, mode->vdisplay, pitch, (format == DRM_FORMAT_RGB565 ? 2 : 4), + offset, pitch, fd)); auto mmapped = mmap(nullptr, surf->pitch * surf->height, PROT_WRITE, MAP_SHARED, surf->fd, surf->offset); if (mmapped == MAP_FAILED) { - int saved_errno = errno; - close(surf->fd); - return -saved_errno; + *err = -errno; + return nullptr; } surf->mmapped_buffer_ = static_cast(mmapped); - return 0; + return surf; } +MinuiBackendAdf::MinuiBackendAdf() : intf_fd(-1), dev(), current_surface(0), n_surfaces(0) {} + int MinuiBackendAdf::InterfaceInit() { adf_interface_data intf_data; - int err = adf_get_interface_data(intf_fd, &intf_data); - if (err < 0) return err; + if (int err = adf_get_interface_data(intf_fd, &intf_data); err < 0) return err; - int ret = 0; - err = SurfaceInit(&intf_data.current_mode, &surfaces[0]); - if (err < 0) { - fprintf(stderr, "allocating surface 0 failed: %s\n", strerror(-err)); - ret = err; + int result = 0; + surfaces[0] = GRSurfaceAdf::Create(intf_fd, &intf_data.current_mode, format, &result); + if (!surfaces[0]) { + fprintf(stderr, "Failed to allocate surface 0: %s\n", strerror(-result)); goto done; } - err = SurfaceInit(&intf_data.current_mode, &surfaces[1]); - if (err < 0) { - fprintf(stderr, "allocating surface 1 failed: %s\n", strerror(-err)); - surfaces[1] = {}; + surfaces[1] = GRSurfaceAdf::Create(intf_fd, &intf_data.current_mode, format, &result); + if (!surfaces[1]) { + fprintf(stderr, "Failed to allocate surface 1: %s\n", strerror(-result)); n_surfaces = 1; } else { n_surfaces = 2; @@ -80,7 +90,7 @@ int MinuiBackendAdf::InterfaceInit() { done: adf_free_interface_data(&intf_data); - return ret; + return result; } int MinuiBackendAdf::DeviceInit(adf_device* dev) { @@ -153,12 +163,12 @@ GRSurface* MinuiBackendAdf::Init() { } void MinuiBackendAdf::Sync(GRSurfaceAdf* surf) { - static constexpr unsigned int warningTimeout = 3000; + static constexpr unsigned int kWarningTimeout = 3000; if (surf == nullptr) return; if (surf->fence_fd >= 0) { - int err = sync_wait(surf->fence_fd, warningTimeout); + int err = sync_wait(surf->fence_fd, kWarningTimeout); if (err < 0) { perror("adf sync fence wait error\n"); } @@ -169,33 +179,22 @@ void MinuiBackendAdf::Sync(GRSurfaceAdf* surf) { } GRSurface* MinuiBackendAdf::Flip() { - GRSurfaceAdf* surf = &surfaces[current_surface]; + const auto& surf = surfaces[current_surface]; int fence_fd = adf_interface_simple_post(intf_fd, eng_id, surf->width, surf->height, format, surf->fd, surf->offset, surf->pitch, -1); if (fence_fd >= 0) surf->fence_fd = fence_fd; current_surface = (current_surface + 1) % n_surfaces; - Sync(&surfaces[current_surface]); - return &surfaces[current_surface]; + Sync(surfaces[current_surface].get()); + return surfaces[current_surface].get(); } void MinuiBackendAdf::Blank(bool blank) { adf_interface_blank(intf_fd, blank ? DRM_MODE_DPMS_OFF : DRM_MODE_DPMS_ON); } -void MinuiBackendAdf::SurfaceDestroy(GRSurfaceAdf* surf) { - if (surf->mmapped_buffer_) { - munmap(surf->mmapped_buffer_, surf->pitch * surf->height); - } - close(surf->fence_fd); - close(surf->fd); -} - MinuiBackendAdf::~MinuiBackendAdf() { adf_device_close(&dev); - for (unsigned int i = 0; i < n_surfaces; i++) { - SurfaceDestroy(&surfaces[i]); - } if (intf_fd >= 0) close(intf_fd); } diff --git a/minui/graphics_adf.h b/minui/graphics_adf.h index 099d32962..bf9842878 100644 --- a/minui/graphics_adf.h +++ b/minui/graphics_adf.h @@ -17,6 +17,9 @@ #pragma once #include +#include + +#include #include @@ -25,6 +28,11 @@ class GRSurfaceAdf : public GRSurface { public: + ~GRSurfaceAdf() override; + + static std::unique_ptr Create(int intf_fd, const drm_mode_modeinfo* mode, + __u32 format, int* err); + uint8_t* data() override { return mmapped_buffer_; } @@ -32,34 +40,36 @@ class GRSurfaceAdf : public GRSurface { private: friend class MinuiBackendAdf; - int fence_fd; - int fd; - __u32 offset; - __u32 pitch; + GRSurfaceAdf(int width, int height, int row_bytes, int pixel_bytes, __u32 offset, __u32 pitch, + int fd) + : GRSurface(width, height, row_bytes, pixel_bytes), offset(offset), pitch(pitch), fd(fd) {} + + const __u32 offset; + const __u32 pitch; + int fd; + int fence_fd{ -1 }; uint8_t* mmapped_buffer_{ nullptr }; }; class MinuiBackendAdf : public MinuiBackend { public: + MinuiBackendAdf(); + ~MinuiBackendAdf() override; GRSurface* Init() override; GRSurface* Flip() override; void Blank(bool) override; - ~MinuiBackendAdf() override; - MinuiBackendAdf(); private: - int SurfaceInit(const drm_mode_modeinfo* mode, GRSurfaceAdf* surf); int InterfaceInit(); int DeviceInit(adf_device* dev); - void SurfaceDestroy(GRSurfaceAdf* surf); void Sync(GRSurfaceAdf* surf); int intf_fd; adf_id_t eng_id; __u32 format; adf_device dev; - unsigned int current_surface; - unsigned int n_surfaces; - GRSurfaceAdf surfaces[2]; + size_t current_surface; + size_t n_surfaces; + std::unique_ptr surfaces[2]; }; -- cgit v1.2.3 From 710bc535f45570428f0c7b36bb4f20744247cb38 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 24 Oct 2018 07:59:49 -0700 Subject: minui: Remove the default and copy ctors for GRSurface. As well as all the derived classes. Instances must be created with Create(). A default copy ctor would mess up the ownership of the mapped or allocated buffer in these classes, so that has been explicitly removed. Test: mmma -j bootable/recovery Test: Run recovery_unit_test on marlin. Test: `Run graphics test` on blueline. Change-Id: I69ce001a9ec9e3ac851edb6ec4d3fa11f4aaea08 --- minui/graphics_drm.cpp | 10 +++------- minui/graphics_drm.h | 7 +++---- minui/include/minui/minui.h | 5 ++++- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/minui/graphics_drm.cpp b/minui/graphics_drm.cpp index f81fd9d5c..765e2625a 100644 --- a/minui/graphics_drm.cpp +++ b/minui/graphics_drm.cpp @@ -102,15 +102,15 @@ std::unique_ptr GRSurfaceDrm::Create(int drm_fd, int width, int he return nullptr; } - std::unique_ptr surface = std::make_unique(drm_fd); - surface->handle = create_dumb.handle; + // Cannot use std::make_unique to access non-public ctor. + auto surface = std::unique_ptr(new GRSurfaceDrm( + width, height, create_dumb.pitch, create_dumb.bpp / 8, drm_fd, create_dumb.handle)); uint32_t handles[4], pitches[4], offsets[4]; handles[0] = surface->handle; pitches[0] = create_dumb.pitch; offsets[0] = 0; - if (drmModeAddFB2(drm_fd, width, height, format, handles, pitches, offsets, &surface->fb_id, 0) != 0) { perror("Failed to drmModeAddFB2"); @@ -124,10 +124,6 @@ std::unique_ptr GRSurfaceDrm::Create(int drm_fd, int width, int he return nullptr; } - surface->height = height; - surface->width = width; - surface->row_bytes = create_dumb.pitch; - surface->pixel_bytes = create_dumb.bpp / 8; auto mmapped = mmap(nullptr, surface->height * surface->row_bytes, PROT_READ | PROT_WRITE, MAP_SHARED, drm_fd, map_dumb.offset); if (mmapped == MAP_FAILED) { diff --git a/minui/graphics_drm.h b/minui/graphics_drm.h index 02db89f05..6ba46e60b 100644 --- a/minui/graphics_drm.h +++ b/minui/graphics_drm.h @@ -20,7 +20,6 @@ #include -#include #include #include "graphics.h" @@ -28,7 +27,6 @@ class GRSurfaceDrm : public GRSurface { public: - explicit GRSurfaceDrm(int drm_fd) : drm_fd_(drm_fd) {} ~GRSurfaceDrm() override; // Creates a GRSurfaceDrm instance. @@ -41,13 +39,14 @@ class GRSurfaceDrm : public GRSurface { private: friend class MinuiBackendDrm; + GRSurfaceDrm(int width, int height, int row_bytes, int pixel_bytes, int drm_fd, uint32_t handle) + : GRSurface(width, height, row_bytes, pixel_bytes), drm_fd_(drm_fd), handle(handle) {} + const int drm_fd_; uint32_t fb_id{ 0 }; uint32_t handle{ 0 }; uint8_t* mmapped_buffer_{ nullptr }; - - DISALLOW_COPY_AND_ASSIGN(GRSurfaceDrm); }; class MinuiBackendDrm : public MinuiBackend { diff --git a/minui/include/minui/minui.h b/minui/include/minui/minui.h index 66d992b93..d6881e9a0 100644 --- a/minui/include/minui/minui.h +++ b/minui/include/minui/minui.h @@ -24,13 +24,14 @@ #include #include +#include + // // Graphics. // class GRSurface { public: - GRSurface() = default; virtual ~GRSurface(); // Creates and returns a GRSurface instance that's sufficient for storing an image of the given @@ -58,6 +59,8 @@ class GRSurface { private: uint8_t* data_{ nullptr }; + + DISALLOW_COPY_AND_ASSIGN(GRSurface); }; struct GRFont { -- cgit v1.2.3 From 929e481e7146a9ed926b5d8d9414a68fc8077d7c Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 6 Jun 2018 14:28:35 -0700 Subject: tests: Use FRIEND_TEST in ScreenRecoveryUITest. Test: Run recovery_unit_test on marlin. Change-Id: I93ec6df8c056b2c485200822f18db0b852595242 --- tests/unit/screen_ui_test.cpp | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/tests/unit/screen_ui_test.cpp b/tests/unit/screen_ui_test.cpp index 2b76944b1..3246e6a6e 100644 --- a/tests/unit/screen_ui_test.cpp +++ b/tests/unit/screen_ui_test.cpp @@ -27,6 +27,7 @@ #include #include #include +#include #include "common/test_constants.h" #include "device.h" @@ -306,24 +307,13 @@ class TestableScreenRecoveryUI : public ScreenRecoveryUI { int KeyHandler(int key, bool visible) const; - // The following functions expose the protected members for test purpose. - void RunLoadAnimation() { - LoadAnimation(); - } - - size_t GetLoopFrames() const { - return loop_frames; - } - - size_t GetIntroFrames() const { - return intro_frames; - } - - bool GetRtlLocale() const { - return rtl_locale_; - } - private: + FRIEND_TEST(ScreenRecoveryUITest, Init); + FRIEND_TEST(ScreenRecoveryUITest, RtlLocale); + FRIEND_TEST(ScreenRecoveryUITest, RtlLocaleWithSuffix); + FRIEND_TEST(ScreenRecoveryUITest, LoadAnimation); + FRIEND_TEST(ScreenRecoveryUITest, LoadAnimation_MissingAnimation); + std::vector key_buffer_; size_t key_buffer_index_; }; @@ -387,7 +377,7 @@ TEST_F(ScreenRecoveryUITest, Init) { ASSERT_TRUE(ui_->Init(kTestLocale)); ASSERT_EQ(kTestLocale, ui_->GetLocale()); - ASSERT_FALSE(ui_->GetRtlLocale()); + ASSERT_FALSE(ui_->rtl_locale_); ASSERT_FALSE(ui_->IsTextVisible()); ASSERT_FALSE(ui_->WasTextEverVisible()); } @@ -415,14 +405,14 @@ TEST_F(ScreenRecoveryUITest, RtlLocale) { RETURN_IF_NO_GRAPHICS; ASSERT_TRUE(ui_->Init(kTestRtlLocale)); - ASSERT_TRUE(ui_->GetRtlLocale()); + ASSERT_TRUE(ui_->rtl_locale_); } TEST_F(ScreenRecoveryUITest, RtlLocaleWithSuffix) { RETURN_IF_NO_GRAPHICS; ASSERT_TRUE(ui_->Init(kTestRtlLocaleWithSuffix)); - ASSERT_TRUE(ui_->GetRtlLocale()); + ASSERT_TRUE(ui_->rtl_locale_); } TEST_F(ScreenRecoveryUITest, ShowMenu) { @@ -547,10 +537,10 @@ TEST_F(ScreenRecoveryUITest, LoadAnimation) { } Paths::Get().set_resource_dir(resource_dir.path); - ui_->RunLoadAnimation(); + ui_->LoadAnimation(); - ASSERT_EQ(2u, ui_->GetIntroFrames()); - ASSERT_EQ(3u, ui_->GetLoopFrames()); + ASSERT_EQ(2u, ui_->intro_frames); + ASSERT_EQ(3u, ui_->loop_frames); for (const auto& name : tempfiles) { ASSERT_EQ(0, unlink(name.c_str())); @@ -566,7 +556,7 @@ TEST_F(ScreenRecoveryUITest, LoadAnimation_MissingAnimation) { Paths::Get().set_resource_dir("/proc/self"); ::testing::FLAGS_gtest_death_test_style = "threadsafe"; - ASSERT_EXIT(ui_->RunLoadAnimation(), ::testing::KilledBySignal(SIGABRT), ""); + ASSERT_EXIT(ui_->LoadAnimation(), ::testing::KilledBySignal(SIGABRT), ""); } #undef RETURN_IF_NO_GRAPHICS -- cgit v1.2.3 From 63b59dceadcce2c31b617e884941bf26b2730eb0 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 31 Oct 2018 15:23:04 -0700 Subject: minui: Add GRSurface::Clone(). Clone() allows duplicating the image that's stored in the GRSurface. Test: Run recovery_unit_test. Change-Id: Ia50d507c6200f2de5f17143775de805247a60e1f --- minui/include/minui/minui.h | 4 ++++ minui/resources.cpp | 10 ++++++++-- tests/unit/minui_test.cpp | 14 +++++++++++++- 3 files changed, 25 insertions(+), 3 deletions(-) diff --git a/minui/include/minui/minui.h b/minui/include/minui/minui.h index d6881e9a0..0b499e621 100644 --- a/minui/include/minui/minui.h +++ b/minui/include/minui/minui.h @@ -40,6 +40,9 @@ class GRSurface { static std::unique_ptr Create(int width, int height, int row_bytes, int pixel_bytes, size_t data_size); + // Clones the current GRSurface instance (i.e. an image). + std::unique_ptr Clone() const; + virtual uint8_t* data() { return data_; } @@ -59,6 +62,7 @@ class GRSurface { private: uint8_t* data_{ nullptr }; + size_t data_size_; DISALLOW_COPY_AND_ASSIGN(GRSurface); }; diff --git a/minui/resources.cpp b/minui/resources.cpp index 9c9af0242..9027bc668 100644 --- a/minui/resources.cpp +++ b/minui/resources.cpp @@ -44,13 +44,19 @@ std::unique_ptr GRSurface::Create(int width, int height, int row_byte static constexpr size_t kSurfaceDataAlignment = 8; // Cannot use std::make_unique to access non-public ctor. auto result = std::unique_ptr(new GRSurface(width, height, row_bytes, pixel_bytes)); - size_t aligned_size = + result->data_size_ = (data_size + kSurfaceDataAlignment - 1) / kSurfaceDataAlignment * kSurfaceDataAlignment; - result->data_ = static_cast(aligned_alloc(kSurfaceDataAlignment, aligned_size)); + result->data_ = static_cast(aligned_alloc(kSurfaceDataAlignment, result->data_size_)); if (result->data_ == nullptr) return nullptr; return result; } +std::unique_ptr GRSurface::Clone() const { + auto result = GRSurface::Create(width, height, row_bytes, pixel_bytes, data_size_); + memcpy(result->data_, data_, data_size_); + return result; +} + GRSurface::~GRSurface() { if (data_ != nullptr) { free(data_); diff --git a/tests/unit/minui_test.cpp b/tests/unit/minui_test.cpp index b188b5992..d68e5e3a1 100644 --- a/tests/unit/minui_test.cpp +++ b/tests/unit/minui_test.cpp @@ -15,8 +15,9 @@ */ #include +#include -#include +#include #include @@ -30,3 +31,14 @@ TEST(GRSurfaceTest, Create_aligned) { ASSERT_EQ(0, reinterpret_cast(surface->data()) % kSurfaceDataAlignment); } } + +TEST(GRSurfaceTest, Clone) { + static constexpr size_t kImageSize = 10 * 50; + auto image = GRSurface::Create(50, 10, 50, 1, kImageSize); + for (auto i = 0; i < kImageSize; i++) { + image->data()[i] = rand() % 128; + } + auto image_copy = image->Clone(); + ASSERT_EQ(std::vector(image->data(), image->data() + kImageSize), + std::vector(image_copy->data(), image_copy->data() + kImageSize)); +} -- cgit v1.2.3 From f65d48bb5ac611a9dbfba150cfdb0bfc5fd6aaf8 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Fri, 2 Nov 2018 09:34:49 -0700 Subject: minui: Use android::base::unique_fd in MinuiBackendFbdev. Test: mmma -j bootable/recovery Test: `Run graphics test` on taimen. Change-Id: I5b25cafbd0107943606a87f0619242cf950174ac --- minui/graphics_fbdev.cpp | 17 +++++------------ minui/graphics_fbdev.h | 6 ++++-- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/minui/graphics_fbdev.cpp b/minui/graphics_fbdev.cpp index 4da5613af..93e4420d3 100644 --- a/minui/graphics_fbdev.cpp +++ b/minui/graphics_fbdev.cpp @@ -28,6 +28,8 @@ #include +#include + #include "minui/minui.h" std::unique_ptr GRSurfaceFbdev::Create(int width, int height, int row_bytes, @@ -54,7 +56,7 @@ void MinuiBackendFbdev::SetDisplayedFramebuffer(size_t n) { } GRSurface* MinuiBackendFbdev::Init() { - int fd = open("/dev/graphics/fb0", O_RDWR); + android::base::unique_fd fd(open("/dev/graphics/fb0", O_RDWR)); if (fd == -1) { perror("cannot open fb0"); return nullptr; @@ -63,13 +65,11 @@ GRSurface* MinuiBackendFbdev::Init() { fb_fix_screeninfo fi; if (ioctl(fd, FBIOGET_FSCREENINFO, &fi) < 0) { perror("failed to get fb0 info"); - close(fd); return nullptr; } if (ioctl(fd, FBIOGET_VSCREENINFO, &vi) < 0) { perror("failed to get fb0 info"); - close(fd); return nullptr; } @@ -96,7 +96,6 @@ GRSurface* MinuiBackendFbdev::Init() { void* bits = mmap(0, fi.smem_len, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0); if (bits == MAP_FAILED) { perror("failed to mmap framebuffer"); - close(fd); return nullptr; } @@ -128,10 +127,10 @@ GRSurface* MinuiBackendFbdev::Init() { gr_draw = gr_framebuffer[1].get(); memset(gr_draw->buffer_, 0, gr_draw->height * gr_draw->row_bytes); - fb_fd = fd; + fb_fd = std::move(fd); SetDisplayedFramebuffer(0); - printf("framebuffer: %d (%d x %d)\n", fb_fd, gr_draw->width, gr_draw->height); + printf("framebuffer: %d (%d x %d)\n", fb_fd.get(), gr_draw->width, gr_draw->height); Blank(true); Blank(false); @@ -151,9 +150,3 @@ GRSurface* MinuiBackendFbdev::Flip() { } return gr_draw; } - -MinuiBackendFbdev::~MinuiBackendFbdev() { - if (fb_fd != -1) { - close(fb_fd); - } -} diff --git a/minui/graphics_fbdev.h b/minui/graphics_fbdev.h index 934e584d7..016ab88bc 100644 --- a/minui/graphics_fbdev.h +++ b/minui/graphics_fbdev.h @@ -22,6 +22,8 @@ #include #include +#include + #include "graphics.h" #include "minui/minui.h" @@ -48,7 +50,7 @@ class GRSurfaceFbdev : public GRSurface { class MinuiBackendFbdev : public MinuiBackend { public: MinuiBackendFbdev() = default; - ~MinuiBackendFbdev() override; + ~MinuiBackendFbdev() override = default; GRSurface* Init() override; GRSurface* Flip() override; @@ -64,5 +66,5 @@ class MinuiBackendFbdev : public MinuiBackend { std::vector memory_buffer; size_t displayed_buffer{ 0 }; fb_var_screeninfo vi; - int fb_fd{ -1 }; + android::base::unique_fd fb_fd; }; -- cgit v1.2.3