Start checking inputs to dart api functions.

runtime/vm/dart_api_impl.cc

Index: runtime/vm/dart_api_impl.cc
===================================================================
--- runtime/vm/dart_api_impl.cc	(revision 1423)
+++ runtime/vm/dart_api_impl.cc	(working copy)
@@ -315,11 +315,15 @@
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
   TIMERSCOPE(time_script_loading);
-  const String& url_str = String::CheckedHandle(Api::UnwrapHandle(url));
-  const String& source_str = String::CheckedHandle(Api::UnwrapHandle(source));
+  String& url_str = String::Handle();
+  String& source_str = String::Handle();
+  UNWRAP_NONNULL(url, url_str, String);
+  UNWRAP_NONNULL(source, source_str, String);

[siva, 2011/11/15 22:09:49]

This seems pretty hard to read in my opinion, the declaration and the unwrapping
are disjoint and hard to associate.

It should be possible for us to do some macro magic using the macro CLASS_LIST
to have something like

const String& url_str = Api::UnwrapStringHandle(url);
if (url_str.IsNull()) {
  return REPORT_ERROR(url);  // Reports the proper error.
}

[turnidge, 2011/11/15 23:30:42]

Done.

   Library& library = Library::Handle(isolate->object_store()->root_library());
   if (!library.IsNull()) {
-    return Api::Error("Script already loaded");
+    const String& library_url = String::Handle(library.url());
+    return Api::Error("%s: A script has already been loaded from '%s'.",
+                      CURRENT_FUNC, library_url.ToCString());
   }
   isolate->set_library_tag_handler(handler);
   library = Library::New(url_str);
@@ -393,28 +397,23 @@
 DART_EXPORT Dart_Handle Dart_LibraryUrl(Dart_Handle library) {
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
-  const Library& lib = Library::CheckedHandle(Api::UnwrapHandle(library));
-  if (lib.IsNull()) {
-    return Api::Error("Null library");
-  }
+  Library& lib = Library::Handle();
+  UNWRAP_NONNULL(library, lib, Library);
   const String& url = String::Handle(lib.url());
   ASSERT(!url.IsNull());
   return Api::NewLocalHandle(url);
 }
 
 
-DART_EXPORT Dart_Handle Dart_LibraryImportLibrary(Dart_Handle library_in,
-                                                  Dart_Handle import_in) {
+DART_EXPORT Dart_Handle Dart_LibraryImportLibrary(Dart_Handle library,
+                                                  Dart_Handle import) {
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
-  const Library& library =
-      Library::CheckedHandle(Api::UnwrapHandle(library_in));
-  if (library.IsNull()) {
-    return Api::Error("Null library");
-  }
-  const Library& import =
-      Library::CheckedHandle(Api::UnwrapHandle(import_in));
-  library.AddImport(import);
+  Library& library_vm = Library::Handle();
+  Library& import_vm = Library::Handle();
+  UNWRAP_NONNULL(library, library_vm, Library);
+  UNWRAP_NONNULL(import, import_vm, Library);
+  library_vm.AddImport(import_vm);
   return Api::Success();
 }
 
@@ -437,12 +436,18 @@
 DART_EXPORT Dart_Handle Dart_LoadLibrary(Dart_Handle url, Dart_Handle source) {
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
-  const String& url_str = String::CheckedHandle(Api::UnwrapHandle(url));
-  const String& source_str = String::CheckedHandle(Api::UnwrapHandle(source));
+  String& url_str = String::Handle();
+  String& source_str = String::Handle();
+  UNWRAP_NONNULL(url, url_str, String);
+  UNWRAP_NONNULL(source, source_str, String);
   Library& library = Library::Handle(Library::LookupLibrary(url_str));
   if (library.IsNull()) {
     library = Library::New(url_str);
     library.Register();
+  } else if (library.name() != library.url()) {

[siva, 2011/11/15 22:09:49]

This is not something that we can rely to check that the library has already
been loaded, one could use the same name as the URL in the #library tag.

You should use the loaded_ field in RawLibrary to check this. We have a bug
currently which is not setting the loaded_ field, maybe that should first be
done in a CL and then we could check against that here.

[turnidge, 2011/11/15 23:30:42]

Ok.  I'll wait for the fix.

On 2011/11/15 22:09:49, asiva wrote:

[turnidge, 2011/11/18 00:50:15]

Updated.

On 2011/11/15 23:30:42, turnidge wrote:

+    // The source for this library has already been loaded.
+    return Api::Error("%s: library '%s' has already been loaded.",
+                      CURRENT_FUNC, url_str.ToCString());
   }
   Dart_Handle result;
   CompileSource(library, url_str, source_str, RawScript::kLibrary, &result);
@@ -450,17 +455,19 @@
 }
 
 
-DART_EXPORT Dart_Handle Dart_LoadSource(Dart_Handle library_in,
-                                        Dart_Handle url_in,
-                                        Dart_Handle source_in) {
+DART_EXPORT Dart_Handle Dart_LoadSource(Dart_Handle library,
+                                        Dart_Handle url,
+                                        Dart_Handle source) {
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
-  const String& url = String::CheckedHandle(Api::UnwrapHandle(url_in));
-  const String& source = String::CheckedHandle(Api::UnwrapHandle(source_in));
-  const Library& library =
-      Library::CheckedHandle(Api::UnwrapHandle(library_in));
+  Library& lib = Library::Handle();
+  String& url_str = String::Handle();
+  String& source_str = String::Handle();
+  UNWRAP_NONNULL(library, lib, Library);
+  UNWRAP_NONNULL(url, url_str, String);
+  UNWRAP_NONNULL(source, source_str, String);
   Dart_Handle result;
-  CompileSource(library, url, source, RawScript::kSource, &result);
+  CompileSource(lib, url_str, source_str, RawScript::kSource, &result);
   return result;
 }
 
@@ -470,10 +477,8 @@
     Dart_NativeEntryResolver resolver) {
   Zone zone;  // Setup a VM zone as we are creating some handles.
   HandleScope scope;  // Setup a VM handle scope.
-  const Library& lib = Library::CheckedHandle(Api::UnwrapHandle(library));
-  if (lib.IsNull()) {
-    return Api::Error("Invalid parameter, Unknown library specified");
-  }
+  Library& lib = Library::Handle();
+  UNWRAP_NONNULL(library, lib, Library);
   lib.set_native_entry_resolver(resolver);
   return Api::Success();
 }