edify: Move State.script and State.errmsg to std::string.
This way we kill a few strdup() and free() calls.
Test: 1. recovery_component_test still passes;
2. Applying an update with the new updater works;
3. The error code in a script with abort("E310: xyz") is recorded into
last_install correctly.
Change-Id: Ibda4da5937346e058a0d7cc81764d6f02920010a
diff --git a/edify/edify_parser.cpp b/edify/edify_parser.cpp
index 16ad2ab..15342d3 100644
--- a/edify/edify_parser.cpp
+++ b/edify/edify_parser.cpp
@@ -24,21 +24,20 @@
* makes the tool less useful. We should either extend the tool or remove it.
*/
+#include <errno.h>
#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
+
+#include <string>
+
+#include <android-base/file.h>
#include "expr.h"
-#include "parser.h"
-void ExprDump(int depth, Expr* n, char* script) {
+static void ExprDump(int depth, const Expr* n, const std::string& script) {
printf("%*s", depth*2, "");
- char temp = script[n->end];
- script[n->end] = '\0';
printf("%s %p (%d-%d) \"%s\"\n",
n->name == NULL ? "(NULL)" : n->name, n->fn, n->start, n->end,
- script+n->start);
- script[n->end] = temp;
+ script.substr(n->start, n->end - n->start).c_str());
for (int i = 0; i < n->argc; ++i) {
ExprDump(depth+1, n->argv[i], script);
}
@@ -53,34 +52,25 @@
return 1;
}
- FILE* f = fopen(argv[1], "r");
- if (f == NULL) {
- printf("%s: %s: No such file or directory\n", argv[0], argv[1]);
+ std::string buffer;
+ if (!android::base::ReadFileToString(argv[1], &buffer)) {
+ printf("%s: failed to read %s: %s\n", argv[0], argv[1], strerror(errno));
return 1;
}
- char buffer[8192];
- int size = fread(buffer, 1, 8191, f);
- fclose(f);
- buffer[size] = '\0';
Expr* root;
int error_count = 0;
- int error = parse_string(buffer, &root, &error_count);
+ int error = parse_string(buffer.data(), &root, &error_count);
printf("parse returned %d; %d errors encountered\n", error, error_count);
if (error == 0 || error_count > 0) {
ExprDump(0, root, buffer);
- State state;
- state.cookie = NULL;
- state.script = buffer;
- state.errmsg = NULL;
-
+ State state(buffer, nullptr);
char* result = Evaluate(&state, root);
if (result == NULL) {
printf("result was NULL, message is: %s\n",
- (state.errmsg == NULL ? "(NULL)" : state.errmsg));
- free(state.errmsg);
+ (state.errmsg.empty() ? "(NULL)" : state.errmsg.c_str()));
} else {
printf("result is [%s]\n", result);
}
diff --git a/edify/expr.cpp b/edify/expr.cpp
index ecb1bea..aa3a55a 100644
--- a/edify/expr.cpp
+++ b/edify/expr.cpp
@@ -107,8 +107,7 @@
Value* IfElseFn(const char* name, State* state, int argc, Expr* argv[]) {
if (argc != 2 && argc != 3) {
- free(state->errmsg);
- state->errmsg = strdup("ifelse expects 2 or 3 arguments");
+ state->errmsg = "ifelse expects 2 or 3 arguments";
return NULL;
}
char* cond = Evaluate(state, argv[0]);
@@ -134,11 +133,10 @@
if (argc > 0) {
msg = Evaluate(state, argv[0]);
}
- free(state->errmsg);
if (msg) {
state->errmsg = msg;
} else {
- state->errmsg = strdup("called abort()");
+ state->errmsg = "called abort()";
}
return NULL;
}
@@ -153,15 +151,8 @@
int b = BooleanString(v);
free(v);
if (!b) {
- int prefix_len;
int len = argv[i]->end - argv[i]->start;
- char* err_src = reinterpret_cast<char*>(malloc(len + 20));
- strcpy(err_src, "assert failed: ");
- prefix_len = strlen(err_src);
- memcpy(err_src + prefix_len, state->script + argv[i]->start, len);
- err_src[prefix_len + len] = '\0';
- free(state->errmsg);
- state->errmsg = err_src;
+ state->errmsg = "assert failed: " + state->script.substr(argv[i]->start, len);
return NULL;
}
}
@@ -279,8 +270,7 @@
Value* LessThanIntFn(const char* name, State* state, int argc, Expr* argv[]) {
if (argc != 2) {
- free(state->errmsg);
- state->errmsg = strdup("less_than_int expects 2 arguments");
+ state->errmsg = "less_than_int expects 2 arguments";
return NULL;
}
@@ -314,8 +304,7 @@
Value* GreaterThanIntFn(const char* name, State* state,
int argc, Expr* argv[]) {
if (argc != 2) {
- free(state->errmsg);
- state->errmsg = strdup("greater_than_int expects 2 arguments");
+ state->errmsg = "greater_than_int expects 2 arguments";
return NULL;
}
@@ -499,20 +488,12 @@
return args;
}
-static void ErrorAbortV(State* state, const char* format, va_list ap) {
- std::string buffer;
- android::base::StringAppendV(&buffer, format, ap);
- free(state->errmsg);
- state->errmsg = strdup(buffer.c_str());
- return;
-}
-
// Use printf-style arguments to compose an error message to put into
// *state. Returns nullptr.
Value* ErrorAbort(State* state, const char* format, ...) {
va_list ap;
va_start(ap, format);
- ErrorAbortV(state, format, ap);
+ android::base::StringAppendV(&state->errmsg, format, ap);
va_end(ap);
return nullptr;
}
@@ -520,8 +501,14 @@
Value* ErrorAbort(State* state, CauseCode cause_code, const char* format, ...) {
va_list ap;
va_start(ap, format);
- ErrorAbortV(state, format, ap);
+ android::base::StringAppendV(&state->errmsg, format, ap);
va_end(ap);
state->cause_code = cause_code;
return nullptr;
}
+
+State::State(const std::string& script, void* cookie) :
+ script(script),
+ cookie(cookie) {
+}
+
diff --git a/edify/expr.h b/edify/expr.h
index 8863479..f045d93 100644
--- a/edify/expr.h
+++ b/edify/expr.h
@@ -18,6 +18,7 @@
#define _EXPRESSION_H
#include <unistd.h>
+#include <string>
#include "error_code.h"
#include "yydefs.h"
@@ -26,20 +27,20 @@
typedef struct Expr Expr;
-typedef struct {
+struct State {
+ State(const std::string& script, void* cookie);
+
+ // The source of the original script.
+ const std::string& script;
+
// Optional pointer to app-specific data; the core of edify never
// uses this value.
void* cookie;
- // The source of the original script. Must be NULL-terminated,
- // and in writable memory (Evaluate may make temporary changes to
- // it but will restore it when done).
- char* script;
-
// The error message (if any) returned if the evaluation aborts.
- // Should be NULL initially, will be either NULL or a malloc'd
- // pointer after Evaluate() returns.
- char* errmsg;
+ // Should be empty initially, will be either empty or a string that
+ // Evaluate() returns.
+ std::string errmsg;
// error code indicates the type of failure (e.g. failure to update system image)
// during the OTA process.
@@ -50,8 +51,7 @@
CauseCode cause_code = kNoCause;
bool is_retry = false;
-
-} State;
+};
#define VAL_STRING 1 // data will be NULL-terminated; size doesn't count null
#define VAL_BLOB 2
diff --git a/tests/component/edify_test.cpp b/tests/component/edify_test.cpp
index ded8540..ede2ecb 100644
--- a/tests/component/edify_test.cpp
+++ b/tests/component/edify_test.cpp
@@ -25,10 +25,7 @@
int error_count;
EXPECT_EQ(parse_string(expr_str, &e, &error_count), 0);
- State state;
- state.cookie = nullptr;
- state.errmsg = nullptr;
- state.script = strdup(expr_str);
+ State state(expr_str, nullptr);
char* result = Evaluate(&state, e);
@@ -38,8 +35,6 @@
EXPECT_STREQ(result, expected);
}
- free(state.errmsg);
- free(state.script);
free(result);
}
diff --git a/updater/updater.cpp b/updater/updater.cpp
index c222cee..74a4048 100644
--- a/updater/updater.cpp
+++ b/updater/updater.cpp
@@ -14,22 +14,24 @@
* limitations under the License.
*/
+#include "updater.h"
+
#include <stdio.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
-#include "edify/expr.h"
-#include "updater.h"
-#include "install.h"
-#include "blockimg.h"
-#include "minzip/Zip.h"
-#include "minzip/SysUtil.h"
-#include "config.h"
-
+#include <android-base/strings.h>
#include <selinux/label.h>
#include <selinux/selinux.h>
+#include "config.h"
+#include "edify/expr.h"
+#include "blockimg.h"
+#include "install.h"
+#include "minzip/Zip.h"
+#include "minzip/SysUtil.h"
+
// Generated by the makefile, this function defines the
// RegisterDeviceExtensions() function, which calls all the
// registration functions for device-specific extensions.
@@ -140,10 +142,7 @@
updater_info.package_zip_addr = map.addr;
updater_info.package_zip_len = map.length;
- State state;
- state.cookie = &updater_info;
- state.script = script;
- state.errmsg = NULL;
+ State state(script, &updater_info);
if (argc == 5) {
if (strcmp(argv[4], "retry") == 0) {
@@ -160,22 +159,21 @@
}
if (result == NULL) {
- if (state.errmsg == NULL) {
+ if (state.errmsg.empty()) {
printf("script aborted (no error message)\n");
fprintf(cmd_pipe, "ui_print script aborted (no error message)\n");
} else {
- printf("script aborted: %s\n", state.errmsg);
- char* line = strtok(state.errmsg, "\n");
- while (line) {
+ printf("script aborted: %s\n", state.errmsg.c_str());
+ const std::vector<std::string> lines = android::base::Split(state.errmsg, "\n");
+ for (const std::string& line : lines) {
// Parse the error code in abort message.
// Example: "E30: This package is for bullhead devices."
- if (*line == 'E') {
- if (sscanf(line, "E%u: ", &state.error_code) != 1) {
- printf("Failed to parse error code: [%s]\n", line);
+ if (!line.empty() && line[0] == 'E') {
+ if (sscanf(line.c_str(), "E%u: ", &state.error_code) != 1) {
+ printf("Failed to parse error code: [%s]\n", line.c_str());
}
}
- fprintf(cmd_pipe, "ui_print %s\n", line);
- line = strtok(NULL, "\n");
+ fprintf(cmd_pipe, "ui_print %s\n", line.c_str());
}
fprintf(cmd_pipe, "ui_print\n");
}
@@ -189,7 +187,6 @@
}
}
- free(state.errmsg);
return 7;
} else {
fprintf(cmd_pipe, "ui_print script succeeded: result was [%s]\n", result);