Merge "edify: Rename parse_string to ParseString and let it take std::string."

edify/include/edify/expr.h

@@ -153,6 +153,6 @@ Value* StringValue(const char* str);
 
 Value* StringValue(const std::string& str);
 
-int parse_string(const char* str, std::unique_ptr<Expr>* root, int* error_count);
+int ParseString(const std::string& str, std::unique_ptr<Expr>* root, int* error_count);
 
 #endif  // _EXPRESSION_H

edify/parser.yy

@@ -138,7 +138,7 @@ void yyerror(std::unique_ptr<Expr>* root, int* error_count, const char* s) {
   ++*error_count;
 }
 
-int parse_string(const char* str, std::unique_ptr<Expr>* root, int* error_count) {
+int ParseString(const std::string& str, std::unique_ptr<Expr>* root, int* error_count) {
-    yy_switch_to_buffer(yy_scan_string(str));
+  yy_switch_to_buffer(yy_scan_string(str.c_str()));
-    return yyparse(root, error_count);
+  return yyparse(root, error_count);
 }

tests/component/edify_test.cpp

@@ -21,30 +21,29 @@
 
 #include "edify/expr.h"
 
-static void expect(const char* expr_str, const char* expected) {
+static void expect(const std::string& expr_str, const char* expected) {
-    std::unique_ptr<Expr> e;
+  std::unique_ptr<Expr> e;
-    int error_count = 0;
+  int error_count = 0;
-    EXPECT_EQ(0, parse_string(expr_str, &e, &error_count));
+  EXPECT_EQ(0, ParseString(expr_str, &e, &error_count));
-    EXPECT_EQ(0, error_count);
+  EXPECT_EQ(0, error_count);
 
-    State state(expr_str, nullptr);
+  State state(expr_str, nullptr);
 
-    std::string result;
+  std::string result;
-    bool status = Evaluate(&state, e, &result);
+  bool status = Evaluate(&state, e, &result);
-
-    if (expected == nullptr) {
-        EXPECT_FALSE(status);
-    } else {
-        EXPECT_STREQ(expected, result.c_str());
-    }
 
+  if (expected == nullptr) {
+    EXPECT_FALSE(status);
+  } else {
+    EXPECT_STREQ(expected, result.c_str());
+  }
 }
 
 class EdifyTest : public ::testing::Test {
-  protected:
+ protected:
-    virtual void SetUp() {
+  void SetUp() {
-        RegisterBuiltins();
+    RegisterBuiltins();
-    }
+  }
 };
 
 TEST_F(EdifyTest, parsing) {
@@ -146,25 +145,23 @@ TEST_F(EdifyTest, comparison) {
 }
 
 TEST_F(EdifyTest, big_string) {
-    // big string
+  expect(std::string(8192, 's'), std::string(8192, 's').c_str());
-    expect(std::string(8192, 's').c_str(), std::string(8192, 's').c_str());
 }
 
 TEST_F(EdifyTest, unknown_function) {
-    // unknown function
+  const char* script1 = "unknown_function()";
-    const char* script1 = "unknown_function()";
+  std::unique_ptr<Expr> expr;
-    std::unique_ptr<Expr> expr;
+  int error_count = 0;
-    int error_count = 0;
+  EXPECT_EQ(1, ParseString(script1, &expr, &error_count));
-    EXPECT_EQ(1, parse_string(script1, &expr, &error_count));
+  EXPECT_EQ(1, error_count);
-    EXPECT_EQ(1, error_count);
 
-    const char* script2 = "abc; unknown_function()";
+  const char* script2 = "abc; unknown_function()";
-    error_count = 0;
+  error_count = 0;
-    EXPECT_EQ(1, parse_string(script2, &expr, &error_count));
+  EXPECT_EQ(1, ParseString(script2, &expr, &error_count));
-    EXPECT_EQ(1, error_count);
+  EXPECT_EQ(1, error_count);
 
-    const char* script3 = "unknown_function1() || yes";
+  const char* script3 = "unknown_function1() || yes";
-    error_count = 0;
+  error_count = 0;
-    EXPECT_EQ(1, parse_string(script3, &expr, &error_count));
+  EXPECT_EQ(1, ParseString(script3, &expr, &error_count));
-    EXPECT_EQ(1, error_count);
+  EXPECT_EQ(1, error_count);
 }

tests/component/updater_test.cpp

@@ -57,11 +57,11 @@ static constexpr size_t kTransferListHeaderLines = 4;
 
 struct selabel_handle* sehandle = nullptr;
 
-static void expect(const char* expected, const char* expr_str, CauseCode cause_code,
+static void expect(const char* expected, const std::string& expr_str, CauseCode cause_code,
                    UpdaterInfo* info = nullptr) {
   std::unique_ptr<Expr> e;
   int error_count = 0;
-  ASSERT_EQ(0, parse_string(expr_str, &e, &error_count));
+  ASSERT_EQ(0, ParseString(expr_str, &e, &error_count));
   ASSERT_EQ(0, error_count);
 
   State state(expr_str, info);
@@ -126,7 +126,7 @@ static void RunBlockImageUpdate(bool is_verify, const PackageEntries& entries,
   std::string script = is_verify ? "block_image_verify" : "block_image_update";
   script += R"((")" + image_file + R"(", package_extract_file("transfer_list"), ")" + new_data +
             R"(", "patch_data"))";
-  expect(result.c_str(), script.c_str(), cause_code, &updater_info);
+  expect(result.c_str(), script, cause_code, &updater_info);
 
   ASSERT_EQ(0, fclose(updater_info.cmd_pipe));
   CloseArchive(handle);
@@ -230,7 +230,7 @@ TEST_F(UpdaterTest, apply_patch_check) {
   std::string filename = android::base::Join(
       std::vector<std::string>{ "EMMC", src_file, std::to_string(src_size), src_hash }, ":");
   std::string cmd = "apply_patch_check(\"" + filename + "\")";
-  expect("t", cmd.c_str(), kNoCause);
+  expect("t", cmd, kNoCause);
 
   // EMMC:file:(size-1):sha1:(size+1):sha1 should fail the check.
   std::string filename_bad = android::base::Join(
@@ -238,7 +238,7 @@ TEST_F(UpdaterTest, apply_patch_check) {
                                 std::to_string(src_size + 1), src_hash },
       ":");
   cmd = "apply_patch_check(\"" + filename_bad + "\")";
-  expect("", cmd.c_str(), kNoCause);
+  expect("", cmd, kNoCause);
 
   // EMMC:file:(size-1):sha1:size:sha1:(size+1):sha1 should pass the check.
   filename_bad =
@@ -247,21 +247,21 @@ TEST_F(UpdaterTest, apply_patch_check) {
                                                     std::to_string(src_size + 1), src_hash },
                           ":");
   cmd = "apply_patch_check(\"" + filename_bad + "\")";
-  expect("t", cmd.c_str(), kNoCause);
+  expect("t", cmd, kNoCause);
 
   // Multiple arguments.
   // As long as it successfully loads the partition specified in filename, it won't check against
   // any given SHAs.
   cmd = "apply_patch_check(\"" + filename + "\", \"wrong_sha1\", \"wrong_sha2\")";
-  expect("t", cmd.c_str(), kNoCause);
+  expect("t", cmd, kNoCause);
 
   cmd = "apply_patch_check(\"" + filename + "\", \"wrong_sha1\", \"" + src_hash +
         "\", \"wrong_sha2\")";
-  expect("t", cmd.c_str(), kNoCause);
+  expect("t", cmd, kNoCause);
 
   cmd = "apply_patch_check(\"" + filename_bad + "\", \"wrong_sha1\", \"" + src_hash +
         "\", \"wrong_sha2\")";
-  expect("t", cmd.c_str(), kNoCause);
+  expect("t", cmd, kNoCause);
 }
 
 TEST_F(UpdaterTest, file_getprop) {
@@ -288,28 +288,28 @@ TEST_F(UpdaterTest, file_getprop) {
 
     std::string script1("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.name\")");
-    expect("tardis", script1.c_str(), kNoCause);
+    expect("tardis", script1, kNoCause);
 
     std::string script2("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.board\")");
-    expect("magic", script2.c_str(), kNoCause);
+    expect("magic", script2, kNoCause);
 
     // No match.
     std::string script3("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.wrong\")");
-    expect("", script3.c_str(), kNoCause);
+    expect("", script3, kNoCause);
 
     std::string script4("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.name=\")");
-    expect("", script4.c_str(), kNoCause);
+    expect("", script4, kNoCause);
 
     std::string script5("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.nam\")");
-    expect("", script5.c_str(), kNoCause);
+    expect("", script5, kNoCause);
 
     std::string script6("file_getprop(\"" + std::string(temp_file2.path) +
                        "\", \"ro.product.model\")");
-    expect("", script6.c_str(), kNoCause);
+    expect("", script6, kNoCause);
 }
 
 // TODO: Test extracting to block device.
@@ -329,7 +329,7 @@ TEST_F(UpdaterTest, package_extract_file) {
   // Two-argument version.
   TemporaryFile temp_file1;
   std::string script("package_extract_file(\"a.txt\", \"" + std::string(temp_file1.path) + "\")");
-  expect("t", script.c_str(), kNoCause, &updater_info);
+  expect("t", script, kNoCause, &updater_info);
 
   // Verify the extracted entry.
   std::string data;
@@ -338,30 +338,30 @@ TEST_F(UpdaterTest, package_extract_file) {
 
   // Now extract another entry to the same location, which should overwrite.
   script = "package_extract_file(\"b.txt\", \"" + std::string(temp_file1.path) + "\")";
-  expect("t", script.c_str(), kNoCause, &updater_info);
+  expect("t", script, kNoCause, &updater_info);
 
   ASSERT_TRUE(android::base::ReadFileToString(temp_file1.path, &data));
   ASSERT_EQ(kBTxtContents, data);
 
   // Missing zip entry. The two-argument version doesn't abort.
   script = "package_extract_file(\"doesntexist\", \"" + std::string(temp_file1.path) + "\")";
-  expect("", script.c_str(), kNoCause, &updater_info);
+  expect("", script, kNoCause, &updater_info);
 
   // Extract to /dev/full should fail.
   script = "package_extract_file(\"a.txt\", \"/dev/full\")";
-  expect("", script.c_str(), kNoCause, &updater_info);
+  expect("", script, kNoCause, &updater_info);
 
   // One-argument version. package_extract_file() gives a VAL_BLOB, which needs to be converted to
   // VAL_STRING for equality test.
   script = "blob_to_string(package_extract_file(\"a.txt\")) == \"" + kATxtContents + "\"";
-  expect("t", script.c_str(), kNoCause, &updater_info);
+  expect("t", script, kNoCause, &updater_info);
 
   script = "blob_to_string(package_extract_file(\"b.txt\")) == \"" + kBTxtContents + "\"";
-  expect("t", script.c_str(), kNoCause, &updater_info);
+  expect("t", script, kNoCause, &updater_info);
 
   // Missing entry. The one-argument version aborts the evaluation.
   script = "package_extract_file(\"doesntexist\")";
-  expect(nullptr, script.c_str(), kPackageExtractFileFailure, &updater_info);
+  expect(nullptr, script, kPackageExtractFileFailure, &updater_info);
 
   CloseArchive(handle);
 }
@@ -379,7 +379,7 @@ TEST_F(UpdaterTest, write_value) {
   TemporaryFile temp_file;
   std::string value = "magicvalue";
   std::string script("write_value(\"" + value + "\", \"" + std::string(temp_file.path) + "\")");
-  expect("t", script.c_str(), kNoCause);
+  expect("t", script, kNoCause);
 
   // Verify the content.
   std::string content;
@@ -388,7 +388,7 @@ TEST_F(UpdaterTest, write_value) {
 
   // Allow writing empty string.
   script = "write_value(\"\", \"" + std::string(temp_file.path) + "\")";
-  expect("t", script.c_str(), kNoCause);
+  expect("t", script, kNoCause);
 
   // Verify the content.
   ASSERT_TRUE(android::base::ReadFileToString(temp_file.path, &content));
@@ -396,7 +396,7 @@ TEST_F(UpdaterTest, write_value) {
 
   // It should fail gracefully when write fails.
   script = "write_value(\"value\", \"/proc/0/file1\")";
-  expect("", script.c_str(), kNoCause);
+  expect("", script, kNoCause);
 }
 
 TEST_F(UpdaterTest, get_stage) {
@@ -415,11 +415,11 @@ TEST_F(UpdaterTest, get_stage) {
 
   // Can read the stage value.
   std::string script("get_stage(\"" + temp_file + "\")");
-  expect("2/3", script.c_str(), kNoCause);
+  expect("2/3", script, kNoCause);
 
   // Bad BCB path.
   script = "get_stage(\"doesntexist\")";
-  expect("", script.c_str(), kNoCause);
+  expect("", script, kNoCause);
 }
 
 TEST_F(UpdaterTest, set_stage) {
@@ -439,7 +439,7 @@ TEST_F(UpdaterTest, set_stage) {
 
   // Write with set_stage().
   std::string script("set_stage(\"" + temp_file + "\", \"1/3\")");
-  expect(tf.path, script.c_str(), kNoCause);
+  expect(tf.path, script, kNoCause);
 
   // Verify.
   bootloader_message boot_verify;
@@ -451,10 +451,10 @@ TEST_F(UpdaterTest, set_stage) {
 
   // Bad BCB path.
   script = "set_stage(\"doesntexist\", \"1/3\")";
-  expect("", script.c_str(), kNoCause);
+  expect("", script, kNoCause);
 
   script = "set_stage(\"/dev/full\", \"1/3\")";
-  expect("", script.c_str(), kNoCause);
+  expect("", script, kNoCause);
 }
 
 TEST_F(UpdaterTest, set_progress) {

updater/updater.cpp

@@ -134,7 +134,7 @@ int main(int argc, char** argv) {
 
   std::unique_ptr<Expr> root;
   int error_count = 0;
-  int error = parse_string(script.c_str(), &root, &error_count);
+  int error = ParseString(script, &root, &error_count);
   if (error != 0 || error_count > 0) {
     LOG(ERROR) << error_count << " parse errors";
     CloseArchive(za);