7 files changed, 354 insertions, 626 deletions

diff --git a/edify/Android.mk b/edify/Android.mk
index 71cf7652a..d8058c16f 100644
--- a/edify/Android.mk
+++ b/edify/Android.mk
@@ -1,23 +1,36 @@
 # Copyright 2009 The Android Open Source Project
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
 
 LOCAL_PATH := $(call my-dir)
 
 edify_src_files := \
-	lexer.ll \
-	parser.yy \
-	expr.cpp
+    lexer.ll \
+    parser.yy \
+    expr.cpp
 
 #
-# Build the host-side command line tool
+# Build the host-side command line tool (host executable)
 #
 include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := \
-		$(edify_src_files) \
-		main.cpp
+    $(edify_src_files) \
+    edify_parser.cpp
 
+LOCAL_CFLAGS := -Werror
 LOCAL_CPPFLAGS := -g -O0
-LOCAL_MODULE := edify
+LOCAL_MODULE := edify_parser
 LOCAL_YACCFLAGS := -v
 LOCAL_CPPFLAGS += -Wno-unused-parameter
 LOCAL_CPPFLAGS += -Wno-deprecated-register
@@ -28,12 +41,13 @@ LOCAL_STATIC_LIBRARIES += libbase
 include $(BUILD_HOST_EXECUTABLE)
 
 #
-# Build the device-side library
+# Build the device-side library (static library)
 #
 include $(CLEAR_VARS)
 
 LOCAL_SRC_FILES := $(edify_src_files)
 
+LOCAL_CFLAGS := -Werror
 LOCAL_CPPFLAGS := -Wno-unused-parameter
 LOCAL_CPPFLAGS += -Wno-deprecated-register
 LOCAL_MODULE := libedify
diff --git a/edify/README b/edify/README.md
index 810455cca..b3330e23a 100644
--- a/edify/README
+++ b/edify/README.md
@@ -1,3 +1,6 @@
+edify
+=====
+
 Update scripts (from donut onwards) are written in a new little
 scripting language ("edify") that is superficially somewhat similar to
 the old one ("amend").  This is a brief overview of the new language.
diff --git a/edify/edify_parser.cpp b/edify/edify_parser.cpp
new file mode 100644
index 000000000..908fcf13b
--- /dev/null
+++ b/edify/edify_parser.cpp
@@ -0,0 +1,78 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+/**
+ * This is a host-side tool for validating a given edify script file.
+ *
+ * We used to have edify test cases here, which have been moved to
+ * tests/component/edify_test.cpp.
+ *
+ * Caveat: It doesn't recognize functions defined through updater, which
+ * makes the tool less useful. We should either extend the tool or remove it.
+ */
+
+#include <errno.h>
+#include <stdio.h>
+
+#include <string>
+
+#include <android-base/file.h>
+
+#include "expr.h"
+
+static void ExprDump(int depth, const Expr* n, const std::string& script) {
+    printf("%*s", depth*2, "");
+    printf("%s %p (%d-%d) \"%s\"\n",
+           n->name == NULL ? "(NULL)" : n->name, n->fn, n->start, n->end,
+           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);
+    }
+}
+
+int main(int argc, char** argv) {
+    RegisterBuiltins();
+
+    if (argc != 2) {
+        printf("Usage: %s <edify script>\n", argv[0]);
+        return 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;
+    }
+
+    Expr* root;
+    int error_count = 0;
+    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(buffer, nullptr);
+        std::string result;
+        if (!Evaluate(&state, root, &result)) {
+            printf("result was NULL, message is: %s\n",
+                   (state.errmsg.empty() ? "(NULL)" : state.errmsg.c_str()));
+        } else {
+            printf("result is [%s]\n", result.c_str());
+        }
+    }
+    return 0;
+}
diff --git a/edify/expr.cpp b/edify/expr.cpp
index cc14fbe93..329cf3acd 100644
--- a/edify/expr.cpp
+++ b/edify/expr.cpp
@@ -14,201 +14,172 @@
  * limitations under the License.
  */
 
-#include <string.h>
-#include <stdbool.h>
+#include "expr.h"
+
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdarg.h>
+#include <string.h>
 #include <unistd.h>
 
+#include <memory>
 #include <string>
+#include <unordered_map>
+#include <vector>
 
+#include <android-base/parseint.h>
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 
-#include "expr.h"
-
 // Functions should:
 //
 //    - return a malloc()'d string
-//    - if Evaluate() on any argument returns NULL, return NULL.
+//    - if Evaluate() on any argument returns nullptr, return nullptr.
 
-int BooleanString(const char* s) {
-    return s[0] != '\0';
+static bool BooleanString(const std::string& s) {
+    return !s.empty();
 }
 
-char* Evaluate(State* state, Expr* expr) {
-    Value* v = expr->fn(expr->name, state, expr->argc, expr->argv);
-    if (v == NULL) return NULL;
+bool Evaluate(State* state, Expr* expr, std::string* result) {
+    if (result == nullptr) {
+        return false;
+    }
+
+    std::unique_ptr<Value> v(expr->fn(expr->name, state, expr->argc, expr->argv));
+    if (!v) {
+        return false;
+    }
     if (v->type != VAL_STRING) {
         ErrorAbort(state, kArgsParsingFailure, "expecting string, got value type %d", v->type);
-        FreeValue(v);
-        return NULL;
+        return false;
     }
-    char* result = v->data;
-    free(v);
-    return result;
+
+    *result = v->data;
+    return true;
 }
 
 Value* EvaluateValue(State* state, Expr* expr) {
     return expr->fn(expr->name, state, expr->argc, expr->argv);
 }
 
-Value* StringValue(char* str) {
-    if (str == NULL) return NULL;
-    Value* v = reinterpret_cast<Value*>(malloc(sizeof(Value)));
-    v->type = VAL_STRING;
-    v->size = strlen(str);
-    v->data = str;
-    return v;
+Value* StringValue(const char* str) {
+    if (str == nullptr) {
+        return nullptr;
+    }
+    return new Value(VAL_STRING, str);
 }
 
-void FreeValue(Value* v) {
-    if (v == NULL) return;
-    free(v->data);
-    free(v);
+Value* StringValue(const std::string& str) {
+    return StringValue(str.c_str());
 }
 
 Value* ConcatFn(const char* name, State* state, int argc, Expr* argv[]) {
     if (argc == 0) {
-        return StringValue(strdup(""));
-    }
-    char** strings = reinterpret_cast<char**>(malloc(argc * sizeof(char*)));
-    int i;
-    for (i = 0; i < argc; ++i) {
-        strings[i] = NULL;
-    }
-    char* result = NULL;
-    int length = 0;
-    for (i = 0; i < argc; ++i) {
-        strings[i] = Evaluate(state, argv[i]);
-        if (strings[i] == NULL) {
-            goto done;
-        }
-        length += strlen(strings[i]);
+        return StringValue("");
     }
-
-    result = reinterpret_cast<char*>(malloc(length+1));
-    int p;
-    p = 0;
-    for (i = 0; i < argc; ++i) {
-        strcpy(result+p, strings[i]);
-        p += strlen(strings[i]);
+    std::string result;
+    for (int i = 0; i < argc; ++i) {
+        std::string str;
+        if (!Evaluate(state, argv[i], &str)) {
+            return nullptr;
+        }
+        result += str;
     }
-    result[p] = '\0';
 
-  done:
-    for (i = 0; i < argc; ++i) {
-        free(strings[i]);
-    }
-    free(strings);
     return StringValue(result);
 }
 
 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");
-        return NULL;
+        state->errmsg = "ifelse expects 2 or 3 arguments";
+        return nullptr;
     }
-    char* cond = Evaluate(state, argv[0]);
-    if (cond == NULL) {
-        return NULL;
+
+    std::string cond;
+    if (!Evaluate(state, argv[0], &cond)) {
+        return nullptr;
     }
 
-    if (BooleanString(cond) == true) {
-        free(cond);
+    if (!cond.empty()) {
         return EvaluateValue(state, argv[1]);
-    } else {
-        if (argc == 3) {
-            free(cond);
-            return EvaluateValue(state, argv[2]);
-        } else {
-            return StringValue(cond);
-        }
+    } else if (argc == 3) {
+        return EvaluateValue(state, argv[2]);
     }
+
+    return StringValue("");
 }
 
 Value* AbortFn(const char* name, State* state, int argc, Expr* argv[]) {
-    char* msg = NULL;
-    if (argc > 0) {
-        msg = Evaluate(state, argv[0]);
-    }
-    free(state->errmsg);
-    if (msg) {
+    std::string msg;
+    if (argc > 0 && Evaluate(state, argv[0], &msg)) {
         state->errmsg = msg;
     } else {
-        state->errmsg = strdup("called abort()");
+        state->errmsg = "called abort()";
     }
-    return NULL;
+    return nullptr;
 }
 
 Value* AssertFn(const char* name, State* state, int argc, Expr* argv[]) {
-    int i;
-    for (i = 0; i < argc; ++i) {
-        char* v = Evaluate(state, argv[i]);
-        if (v == NULL) {
-            return NULL;
+    for (int i = 0; i < argc; ++i) {
+        std::string result;
+        if (!Evaluate(state, argv[i], &result)) {
+            return nullptr;
         }
-        int b = BooleanString(v);
-        free(v);
-        if (!b) {
-            int prefix_len;
+        if (result.empty()) {
             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;
-            return NULL;
+            state->errmsg = "assert failed: " + state->script.substr(argv[i]->start, len);
+            return nullptr;
         }
     }
-    return StringValue(strdup(""));
+    return StringValue("");
 }
 
 Value* SleepFn(const char* name, State* state, int argc, Expr* argv[]) {
-    char* val = Evaluate(state, argv[0]);
-    if (val == NULL) {
-        return NULL;
+    std::string val;
+    if (!Evaluate(state, argv[0], &val)) {
+        return nullptr;
+    }
+
+    int v;
+    if (!android::base::ParseInt(val.c_str(), &v, 0)) {
+        return nullptr;
     }
-    int v = strtol(val, NULL, 10);
     sleep(v);
+
     return StringValue(val);
 }
 
 Value* StdoutFn(const char* name, State* state, int argc, Expr* argv[]) {
-    int i;
-    for (i = 0; i < argc; ++i) {
-        char* v = Evaluate(state, argv[i]);
-        if (v == NULL) {
-            return NULL;
+    for (int i = 0; i < argc; ++i) {
+        std::string v;
+        if (!Evaluate(state, argv[i], &v)) {
+            return nullptr;
         }
-        fputs(v, stdout);
-        free(v);
+        fputs(v.c_str(), stdout);
     }
-    return StringValue(strdup(""));
+    return StringValue("");
 }
 
 Value* LogicalAndFn(const char* name, State* state,
                    int argc, Expr* argv[]) {
-    char* left = Evaluate(state, argv[0]);
-    if (left == NULL) return NULL;
-    if (BooleanString(left) == true) {
-        free(left);
+    std::string left;
+    if (!Evaluate(state, argv[0], &left)) {
+        return nullptr;
+    }
+    if (BooleanString(left)) {
         return EvaluateValue(state, argv[1]);
     } else {
-        return StringValue(left);
+        return StringValue("");
     }
 }
 
 Value* LogicalOrFn(const char* name, State* state,
                    int argc, Expr* argv[]) {
-    char* left = Evaluate(state, argv[0]);
-    if (left == NULL) return NULL;
-    if (BooleanString(left) == false) {
-        free(left);
+    std::string left;
+    if (!Evaluate(state, argv[0], &left)) {
+        return nullptr;
+    }
+    if (!BooleanString(left)) {
         return EvaluateValue(state, argv[1]);
     } else {
         return StringValue(left);
@@ -217,105 +188,98 @@ Value* LogicalOrFn(const char* name, State* state,
 
 Value* LogicalNotFn(const char* name, State* state,
                     int argc, Expr* argv[]) {
-    char* val = Evaluate(state, argv[0]);
-    if (val == NULL) return NULL;
-    bool bv = BooleanString(val);
-    free(val);
-    return StringValue(strdup(bv ? "" : "t"));
+    std::string val;
+    if (!Evaluate(state, argv[0], &val)) {
+        return nullptr;
+    }
+
+    return StringValue(BooleanString(val) ? "" : "t");
 }
 
 Value* SubstringFn(const char* name, State* state,
                    int argc, Expr* argv[]) {
-    char* needle = Evaluate(state, argv[0]);
-    if (needle == NULL) return NULL;
-    char* haystack = Evaluate(state, argv[1]);
-    if (haystack == NULL) {
-        free(needle);
-        return NULL;
+    std::string needle;
+    if (!Evaluate(state, argv[0], &needle)) {
+        return nullptr;
+    }
+
+    std::string haystack;
+    if (!Evaluate(state, argv[1], &haystack)) {
+        return nullptr;
     }
 
-    char* result = strdup(strstr(haystack, needle) ? "t" : "");
-    free(needle);
-    free(haystack);
+    std::string result = (haystack.find(needle) != std::string::npos) ? "t" : "";
     return StringValue(result);
 }
 
 Value* EqualityFn(const char* name, State* state, int argc, Expr* argv[]) {
-    char* left = Evaluate(state, argv[0]);
-    if (left == NULL) return NULL;
-    char* right = Evaluate(state, argv[1]);
-    if (right == NULL) {
-        free(left);
-        return NULL;
+    std::string left;
+    if (!Evaluate(state, argv[0], &left)) {
+        return nullptr;
+    }
+    std::string right;
+    if (!Evaluate(state, argv[1], &right)) {
+        return nullptr;
     }
 
-    char* result = strdup(strcmp(left, right) == 0 ? "t" : "");
-    free(left);
-    free(right);
+    const char* result = (left == right) ? "t" : "";
     return StringValue(result);
 }
 
 Value* InequalityFn(const char* name, State* state, int argc, Expr* argv[]) {
-    char* left = Evaluate(state, argv[0]);
-    if (left == NULL) return NULL;
-    char* right = Evaluate(state, argv[1]);
-    if (right == NULL) {
-        free(left);
-        return NULL;
+    std::string left;
+    if (!Evaluate(state, argv[0], &left)) {
+        return nullptr;
+    }
+    std::string right;
+    if (!Evaluate(state, argv[1], &right)) {
+        return nullptr;
     }
 
-    char* result = strdup(strcmp(left, right) != 0 ? "t" : "");
-    free(left);
-    free(right);
+    const char* result = (left != right) ? "t" : "";
     return StringValue(result);
 }
 
 Value* SequenceFn(const char* name, State* state, int argc, Expr* argv[]) {
-    Value* left = EvaluateValue(state, argv[0]);
-    if (left == NULL) return NULL;
-    FreeValue(left);
+    std::unique_ptr<Value> left(EvaluateValue(state, argv[0]));
+    if (!left) {
+        return nullptr;
+    }
     return EvaluateValue(state, argv[1]);
 }
 
 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");
-        return NULL;
+        state->errmsg = "less_than_int expects 2 arguments";
+        return nullptr;
     }
 
-    char* left;
-    char* right;
-    if (ReadArgs(state, argv, 2, &left, &right) < 0) return NULL;
-
-    bool result = false;
-    char* end;
-
-    long l_int = strtol(left, &end, 10);
-    if (left[0] == '\0' || *end != '\0') {
-        goto done;
+    std::vector<std::string> args;
+    if (!ReadArgs(state, 2, argv, &args)) {
+        return nullptr;
     }
 
-    long r_int;
-    r_int = strtol(right, &end, 10);
-    if (right[0] == '\0' || *end != '\0') {
-        goto done;
+    // Parse up to at least long long or 64-bit integers.
+    int64_t l_int;
+    if (!android::base::ParseInt(args[0].c_str(), &l_int)) {
+        state->errmsg = "failed to parse int in " + args[0];
+        return nullptr;
     }
 
-    result = l_int < r_int;
+    int64_t r_int;
+    if (!android::base::ParseInt(args[1].c_str(), &r_int)) {
+        state->errmsg = "failed to parse int in " + args[1];
+        return nullptr;
+    }
 
-  done:
-    free(left);
-    free(right);
-    return StringValue(strdup(result ? "t" : ""));
+    return StringValue(l_int < r_int ? "t" : "");
 }
 
 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");
-        return NULL;
+        state->errmsg = "greater_than_int expects 2 arguments";
+        return nullptr;
     }
 
     Expr* temp[2];
@@ -326,64 +290,25 @@ Value* GreaterThanIntFn(const char* name, State* state,
 }
 
 Value* Literal(const char* name, State* state, int argc, Expr* argv[]) {
-    return StringValue(strdup(name));
-}
-
-Expr* Build(Function fn, YYLTYPE loc, int count, ...) {
-    va_list v;
-    va_start(v, count);
-    Expr* e = reinterpret_cast<Expr*>(malloc(sizeof(Expr)));
-    e->fn = fn;
-    e->name = "(operator)";
-    e->argc = count;
-    e->argv = reinterpret_cast<Expr**>(malloc(count * sizeof(Expr*)));
-    int i;
-    for (i = 0; i < count; ++i) {
-        e->argv[i] = va_arg(v, Expr*);
-    }
-    va_end(v);
-    e->start = loc.start;
-    e->end = loc.end;
-    return e;
+    return StringValue(name);
 }
 
 // -----------------------------------------------------------------
 //   the function table
 // -----------------------------------------------------------------
 
-static int fn_entries = 0;
-static int fn_size = 0;
-NamedFunction* fn_table = NULL;
-
-void RegisterFunction(const char* name, Function fn) {
-    if (fn_entries >= fn_size) {
-        fn_size = fn_size*2 + 1;
-        fn_table = reinterpret_cast<NamedFunction*>(realloc(fn_table, fn_size * sizeof(NamedFunction)));
-    }
-    fn_table[fn_entries].name = name;
-    fn_table[fn_entries].fn = fn;
-    ++fn_entries;
-}
+static std::unordered_map<std::string, Function> fn_table;
 
-static int fn_entry_compare(const void* a, const void* b) {
-    const char* na = ((const NamedFunction*)a)->name;
-    const char* nb = ((const NamedFunction*)b)->name;
-    return strcmp(na, nb);
+void RegisterFunction(const std::string& name, Function fn) {
+    fn_table[name] = fn;
 }
 
-void FinishRegistration() {
-    qsort(fn_table, fn_entries, sizeof(NamedFunction), fn_entry_compare);
-}
-
-Function FindFunction(const char* name) {
-    NamedFunction key;
-    key.name = name;
-    NamedFunction* nf = reinterpret_cast<NamedFunction*>(bsearch(&key, fn_table, fn_entries,
-            sizeof(NamedFunction), fn_entry_compare));
-    if (nf == NULL) {
-        return NULL;
+Function FindFunction(const std::string& name) {
+    if (fn_table.find(name) == fn_table.end()) {
+        return nullptr;
+    } else {
+        return fn_table[name];
     }
-    return nf->fn;
 }
 
 void RegisterBuiltins() {
@@ -404,106 +329,41 @@ void RegisterBuiltins() {
 //   convenience methods for functions
 // -----------------------------------------------------------------
 
-// Evaluate the expressions in argv, giving 'count' char* (the ... is
-// zero or more char** to put them in).  If any expression evaluates
-// to NULL, free the rest and return -1.  Return 0 on success.
-int ReadArgs(State* state, Expr* argv[], int count, ...) {
-    char** args = reinterpret_cast<char**>(malloc(count * sizeof(char*)));
-    va_list v;
-    va_start(v, count);
-    int i;
-    for (i = 0; i < count; ++i) {
-        args[i] = Evaluate(state, argv[i]);
-        if (args[i] == NULL) {
-            va_end(v);
-            int j;
-            for (j = 0; j < i; ++j) {
-                free(args[j]);
-            }
-            free(args);
-            return -1;
-        }
-        *(va_arg(v, char**)) = args[i];
+// Evaluate the expressions in argv, and put the results of strings in
+// args. If any expression evaluates to nullptr, free the rest and return
+// false. Return true on success.
+bool ReadArgs(State* state, int argc, Expr* argv[], std::vector<std::string>* args) {
+    if (args == nullptr) {
+        return false;
     }
-    va_end(v);
-    free(args);
-    return 0;
-}
-
-// Evaluate the expressions in argv, giving 'count' Value* (the ... is
-// zero or more Value** to put them in).  If any expression evaluates
-// to NULL, free the rest and return -1.  Return 0 on success.
-int ReadValueArgs(State* state, Expr* argv[], int count, ...) {
-    Value** args = reinterpret_cast<Value**>(malloc(count * sizeof(Value*)));
-    va_list v;
-    va_start(v, count);
-    int i;
-    for (i = 0; i < count; ++i) {
-        args[i] = EvaluateValue(state, argv[i]);
-        if (args[i] == NULL) {
-            va_end(v);
-            int j;
-            for (j = 0; j < i; ++j) {
-                FreeValue(args[j]);
-            }
-            free(args);
-            return -1;
+    for (int i = 0; i < argc; ++i) {
+        std::string var;
+        if (!Evaluate(state, argv[i], &var)) {
+            args->clear();
+            return false;
         }
-        *(va_arg(v, Value**)) = args[i];
+        args->push_back(var);
     }
-    va_end(v);
-    free(args);
-    return 0;
+    return true;
 }
 
-// Evaluate the expressions in argv, returning an array of char*
-// results.  If any evaluate to NULL, free the rest and return NULL.
-// The caller is responsible for freeing the returned array and the
-// strings it contains.
-char** ReadVarArgs(State* state, int argc, Expr* argv[]) {
-    char** args = (char**)malloc(argc * sizeof(char*));
-    int i = 0;
-    for (i = 0; i < argc; ++i) {
-        args[i] = Evaluate(state, argv[i]);
-        if (args[i] == NULL) {
-            int j;
-            for (j = 0; j < i; ++j) {
-                free(args[j]);
-            }
-            free(args);
-            return NULL;
-        }
+// Evaluate the expressions in argv, and put the results of Value* in
+// args. If any expression evaluate to nullptr, free the rest and return
+// false. Return true on success.
+bool ReadValueArgs(State* state, int argc, Expr* argv[],
+                   std::vector<std::unique_ptr<Value>>* args) {
+    if (args == nullptr) {
+        return false;
     }
-    return args;
-}
-
-// Evaluate the expressions in argv, returning an array of Value*
-// results.  If any evaluate to NULL, free the rest and return NULL.
-// The caller is responsible for freeing the returned array and the
-// Values it contains.
-Value** ReadValueVarArgs(State* state, int argc, Expr* argv[]) {
-    Value** args = (Value**)malloc(argc * sizeof(Value*));
-    int i = 0;
-    for (i = 0; i < argc; ++i) {
-        args[i] = EvaluateValue(state, argv[i]);
-        if (args[i] == NULL) {
-            int j;
-            for (j = 0; j < i; ++j) {
-                FreeValue(args[j]);
-            }
-            free(args);
-            return NULL;
+    for (int i = 0; i < argc; ++i) {
+        std::unique_ptr<Value> v(EvaluateValue(state, argv[i]));
+        if (!v) {
+            args->clear();
+            return false;
         }
+        args->push_back(std::move(v));
     }
-    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;
+    return true;
 }
 
 // Use printf-style arguments to compose an error message to put into
@@ -511,7 +371,7 @@ static void ErrorAbortV(State* state, const char* format, va_list ap) {
 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;
 }
@@ -519,8 +379,14 @@ Value* ErrorAbort(State* state, const char* format, ...) {
 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 886347991..911adbc82 100644
--- a/edify/expr.h
+++ b/edify/expr.h
@@ -18,28 +18,24 @@
 #define _EXPRESSION_H
 
 #include <unistd.h>
+#include <string>
 
 #include "error_code.h"
-#include "yydefs.h"
 
-#define MAX_STRING_LEN 1024
+struct State {
+    State(const std::string& script, void* cookie);
 
-typedef struct Expr Expr;
+    // The source of the original script.
+    const std::string& script;
 
-typedef struct {
     // 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,20 +46,26 @@ typedef struct {
     CauseCode cause_code = kNoCause;
 
     bool is_retry = false;
+};
 
-} State;
+enum ValueType {
+    VAL_INVALID = -1,
+    VAL_STRING = 1,
+    VAL_BLOB = 2,
+};
 
-#define VAL_STRING  1  // data will be NULL-terminated; size doesn't count null
-#define VAL_BLOB    2
+struct Value {
+    ValueType type;
+    std::string data;
 
-typedef struct {
-    int type;
-    ssize_t size;
-    char* data;
-} Value;
+    Value(ValueType type, const std::string& str) :
+        type(type),
+        data(str) {}
+};
 
-typedef Value* (*Function)(const char* name, State* state,
-                           int argc, Expr* argv[]);
+struct Expr;
+
+using Function = Value* (*)(const char* name, State* state, int argc, Expr* argv[]);
 
 struct Expr {
     Function fn;
@@ -79,11 +81,11 @@ struct Expr {
 Value* EvaluateValue(State* state, Expr* expr);
 
 // Take one of the Expr*s passed to the function as an argument,
-// evaluate it, assert that it is a string, and return the resulting
-// char*.  The caller takes ownership of the returned char*.  This is
-// a convenience function for older functions that want to deal only
-// with strings.
-char* Evaluate(State* state, Expr* expr);
+// evaluate it, assert that it is a string, and update the result
+// parameter. This function returns true if the evaluation succeeds.
+// This is a convenience function for older functions that want to
+// deal only with strings.
+bool Evaluate(State* state, Expr* expr, std::string* result);
 
 // Glue to make an Expr out of a literal.
 Value* Literal(const char* name, State* state, int argc, Expr* argv[]);
@@ -100,67 +102,33 @@ Value* EqualityFn(const char* name, State* state, int argc, Expr* argv[]);
 Value* InequalityFn(const char* name, State* state, int argc, Expr* argv[]);
 Value* SequenceFn(const char* name, State* state, int argc, Expr* argv[]);
 
-// Convenience function for building expressions with a fixed number
-// of arguments.
-Expr* Build(Function fn, YYLTYPE loc, int count, ...);
-
 // Global builtins, registered by RegisterBuiltins().
 Value* IfElseFn(const char* name, State* state, int argc, Expr* argv[]);
 Value* AssertFn(const char* name, State* state, int argc, Expr* argv[]);
 Value* AbortFn(const char* name, State* state, int argc, Expr* argv[]);
 
-
-// For setting and getting the global error string (when returning
-// NULL from a function).
-void SetError(const char* message);  // makes a copy
-const char* GetError();              // retains ownership
-void ClearError();
-
-
-typedef struct {
-  const char* name;
-  Function fn;
-} NamedFunction;
-
 // Register a new function.  The same Function may be registered under
 // multiple names, but a given name should only be used once.
-void RegisterFunction(const char* name, Function fn);
+void RegisterFunction(const std::string& name, Function fn);
 
 // Register all the builtins.
 void RegisterBuiltins();
 
-// Call this after all calls to RegisterFunction() but before parsing
-// any scripts to finish building the function table.
-void FinishRegistration();
-
 // Find the Function for a given name; return NULL if no such function
 // exists.
-Function FindFunction(const char* name);
-
+Function FindFunction(const std::string& name);
 
 // --- convenience functions for use in functions ---
 
-// Evaluate the expressions in argv, giving 'count' char* (the ... is
-// zero or more char** to put them in).  If any expression evaluates
-// to NULL, free the rest and return -1.  Return 0 on success.
-int ReadArgs(State* state, Expr* argv[], int count, ...);
-
-// Evaluate the expressions in argv, giving 'count' Value* (the ... is
-// zero or more Value** to put them in).  If any expression evaluates
-// to NULL, free the rest and return -1.  Return 0 on success.
-int ReadValueArgs(State* state, Expr* argv[], int count, ...);
-
-// Evaluate the expressions in argv, returning an array of char*
-// results.  If any evaluate to NULL, free the rest and return NULL.
-// The caller is responsible for freeing the returned array and the
-// strings it contains.
-char** ReadVarArgs(State* state, int argc, Expr* argv[]);
+// Evaluate the expressions in argv, and put the results of strings in
+// args. If any expression evaluates to nullptr, free the rest and return
+// false. Return true on success.
+bool ReadArgs(State* state, int argc, Expr* argv[], std::vector<std::string>* args);
 
-// Evaluate the expressions in argv, returning an array of Value*
-// results.  If any evaluate to NULL, free the rest and return NULL.
-// The caller is responsible for freeing the returned array and the
-// Values it contains.
-Value** ReadValueVarArgs(State* state, int argc, Expr* argv[]);
+// Evaluate the expressions in argv, and put the results of Value* in
+// args. If any expression evaluate to nullptr, free the rest and return
+// false. Return true on success.
+bool ReadValueArgs(State* state, int argc, Expr* argv[], std::vector<std::unique_ptr<Value>>* args);
 
 // Use printf-style arguments to compose an error message to put into
 // *state.  Returns NULL.
@@ -172,11 +140,10 @@ Value* ErrorAbort(State* state, const char* format, ...)
 Value* ErrorAbort(State* state, CauseCode cause_code, const char* format, ...)
     __attribute__((format(printf, 3, 4)));
 
-// Wrap a string into a Value, taking ownership of the string.
-Value* StringValue(char* str);
+// Copying the string into a Value.
+Value* StringValue(const char* str);
 
-// Free a Value object.
-void FreeValue(Value* v);
+Value* StringValue(const std::string& str);
 
 int parse_string(const char* str, Expr** root, int* error_count);
 
diff --git a/edify/main.cpp b/edify/main.cpp
deleted file mode 100644
index 6007a3d58..000000000
--- a/edify/main.cpp
+++ /dev/null
@@ -1,219 +0,0 @@
-/*
- * Copyright (C) 2009 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <string>
-
-#include "expr.h"
-#include "parser.h"
-
-extern int yyparse(Expr** root, int* error_count);
-
-int expect(const char* expr_str, const char* expected, int* errors) {
-    Expr* e;
-    char* result;
-
-    printf(".");
-
-    int error_count = parse_string(expr_str, &e, &error_count);
-    if (error_count > 0) {
-        printf("error parsing \"%s\" (%d errors)\n",
-               expr_str, error_count);
-        ++*errors;
-        return 0;
-    }
-
-    State state;
-    state.cookie = NULL;
-    state.script = strdup(expr_str);
-    state.errmsg = NULL;
-
-    result = Evaluate(&state, e);
-    free(state.errmsg);
-    free(state.script);
-    if (result == NULL && expected != NULL) {
-        printf("error evaluating \"%s\"\n", expr_str);
-        ++*errors;
-        return 0;
-    }
-
-    if (result == NULL && expected == NULL) {
-        return 1;
-    }
-
-    if (strcmp(result, expected) != 0) {
-        printf("evaluating \"%s\": expected \"%s\", got \"%s\"\n",
-               expr_str, expected, result);
-        ++*errors;
-        free(result);
-        return 0;
-    }
-
-    free(result);
-    return 1;
-}
-
-int test() {
-    int errors = 0;
-
-    expect("a", "a", &errors);
-    expect("\"a\"", "a", &errors);
-    expect("\"\\x61\"", "a", &errors);
-    expect("# this is a comment\n"
-           "  a\n"
-           "   \n",
-           "a", &errors);
-
-
-    // sequence operator
-    expect("a; b; c", "c", &errors);
-
-    // string concat operator
-    expect("a + b", "ab", &errors);
-    expect("a + \n \"b\"", "ab", &errors);
-    expect("a + b +\nc\n", "abc", &errors);
-
-    // string concat function
-    expect("concat(a, b)", "ab", &errors);
-    expect("concat(a,\n \"b\")", "ab", &errors);
-    expect("concat(a + b,\nc,\"d\")", "abcd", &errors);
-    expect("\"concat\"(a + b,\nc,\"d\")", "abcd", &errors);
-
-    // logical and
-    expect("a && b", "b", &errors);
-    expect("a && \"\"", "", &errors);
-    expect("\"\" && b", "", &errors);
-    expect("\"\" && \"\"", "", &errors);
-    expect("\"\" && abort()", "", &errors);   // test short-circuiting
-    expect("t && abort()", NULL, &errors);
-
-    // logical or
-    expect("a || b", "a", &errors);
-    expect("a || \"\"", "a", &errors);
-    expect("\"\" || b", "b", &errors);
-    expect("\"\" || \"\"", "", &errors);
-    expect("a || abort()", "a", &errors);     // test short-circuiting
-    expect("\"\" || abort()", NULL, &errors);
-
-    // logical not
-    expect("!a", "", &errors);
-    expect("! \"\"", "t", &errors);
-    expect("!!a", "t", &errors);
-
-    // precedence
-    expect("\"\" == \"\" && b", "b", &errors);
-    expect("a + b == ab", "t", &errors);
-    expect("ab == a + b", "t", &errors);
-    expect("a + (b == ab)", "a", &errors);
-    expect("(ab == a) + b", "b", &errors);
-
-    // substring function
-    expect("is_substring(cad, abracadabra)", "t", &errors);
-    expect("is_substring(abrac, abracadabra)", "t", &errors);
-    expect("is_substring(dabra, abracadabra)", "t", &errors);
-    expect("is_substring(cad, abracxadabra)", "", &errors);
-    expect("is_substring(abrac, axbracadabra)", "", &errors);
-    expect("is_substring(dabra, abracadabrxa)", "", &errors);
-
-    // ifelse function
-    expect("ifelse(t, yes, no)", "yes", &errors);
-    expect("ifelse(!t, yes, no)", "no", &errors);
-    expect("ifelse(t, yes, abort())", "yes", &errors);
-    expect("ifelse(!t, abort(), no)", "no", &errors);
-
-    // if "statements"
-    expect("if t then yes else no endif", "yes", &errors);
-    expect("if \"\" then yes else no endif", "no", &errors);
-    expect("if \"\" then yes endif", "", &errors);
-    expect("if \"\"; t then yes endif", "yes", &errors);
-
-    // numeric comparisons
-    expect("less_than_int(3, 14)", "t", &errors);
-    expect("less_than_int(14, 3)", "", &errors);
-    expect("less_than_int(x, 3)", "", &errors);
-    expect("less_than_int(3, x)", "", &errors);
-    expect("greater_than_int(3, 14)", "", &errors);
-    expect("greater_than_int(14, 3)", "t", &errors);
-    expect("greater_than_int(x, 3)", "", &errors);
-    expect("greater_than_int(3, x)", "", &errors);
-
-    // big string
-    expect(std::string(8192, 's').c_str(), std::string(8192, 's').c_str(), &errors);
-
-    printf("\n");
-
-    return errors;
-}
-
-void ExprDump(int depth, Expr* n, char* 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;
-    int i;
-    for (i = 0; i < n->argc; ++i) {
-        ExprDump(depth+1, n->argv[i], script);
-    }
-}
-
-int main(int argc, char** argv) {
-    RegisterBuiltins();
-    FinishRegistration();
-
-    if (argc == 1) {
-        return test() != 0;
-    }
-
-    FILE* f = fopen(argv[1], "r");
-    if (f == NULL) {
-        printf("%s: %s: No such file or directory\n", argv[0], argv[1]);
-        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);
-    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;
-
-        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);
-        } else {
-            printf("result is [%s]\n", result);
-        }
-    }
-    return 0;
-}
diff --git a/edify/parser.yy b/edify/parser.yy
index 098a6370a..58a8dec65 100644
--- a/edify/parser.yy
+++ b/edify/parser.yy
@@ -33,6 +33,25 @@ struct yy_buffer_state;
 void yy_switch_to_buffer(struct yy_buffer_state* new_buffer);
 struct yy_buffer_state* yy_scan_string(const char* yystr);
 
+// Convenience function for building expressions with a fixed number
+// of arguments.
+static Expr* Build(Function fn, YYLTYPE loc, size_t count, ...) {
+    va_list v;
+    va_start(v, count);
+    Expr* e = static_cast<Expr*>(malloc(sizeof(Expr)));
+    e->fn = fn;
+    e->name = "(operator)";
+    e->argc = count;
+    e->argv = static_cast<Expr**>(malloc(count * sizeof(Expr*)));
+    for (size_t i = 0; i < count; ++i) {
+        e->argv[i] = va_arg(v, Expr*);
+    }
+    va_end(v);
+    e->start = loc.start;
+    e->end = loc.end;
+    return e;
+}
+
 %}
 
 %locations
@@ -70,7 +89,7 @@ input:  expr           { *root = $1; }
 ;
 
 expr:  STRING {
-    $$ = reinterpret_cast<Expr*>(malloc(sizeof(Expr)));
+    $$ = static_cast<Expr*>(malloc(sizeof(Expr)));
     $$->fn = Literal;
     $$->name = $1;
     $$->argc = 0;
@@ -91,9 +110,9 @@ expr:  STRING {
 |  IF expr THEN expr ENDIF           { $$ = Build(IfElseFn, @$, 2, $2, $4); }
 |  IF expr THEN expr ELSE expr ENDIF { $$ = Build(IfElseFn, @$, 3, $2, $4, $6); }
 | STRING '(' arglist ')' {
-    $$ = reinterpret_cast<Expr*>(malloc(sizeof(Expr)));
+    $$ = static_cast<Expr*>(malloc(sizeof(Expr)));
     $$->fn = FindFunction($1);
-    if ($$->fn == NULL) {
+    if ($$->fn == nullptr) {
         char buffer[256];
         snprintf(buffer, sizeof(buffer), "unknown function \"%s\"", $1);
         yyerror(root, error_count, buffer);
@@ -113,12 +132,12 @@ arglist:    /* empty */ {
 }
 | expr {
     $$.argc = 1;
-    $$.argv = reinterpret_cast<Expr**>(malloc(sizeof(Expr*)));
+    $$.argv = static_cast<Expr**>(malloc(sizeof(Expr*)));
     $$.argv[0] = $1;
 }
 | arglist ',' expr {
     $$.argc = $1.argc + 1;
-    $$.argv = reinterpret_cast<Expr**>(realloc($$.argv, $$.argc * sizeof(Expr*)));
+    $$.argv = static_cast<Expr**>(realloc($$.argv, $$.argc * sizeof(Expr*)));
     $$.argv[$$.argc-1] = $3;
 }
 ;