Implement fromEnvironment on bool, int and String

runtime/bin/main.cc

Index: runtime/bin/main.cc
diff --git a/runtime/bin/main.cc b/runtime/bin/main.cc
index f8f308a0fcd1294bb4feec088f5ec14254d43744..b4ead74703a8d2578e3f698baf46bc3918ab4493 100644
--- a/runtime/bin/main.cc
+++ b/runtime/bin/main.cc
@@ -72,6 +72,8 @@ static bool start_vm_service = false;
 static int vm_service_server_port = -1;
 static const int DEFAULT_VM_SERVICE_SERVER_PORT = 8181;
 
+// The configuration provided through the command line using -D options.
+static HashMap* configuration = NULL;

[Ivan Posva, 2013/10/29 21:42:59]

environment to match the renamed const factory constructor. Here and everywhere
else.

[Søren Gjesse, 2013/10/30 11:44:02]

Done.

 
 static bool IsValidFlag(const char* name,
                         const char* prefix,
@@ -132,6 +134,48 @@ static bool ProcessPackageRootOption(const char* arg) {
 }
 
 
+static bool SameStringValue(void* key1, void* key2) {
+  return strcmp(reinterpret_cast<char*>(key1),
+                reinterpret_cast<char*>(key2)) == 0;
+}
+
+static void* GetHashmapKeyFromString(char* key) {
+  return reinterpret_cast<void*>(key);
+}
+
+static uint32_t GetHashmapHashFromString(char* key) {
+  return key == NULL ? 0 : *key;

[Ivan Posva, 2013/10/29 21:42:59]

Why bother with a hash table if there are only 26 likely hash values?  Just use
a linked list of entries, it is easier to understand and we do not expect to
have large numbers of entries here anyway.

[Søren Gjesse, 2013/10/30 11:44:02]

Added a real hash function to platform/hashmap.h and used that.

I can't see why a linked list would be better. The hash map implementation is
already there.

+}
+
+static bool ProcessDefineOption(const char* arg) {
+  ASSERT(arg != NULL);
+  if (*arg == '\0') {
+    return false;
+  }
+  if (configuration == NULL) configuration = new HashMap(&SameStringValue, 4);

[Ivan Posva, 2013/10/29 21:42:59]

if (...) {
  environment = ...
}

[Søren Gjesse, 2013/10/30 11:44:02]

Done.

+  char* name;

[Ivan Posva, 2013/10/29 21:42:59]

Please add comments about what you are doing here. For example:
// Split the args at "=".

[Søren Gjesse, 2013/10/30 11:44:02]

Done.

+  char* value = NULL;
+  const char* equals_pos = strchr(arg, '=');
+  if (equals_pos == NULL) {
+    name = strdup(arg);
+  } else {
+    int name_len = equals_pos - arg;
+    if (name_len == 0) return false;

[Ivan Posva, 2013/10/29 21:42:59]

if (...) {
  return false;
}

[Søren Gjesse, 2013/10/30 11:44:02]

Done.

+    name = reinterpret_cast<char*>(malloc(name_len + 1));
+    strncpy(name, arg, name_len);
+    *(name + name_len) = '\0';

[Ivan Posva, 2013/10/29 21:42:59]

name[name_len] = '\0';

[Søren Gjesse, 2013/10/30 11:44:02]

Done.

+    if (*(equals_pos + 1) != '\0') {
+      value = strdup(equals_pos + 1);
+    }
+  }
+  HashMap::Entry* entry = configuration->Lookup(
+      GetHashmapKeyFromString(name), GetHashmapHashFromString(name), true);
+  ASSERT(entry != NULL);

[Ivan Posva, 2013/10/29 21:42:59]

In a debug VM you will fail, but in a release VM you will just override. Just
define it to overwrite, like -D on a gcc command line and remove the assertion.
Alternatively if we do not want to allow overwriting, then you should return a
failure from here.

[Søren Gjesse, 2013/10/30 11:44:02]

When Lookup is called with the last argument set to true it will allocate a new
entry if it is not found. Therefore the ASSERT. Overwriting works fine. I will
add a test for overwrite.

+  entry->value = value != NULL ? value : const_cast<char*>("");

[Ivan Posva, 2013/10/29 21:42:59]

(value != NULL) ?

[Søren Gjesse, 2013/10/30 11:44:02]

That was wrong. A non-existent value is now passed as NULL.

+  return true;
+}
+
+
 static bool ProcessCompileAllOption(const char* arg) {
   ASSERT(arg != NULL);
   if (*arg != '\0') {
@@ -236,6 +280,7 @@ static struct {
   { "--verbose", ProcessVerboseOption },
   { "-v", ProcessVerboseOption },
   { "--package-root=", ProcessPackageRootOption },
+  { "-D", ProcessDefineOption },
   // VM specific options to the standalone dart program.
   { "--break-at=", ProcessBreakpointOption },
   { "--compile_all", ProcessCompileAllOption },
@@ -422,6 +467,37 @@ static Dart_Handle SetupRuntimeOptions(CommandLineOptions* options) {
   }                                                                            \
 
 
+static Dart_Handle ConfigCallback(Dart_ConfigType type, Dart_Handle name) {
+  uint8_t* utf8_array;
+  intptr_t utf8_len;
+  Dart_Handle result = Dart_Null();
+  Dart_Handle handle = Dart_StringToUTF8(name, &utf8_array, &utf8_len);
+  if (Dart_IsError(handle)) {
+    handle = Dart_ThrowException(
+        DartUtils::NewDartArgumentError(Dart_GetError(handle)));
+  } else {
+    char* name_chars = reinterpret_cast<char*>(malloc(utf8_len + 1));
+    memmove(name_chars, utf8_array, utf8_len);
+    name_chars[utf8_len] = '\0';
+    const char* value = NULL;
+    if (configuration != NULL) {
+      HashMap::Entry* entry = configuration->Lookup(
+          GetHashmapKeyFromString(name_chars),
+          GetHashmapHashFromString(name_chars),
+          false);
+      if (entry != NULL) {
+        value = reinterpret_cast<char*>(entry->value);
+      }
+    }
+    if (value != NULL) {
+      result = Dart_NewStringFromUTF8(reinterpret_cast<const uint8_t*>(value),
+                                      strlen(value));
+    }
+  }
+  return result;
+}
+
+
 // Returns true on success, false on failure.
 static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
                                                 const char* main,
@@ -446,6 +522,9 @@ static Dart_Isolate CreateIsolateAndSetupHelper(const char* script_uri,
   Dart_Handle result = Dart_SetLibraryTagHandler(DartUtils::LibraryTagHandler);
   CHECK_RESULT(result);
 
+  result = Dart_SetConfigCallback(ConfigCallback);
+  CHECK_RESULT(result);
+
   // Load the specified application script into the newly created isolate.
 
   // Prepare builtin and its dependent libraries for use to resolve URIs.