edify: Some clean-ups to libedify.

- Remove dead declarations in expr.h: SetError(), GetError(),
  ClearError().
- Remove the declaration of Build() out of expr.h.
- Use std::unordered_map to implement RegisterFunction() and
  FindFunction(); kill FinishRegistration().
- Add a testcase for calling unknown functions.

Test: mmma bootable/recovery; recovery_component_test passes.
Change-Id: I9af6825ae677f92b22d716a4a5682f58522af03b

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.

edify/edify_parser.cpp

@@ -45,7 +45,6 @@ static void ExprDump(int depth, const Expr* n, const std::string& script) {
 
 int main(int argc, char** argv) {
     RegisterBuiltins();
-    FinishRegistration();
 
     if (argc != 2) {
         printf("Usage: %s <edify script>\n", argv[0]);

edify/expr.cpp

@@ -14,20 +14,20 @@
  * limitations under the License.
  */
 
-#include <string.h>
+#include "expr.h"
-#include <stdbool.h>
+
+#include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
-#include <stdarg.h>
+#include <string.h>
 #include <unistd.h>
 
 #include <string>
+#include <unordered_map>
 
 #include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 
-#include "expr.h"
-
 // Functions should:
 //
 //    - return a malloc()'d string
@@ -319,61 +319,22 @@ 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;
-}
-
 // -----------------------------------------------------------------
 //   the function table
 // -----------------------------------------------------------------
 
-static int fn_entries = 0;
+static std::unordered_map<std::string, Function> fn_table;
-static int fn_size = 0;
-NamedFunction* fn_table = NULL;
 
-void RegisterFunction(const char* name, Function fn) {
+void RegisterFunction(const std::string& name, Function fn) {
-    if (fn_entries >= fn_size) {
+    fn_table[name] = fn;
-        fn_size = fn_size*2 + 1;
+}
-        fn_table = reinterpret_cast<NamedFunction*>(realloc(fn_table, fn_size * sizeof(NamedFunction)));
+
+Function FindFunction(const std::string& name) {
+    if (fn_table.find(name) == fn_table.end()) {
+        return nullptr;
+    } else {
+        return fn_table[name];
     }
-    fn_table[fn_entries].name = name;
-    fn_table[fn_entries].fn = fn;
-    ++fn_entries;
-}
-
-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 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;
-    }
-    return nf->fn;
 }
 
 void RegisterBuiltins() {

edify/expr.h

@@ -21,11 +21,6 @@
 #include <string>
 
 #include "error_code.h"
-#include "yydefs.h"
-
-#define MAX_STRING_LEN 1024
-
-typedef struct Expr Expr;
 
 struct State {
     State(const std::string& script, void* cookie);
@@ -56,14 +51,15 @@ struct State {
 #define VAL_STRING  1  // data will be NULL-terminated; size doesn't count null
 #define VAL_BLOB    2
 
-typedef struct {
+struct Value {
     int type;
     ssize_t size;
     char* data;
-} Value;
+};
 
-typedef Value* (*Function)(const char* name, State* state,
+struct Expr;
-                           int argc, Expr* argv[]);
+
+using Function = Value* (*)(const char* name, State* state, int argc, Expr* argv[]);
 
 struct Expr {
     Function fn;
@@ -100,43 +96,21 @@ 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 ---
 

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;
 }
 ;

tests/component/edify_test.cpp

@@ -22,17 +22,18 @@
 
 static void expect(const char* expr_str, const char* expected) {
     Expr* e;
-    int error_count;
+    int error_count = 0;
-    EXPECT_EQ(parse_string(expr_str, &e, &error_count), 0);
+    EXPECT_EQ(0, parse_string(expr_str, &e, &error_count));
+    EXPECT_EQ(0, error_count);
 
     State state(expr_str, nullptr);
 
     char* result = Evaluate(&state, e);
 
     if (expected == nullptr) {
-        EXPECT_EQ(result, nullptr);
+        EXPECT_EQ(nullptr, result);
     } else {
-        EXPECT_STREQ(result, expected);
+        EXPECT_STREQ(expected, result);
     }
 
     free(result);
@@ -42,7 +43,6 @@ class EdifyTest : public ::testing::Test {
   protected:
     virtual void SetUp() {
         RegisterBuiltins();
-        FinishRegistration();
     }
 };
 
@@ -149,3 +149,21 @@ TEST_F(EdifyTest, big_string) {
     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()";
+    Expr* expr;
+    int error_count = 0;
+    EXPECT_EQ(1, parse_string(script1, &expr, &error_count));
+    EXPECT_EQ(1, error_count);
+
+    const char* script2 = "abc; unknown_function()";
+    error_count = 0;
+    EXPECT_EQ(1, parse_string(script2, &expr, &error_count));
+    EXPECT_EQ(1, error_count);
+
+    const char* script3 = "unknown_function1() || yes";
+    error_count = 0;
+    EXPECT_EQ(1, parse_string(script3, &expr, &error_count));
+    EXPECT_EQ(1, error_count);
+}

tests/component/updater_test.cpp

@@ -54,7 +54,6 @@ class UpdaterTest : public ::testing::Test {
     virtual void SetUp() {
         RegisterBuiltins();
         RegisterInstallFunctions();
-        FinishRegistration();
     }
 };
 

updater/updater.cpp

@@ -111,7 +111,6 @@ int main(int argc, char** argv) {
     RegisterInstallFunctions();
     RegisterBlockImageFunctions();
     RegisterDeviceExtensions();
-    FinishRegistration();
 
     // Parse the script.