fastboot: clean up CheckRequirements

CheckRequirements() had various issues that are cleaned up here,

1) Move from C string parsing to C++
2) Moved from C data structures to C++, including fixing memory leaks.
3) Removed the 'cur_product' global and the 'query_save' function that
   stores it
4) Actually writing tests for the parsing function for
android-info.txt
5) Check that a variable needs to be checked for a given product before
   trying to read it.  Previously, fastboot would fail if a variable
   isn't recognized on a device, even if the check should be ignored.

A lot of flexibility is allowed for the input strings, to keep
backwards compatibility with the previous parsers.

Test: fastboot works, unit tests

Change-Id: Idc3bba8b8fe829d8eefe5f6c495e63a9441c0b60
This commit is contained in:
Tom Cherry 2018-09-05 15:11:44 -07:00
parent dfd85df11a
commit 4aa60b382c
7 changed files with 268 additions and 200 deletions

View file

@ -136,69 +136,6 @@ void fb_resize_partition(const std::string& partition, const std::string& size)
RUN_COMMAND(fb->RawCommand(FB_CMD_RESIZE_PARTITION ":" + partition + ":" + size));
}
static int match(const char* str, const char** value, unsigned count) {
unsigned n;
for (n = 0; n < count; n++) {
const char *val = value[n];
int len = strlen(val);
int match;
if ((len > 1) && (val[len-1] == '*')) {
len--;
match = !strncmp(val, str, len);
} else {
match = !strcmp(val, str);
}
if (match) return 1;
}
return 0;
}
void fb_require(const std::string& product, const std::string& var, bool invert, size_t count,
const char** values) {
Status("Checking '" + var + "'");
double start = now();
std::string var_value;
auto status = fb->GetVar(var, &var_value);
if (status) {
fprintf(stderr, "getvar:%s FAILED (%s)\n", var.c_str(), fb->Error().c_str());
die("requirements not met!");
}
if (!product.empty()) {
if (product != cur_product) {
double split = now();
fprintf(stderr, "IGNORE, product is %s required only for %s [%7.3fs]\n", cur_product,
product.c_str(), (split - start));
return;
}
}
int yes = match(var_value.c_str(), values, count);
if (invert) yes = !yes;
if (yes) {
double split = now();
fprintf(stderr, "OKAY [%7.3fs]\n", (split - start));
return;
}
fprintf(stderr, "FAILED\n\n");
fprintf(stderr, "Device %s is '%s'.\n", var.c_str(), var_value.c_str());
fprintf(stderr, "Update %s '%s'", invert ? "rejects" : "requires", values[0]);
for (size_t n = 1; n < count; n++) {
fprintf(stderr, " or '%s'", values[n]);
}
fprintf(stderr, ".\n\n");
die("requirements not met!");
}
void fb_display(const std::string& label, const std::string& var) {
std::string value;
auto status = fb->GetVar(var, &value);
@ -210,18 +147,6 @@ void fb_display(const std::string& label, const std::string& var) {
fprintf(stderr, "%s: %s\n", label.c_str(), value.c_str());
}
void fb_query_save(const std::string& var, char* dest, uint32_t dest_size) {
std::string value;
auto status = fb->GetVar(var, &value);
if (status) {
fprintf(stderr, "getvar:%s FAILED (%s)\n", var.c_str(), fb->Error().c_str());
return;
}
strncpy(dest, value.c_str(), dest_size);
}
void fb_reboot() {
fprintf(stderr, "Rebooting");
fb->Reboot();

View file

@ -53,10 +53,7 @@ void fb_flash_fd(const std::string& partition, int fd, uint32_t sz);
void fb_flash_sparse(const std::string& partition, struct sparse_file* s, uint32_t sz,
size_t current, size_t total);
void fb_erase(const std::string& partition);
void fb_require(const std::string& prod, const std::string& var, bool invert, size_t nvalues,
const char** values);
void fb_display(const std::string& label, const std::string& var);
void fb_query_save(const std::string& var, char* dest, uint32_t dest_size);
void fb_reboot();
void fb_command(const std::string& cmd, const std::string& msg);
void fb_download(const std::string& name, const std::vector<char>& data);

View file

@ -43,6 +43,7 @@
#include <chrono>
#include <functional>
#include <regex>
#include <thread>
#include <utility>
#include <vector>
@ -70,14 +71,14 @@
#include "usb.h"
using android::base::ReadFully;
using android::base::Split;
using android::base::Trim;
using android::base::unique_fd;
#ifndef O_BINARY
#define O_BINARY 0
#endif
char cur_product[FB_RESPONSE_SZ + 1];
static const char* serial = nullptr;
static bool g_long_listing = false;
@ -588,109 +589,145 @@ static int unzip_to_file(ZipArchiveHandle zip, const char* entry_name) {
return fd.release();
}
static char* strip(char* s) {
while (*s && isspace(*s)) s++;
static void CheckRequirement(const std::string& cur_product, const std::string& var,
const std::string& product, bool invert,
const std::vector<std::string>& options) {
Status("Checking '" + var + "'");
int n = strlen(s);
while (n-- > 0) {
if (!isspace(s[n])) break;
s[n] = 0;
double start = now();
if (!product.empty()) {
if (product != cur_product) {
double split = now();
fprintf(stderr, "IGNORE, product is %s required only for %s [%7.3fs]\n",
cur_product.c_str(), product.c_str(), (split - start));
return;
}
}
return s;
std::string var_value;
if (!fb_getvar(var, &var_value)) {
fprintf(stderr, "FAILED\n\n");
fprintf(stderr, "Could not getvar for '%s' (%s)\n\n", var.c_str(), fb_get_error().c_str());
die("requirements not met!");
}
bool match = false;
for (const auto& option : options) {
if (option == var_value || (option.back() == '*' &&
!var_value.compare(0, option.length() - 1, option, 0,
option.length() - 1))) {
match = true;
break;
}
}
if (invert) {
match = !match;
}
if (match) {
double split = now();
fprintf(stderr, "OKAY [%7.3fs]\n", (split - start));
return;
}
fprintf(stderr, "FAILED\n\n");
fprintf(stderr, "Device %s is '%s'.\n", var.c_str(), var_value.c_str());
fprintf(stderr, "Update %s '%s'", invert ? "rejects" : "requires", options[0].c_str());
for (auto it = std::next(options.begin()); it != options.end(); ++it) {
fprintf(stderr, " or '%s'", it->c_str());
}
fprintf(stderr, ".\n\n");
die("requirements not met!");
}
#define MAX_OPTIONS 32
static void check_requirement(char* line) {
char *val[MAX_OPTIONS];
unsigned count;
char *x;
int invert = 0;
bool ParseRequirementLine(const std::string& line, std::string* name, std::string* product,
bool* invert, std::vector<std::string>* options) {
// "require product=alpha|beta|gamma"
// "require version-bootloader=1234"
// "require-for-product:gamma version-bootloader=istanbul|constantinople"
// "require partition-exists=vendor"
*product = "";
*invert = false;
char* name = line;
const char* product = "";
if (!strncmp(name, "reject ", 7)) {
name += 7;
invert = 1;
} else if (!strncmp(name, "require ", 8)) {
name += 8;
invert = 0;
} else if (!strncmp(name, "require-for-product:", 20)) {
// Get the product and point name past it
product = name + 20;
name = strchr(name, ' ');
if (!name) die("android-info.txt syntax error: %s", line);
*name = 0;
name += 1;
invert = 0;
}
x = strchr(name, '=');
if (x == 0) return;
*x = 0;
val[0] = x + 1;
name = strip(name);
// "require partition-exists=x" is a special case, added because of the trouble we had when
// Pixel 2 shipped with new partitions and users used old versions of fastboot to flash them,
// missing out new partitions. A device with new partitions can use "partition-exists" to
// override the fields `optional_if_no_image` in the `images` array.
if (!strcmp(name, "partition-exists")) {
const char* partition_name = val[0];
std::string has_slot;
if (!fb_getvar(std::string("has-slot:") + partition_name, &has_slot) ||
(has_slot != "yes" && has_slot != "no")) {
die("device doesn't have required partition %s!", partition_name);
}
bool known_partition = false;
for (size_t i = 0; i < arraysize(images); ++i) {
if (images[i].nickname && !strcmp(images[i].nickname, partition_name)) {
images[i].optional_if_no_image = false;
known_partition = true;
}
}
if (!known_partition) {
die("device requires partition %s which is not known to this version of fastboot",
partition_name);
}
return;
}
for(count = 1; count < MAX_OPTIONS; count++) {
x = strchr(val[count - 1],'|');
if (x == 0) break;
*x = 0;
val[count] = x + 1;
auto require_reject_regex = std::regex{"(require\\s+|reject\\s+)?\\s*(\\S+)\\s*=\\s*(.*)"};
auto require_product_regex =
std::regex{"require-for-product:\\s*(\\S+)\\s+(\\S+)\\s*=\\s*(.*)"};
std::smatch match_results;
if (std::regex_match(line, match_results, require_reject_regex)) {
*invert = Trim(match_results[1]) == "reject";
} else if (std::regex_match(line, match_results, require_product_regex)) {
*product = match_results[1];
} else {
return false;
}
*name = match_results[2];
// Work around an unfortunate name mismatch.
const char* var = name;
if (!strcmp(name, "board")) var = "product";
const char** out = reinterpret_cast<const char**>(malloc(sizeof(char*) * count));
if (out == nullptr) die("out of memory");
for (size_t i = 0; i < count; ++i) {
out[i] = xstrdup(strip(val[i]));
if (*name == "board") {
*name = "product";
}
fb_require(product, var, invert, count, out);
auto raw_options = Split(match_results[3], "|");
for (const auto& option : raw_options) {
auto trimmed_option = Trim(option);
options->emplace_back(trimmed_option);
}
return true;
}
static void check_requirements(char* data, int64_t sz) {
char* s = data;
while (sz-- > 0) {
if (*s == '\n') {
*s++ = 0;
check_requirement(data);
data = s;
// "require partition-exists=x" is a special case, added because of the trouble we had when
// Pixel 2 shipped with new partitions and users used old versions of fastboot to flash them,
// missing out new partitions. A device with new partitions can use "partition-exists" to
// override the fields `optional_if_no_image` in the `images` array.
static void HandlePartitionExists(const std::vector<std::string>& options) {
const std::string& partition_name = options[0];
std::string has_slot;
if (!fb_getvar("has-slot:" + partition_name, &has_slot) ||
(has_slot != "yes" && has_slot != "no")) {
die("device doesn't have required partition %s!", partition_name.c_str());
}
bool known_partition = false;
for (size_t i = 0; i < arraysize(images); ++i) {
if (images[i].nickname && images[i].nickname == partition_name) {
images[i].optional_if_no_image = false;
known_partition = true;
}
}
if (!known_partition) {
die("device requires partition %s which is not known to this version of fastboot",
partition_name.c_str());
}
}
static void CheckRequirements(const std::string& data) {
std::string cur_product;
if (!fb_getvar("product", &cur_product)) {
fprintf(stderr, "getvar:product FAILED (%s)\n", fb_get_error().c_str());
}
auto lines = Split(data, "\n");
for (const auto& line : lines) {
if (line.empty()) {
continue;
}
std::string name;
std::string product;
bool invert;
std::vector<std::string> options;
if (!ParseRequirementLine(line, &name, &product, &invert, &options)) {
fprintf(stderr, "android-info.txt syntax error: %s\n", line.c_str());
continue;
}
if (name == "partition-exists") {
HandlePartitionExists(options);
} else {
s++;
CheckRequirement(cur_product, name, product, invert, options);
}
}
}
@ -1156,7 +1193,7 @@ void FlashAllTool::CheckRequirements() {
if (!source_.ReadFile("android-info.txt", &contents)) {
die("could not read android-info.txt");
}
check_requirements(reinterpret_cast<char*>(contents.data()), contents.size());
::CheckRequirements({contents.data(), contents.size()});
}
void FlashAllTool::DetermineSecondarySlot() {
@ -1265,8 +1302,6 @@ int ZipImageSource::OpenFile(const std::string& name) const {
static void do_update(const char* filename, const std::string& slot_override, bool skip_secondary) {
dump_info();
fb_query_save("product", cur_product, sizeof(cur_product));
ZipArchiveHandle zip;
int error = OpenArchive(filename, &zip);
if (error != 0) {
@ -1302,8 +1337,6 @@ static void do_flashall(const std::string& slot_override, bool skip_secondary, b
std::string fname;
dump_info();
fb_query_save("product", cur_product, sizeof(cur_product));
FlashAllTool tool(LocalImageSource(), slot_override, skip_secondary, wipe);
tool.Flash();
}

View file

@ -147,28 +147,6 @@ RetCode FastBootDriver::Partitions(std::vector<std::tuple<std::string, uint64_t>
return SUCCESS;
}
RetCode FastBootDriver::Require(const std::string& var, const std::vector<std::string>& allowed,
bool* reqmet, bool invert) {
*reqmet = invert;
RetCode ret;
std::string response;
if ((ret = GetVar(var, &response))) {
return ret;
}
// Now check if we have a match
for (const auto s : allowed) {
// If it ends in *, and starting substring match
if (response == s || (s.length() && s.back() == '*' &&
!response.compare(0, s.length() - 1, s, 0, s.length() - 1))) {
*reqmet = !invert;
break;
}
}
return SUCCESS;
}
RetCode FastBootDriver::Download(int fd, size_t size, std::string* response,
std::vector<std::string>* info) {
RetCode ret;

View file

@ -59,3 +59,145 @@ TEST(FastBoot, ParseOsVersion) {
EXPECT_DEATH(fb.ParseOsVersion(&hdr, "1.128.3"), "bad OS version");
EXPECT_DEATH(fb.ParseOsVersion(&hdr, "1.2.128"), "bad OS version");
}
extern bool ParseRequirementLine(const std::string& line, std::string* name, std::string* product,
bool* invert, std::vector<std::string>* options);
static void ParseRequirementLineTest(const std::string& line, const std::string& expected_name,
const std::string& expected_product, bool expected_invert,
const std::vector<std::string>& expected_options) {
std::string name;
std::string product;
bool invert;
std::vector<std::string> options;
EXPECT_TRUE(ParseRequirementLine(line, &name, &product, &invert, &options)) << line;
EXPECT_EQ(expected_name, name) << line;
EXPECT_EQ(expected_product, product) << line;
EXPECT_EQ(expected_invert, invert) << line;
EXPECT_EQ(expected_options, options) << line;
}
TEST(FastBoot, ParseRequirementLineSuccesses) {
// Examples provided in the code + slight variations.
ParseRequirementLineTest("require product=alpha", "product", "", false, {"alpha"});
ParseRequirementLineTest("require product=alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require version-bootloader=1234", "version-bootloader", "", false,
{"1234"});
ParseRequirementLineTest("require-for-product:gamma version-bootloader=istanbul",
"version-bootloader", "gamma", false, {"istanbul"});
ParseRequirementLineTest("require-for-product:gamma version-bootloader=istanbul|constantinople",
"version-bootloader", "gamma", false, {"istanbul", "constantinople"});
ParseRequirementLineTest("require partition-exists=vendor", "partition-exists", "", false,
{"vendor"});
ParseRequirementLineTest("reject product=alpha", "product", "", true, {"alpha"});
ParseRequirementLineTest("reject product=alpha|beta|gamma", "product", "", true,
{"alpha", "beta", "gamma"});
// Without any prefix, assume 'require'
ParseRequirementLineTest("product=alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
// Including if the variable name is otherwise a prefix keyword
ParseRequirementLineTest("require = alpha", "require", "", false, {"alpha"});
ParseRequirementLineTest("reject = alpha", "reject", "", false, {"alpha"});
ParseRequirementLineTest("require-for-product:gamma = alpha", "require-for-product:gamma", "",
false, {"alpha"});
// Extra spaces are allowed.
ParseRequirementLineTest("require product=alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product =alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product= alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product = alpha|beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product=alpha |beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product=alpha| beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product=alpha | beta|gamma", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require product=alpha|beta|gamma ", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("product = alpha | beta | gamma ", "product", "", false,
{"alpha", "beta", "gamma"});
ParseRequirementLineTest("require-for-product: gamma version-bootloader=istanbul",
"version-bootloader", "gamma", false, {"istanbul"});
// Extraneous ending | is okay, implies accepting an empty string.
ParseRequirementLineTest("require product=alpha|", "product", "", false, {"alpha", ""});
ParseRequirementLineTest("require product=alpha|beta|gamma|", "product", "", false,
{"alpha", "beta", "gamma", ""});
// Accept empty options, double ||, etc, implies accepting an empty string.
ParseRequirementLineTest("require product=alpha||beta| |gamma", "product", "", false,
{"alpha", "", "beta", "", "gamma"});
ParseRequirementLineTest("require product=alpha||beta|gamma", "product", "", false,
{"alpha", "", "beta", "gamma"});
ParseRequirementLineTest("require product=alpha|beta| |gamma", "product", "", false,
{"alpha", "beta", "", "gamma"});
ParseRequirementLineTest("require product=alpha||", "product", "", false, {"alpha", "", ""});
ParseRequirementLineTest("require product=alpha|| ", "product", "", false, {"alpha", "", ""});
ParseRequirementLineTest("require product=alpha| ", "product", "", false, {"alpha", ""});
ParseRequirementLineTest("require product=alpha|beta| ", "product", "", false,
{"alpha", "beta", ""});
// No option string is also treating as accepting an empty string.
ParseRequirementLineTest("require =", "require", "", false, {""});
ParseRequirementLineTest("require = |", "require", "", false, {"", ""});
ParseRequirementLineTest("reject =", "reject", "", false, {""});
ParseRequirementLineTest("reject = |", "reject", "", false, {"", ""});
ParseRequirementLineTest("require-for-product: =", "require-for-product:", "", false, {""});
ParseRequirementLineTest("require-for-product: = | ", "require-for-product:", "", false,
{"", ""});
ParseRequirementLineTest("require product=", "product", "", false, {""});
ParseRequirementLineTest("require product = ", "product", "", false, {""});
ParseRequirementLineTest("require product = | ", "product", "", false, {"", ""});
ParseRequirementLineTest("reject product=", "product", "", true, {""});
ParseRequirementLineTest("reject product = ", "product", "", true, {""});
ParseRequirementLineTest("reject product = | ", "product", "", true, {"", ""});
ParseRequirementLineTest("require-for-product:gamma product=", "product", "gamma", false, {""});
ParseRequirementLineTest("require-for-product:gamma product = ", "product", "gamma", false,
{""});
ParseRequirementLineTest("require-for-product:gamma product = |", "product", "gamma", false,
{"", ""});
// Check for board -> product substitution.
ParseRequirementLineTest("require board=alpha", "product", "", false, {"alpha"});
ParseRequirementLineTest("board=alpha", "product", "", false, {"alpha"});
}
static void ParseRequirementLineTestMalformed(const std::string& line) {
std::string name;
std::string product;
bool invert;
std::vector<std::string> options;
EXPECT_FALSE(ParseRequirementLine(line, &name, &product, &invert, &options)) << line;
}
TEST(FastBoot, ParseRequirementLineMalformed) {
ParseRequirementLineTestMalformed("nothing");
ParseRequirementLineTestMalformed("");
ParseRequirementLineTestMalformed("=");
ParseRequirementLineTestMalformed("|");
ParseRequirementLineTestMalformed("require");
ParseRequirementLineTestMalformed("require ");
ParseRequirementLineTestMalformed("reject");
ParseRequirementLineTestMalformed("reject ");
ParseRequirementLineTestMalformed("require-for-product:");
ParseRequirementLineTestMalformed("require-for-product: ");
ParseRequirementLineTestMalformed("require product");
ParseRequirementLineTestMalformed("reject product");
ParseRequirementLineTestMalformed("require-for-product:gamma");
ParseRequirementLineTestMalformed("require-for-product:gamma product");
// No spaces allowed before between require-for-product and :.
ParseRequirementLineTestMalformed("require-for-product :");
}

View file

@ -74,9 +74,3 @@ void Status(const std::string& message) {
static constexpr char kStatusFormat[] = "%-50s ";
fprintf(stderr, kStatusFormat, message.c_str());
}
char* xstrdup(const char* s) {
char* result = strdup(s);
if (!result) die("out of memory");
return result;
}

View file

@ -9,7 +9,6 @@
/* util stuff */
double now();
char* xstrdup(const char*);
void set_verbose();
void Status(const std::string& message);