unix, win: rework uv_dlopen() API

joyent · May 3, 2012 · 5d19aa8 · 5d19aa8 · txdv · May 5, 2012
1 parent 93d16e6
commit 5d19aa8
Show file tree

Hide file tree

Showing 6 changed files with 114 additions and 96 deletions.
diff --git a/include/uv-private/uv-unix.h b/include/uv-private/uv-unix.h
@@ -59,8 +59,11 @@ typedef gid_t uv_gid_t;
 typedef uid_t uv_uid_t;
 
 /* Platform-specific definitions for uv_dlopen support. */
-typedef void* uv_lib_t;
 #define UV_DYNAMIC /* empty */
+typedef struct {
+  void* handle;
+  char* errmsg;
+} uv_lib_t;
 
 #define UV_HANDLE_TYPE_PRIVATE /* empty */
 #define UV_REQ_TYPE_PRIVATE /* empty */

diff --git a/include/uv-private/uv-win.h b/include/uv-private/uv-win.h
@@ -192,8 +192,11 @@ typedef unsigned char uv_uid_t;
 typedef unsigned char uv_gid_t;
 
 /* Platform-specific definitions for uv_dlopen support. */
-typedef HMODULE uv_lib_t;
 #define UV_DYNAMIC FAR WINAPI
+typedef struct {
+  HMODULE handle;
+  char* errmsg;
+} uv_lib_t;
 
 RB_HEAD(uv_timer_tree_s, uv_timer_s);
 

diff --git a/include/uv.h b/include/uv.h
@@ -1532,23 +1532,26 @@ UV_EXTERN extern uint64_t uv_hrtime(void);
 
 
 /*
- * Opens a shared library. The filename is in utf-8. On success, -1 is returned
- * and the variable pointed by library receives a handle to the library.
+ * Opens a shared library. The filename is in utf-8. Returns 0 on success and
+ * -1 on error. Call `uv_dlerror(uv_lib_t*)` to get the error message.
  */
-UV_EXTERN uv_err_t uv_dlopen(const char* filename, uv_lib_t* library);
-UV_EXTERN uv_err_t uv_dlclose(uv_lib_t library);
+UV_EXTERN int uv_dlopen(const char* filename, uv_lib_t* lib);
+
+/*
+ * Close the shared libary.
+ */
+UV_EXTERN void uv_dlclose(uv_lib_t* lib);
 
 /*
  * Retrieves a data pointer from a dynamic library. It is legal for a symbol to
- * map to NULL.
+ * map to NULL. Returns 0 on success and -1 if the symbol was not found.
  */
-UV_EXTERN uv_err_t uv_dlsym(uv_lib_t library, const char* name, void** ptr);
+UV_EXTERN int uv_dlsym(uv_lib_t* lib, const char* name, void** ptr);
 
 /*
- * Retrieves and frees an error message of dynamic linking loaders.
+ * Returns the last uv_dlopen() or uv_dlsym() error message.
  */
-UV_EXTERN const char *uv_dlerror(uv_lib_t library);
-UV_EXTERN void uv_dlerror_free(uv_lib_t library, const char *msg);
+UV_EXTERN const char* uv_dlerror(uv_lib_t* lib);
 
 /*
  * The mutex functions return 0 on success, -1 on error

diff --git a/src/unix/dl.c b/src/unix/dl.c
@@ -27,65 +27,61 @@
 #include <string.h>
 #include <locale.h>
 
-/* The dl family of functions don't set errno. We need a good way to communicate
- * errors to the caller but there is only dlerror() and that returns a string -
- * a string that may or may not be safe to keep a reference to...
- */
-static const uv_err_t uv_err_ = { UV_ENOENT, ENOENT };
-
+static int uv__dlerror(uv_lib_t* lib);
 
-uv_err_t uv_dlopen(const char* filename, uv_lib_t* library) {
-  void* handle = dlopen(filename, RTLD_LAZY);
-  if (handle == NULL) {
-    return uv_err_;
-  }
 
-  *library = handle;
-  return uv_ok_;
+int uv_dlopen(const char* filename, uv_lib_t* lib) {
+  lib->errmsg = NULL;
+  lib->handle = dlopen(filename, RTLD_LAZY);
+  return uv__dlerror(lib);
 }
 
 
-uv_err_t uv_dlclose(uv_lib_t library) {
-  if (dlclose(library) != 0) {
-    return uv_err_;
+void uv_dlclose(uv_lib_t* lib) {
+  if (lib->errmsg) {
+    free(lib->errmsg);
+    lib->errmsg = NULL;
   }
 
-  return uv_ok_;
+  if (lib->handle) {
+    /* Ignore errors. No good way to signal them without leaking memory. */
+    dlclose(lib->handle);
+    lib->handle = NULL;
+  }
 }
 
 
-uv_err_t uv_dlsym(uv_lib_t library, const char* name, void** ptr) {
-  void* address;
-
-  /* Reset error status. */
-  dlerror();
-
-  address = dlsym(library, name);
+int uv_dlsym(uv_lib_t* lib, const char* name, void** ptr) {
+  dlerror(); /* Reset error status. */
+  *ptr = dlsym(lib->handle, name);
+  return uv__dlerror(lib);
+}
 
-  if (dlerror()) {
-    return uv_err_;
-  }
 
-  *ptr = (void*) address;
-  return uv_ok_;
+const char* uv_dlerror(uv_lib_t* lib) {
+  return lib->errmsg ? lib->errmsg : "no error";
 }
 
 
-const char *uv_dlerror(uv_lib_t library) {
-  const char* buf = NULL;
-  /* Make uv_dlerror() be independent of locale */
-  char* loc = setlocale(LC_MESSAGES, NULL);
-  if(strcmp(loc, "C") == 0) {
-    return strdup(dlerror());
+static int uv__dlerror(uv_lib_t* lib) {
+  char* errmsg;
+  char* locale;
+
+  /* Make error message independent of locale. */
+  locale = setlocale(LC_MESSAGES, NULL);
+  if (strcmp(locale, "C") == 0) {
+    errmsg = dlerror();
   } else {
     setlocale(LC_MESSAGES, "C");
-    buf = dlerror();
-    setlocale(LC_MESSAGES, loc);
-    return strdup(buf);
+    errmsg = dlerror();
+    setlocale(LC_MESSAGES, locale);
   }
-}
 
+  if (lib->errmsg)
+    free(lib->errmsg);
 
-void uv_dlerror_free(uv_lib_t library, const char *msg) {
-  free((void*)msg);
+  if (errmsg)
+    return lib->errmsg = strdup(errmsg), -1;
+  else
+    return lib->errmsg = NULL, 0;
 }
diff --git a/src/win/dl.c b/src/win/dl.c
@@ -22,61 +22,65 @@
 #include "uv.h"
 #include "internal.h"
 
-__declspec( thread ) DWORD saved_errno = 0;
+static int uv__dlerror(uv_lib_t* lib, int errorno);
 
-uv_err_t uv_dlopen(const char* filename, uv_lib_t* library) {
+
+int uv_dlopen(const char* filename, uv_lib_t* lib) {
   wchar_t filename_w[32768];
-  HMODULE handle;
 
-  if (!uv_utf8_to_utf16(filename,
-                        filename_w,
-                        sizeof(filename_w) / sizeof(wchar_t))) {
-    saved_errno = GetLastError();
-    return uv__new_sys_error(saved_errno);
+  lib->handle = NULL;
+  lib->errmsg = NULL;
+
+  if (!uv_utf8_to_utf16(filename, filename_w, ARRAY_SIZE(filename_w))) {
+    return uv__dlerror(lib, GetLastError());
   }
 
-  handle = LoadLibraryExW(filename_w, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
-  if (handle == NULL) {
-    saved_errno = GetLastError();
-    return uv__new_sys_error(saved_errno);
+  lib->handle = LoadLibraryExW(filename_w, NULL, LOAD_WITH_ALTERED_SEARCH_PATH);
+  if (lib->handle == NULL) {
+    return uv__dlerror(lib, GetLastError());
   }
 
-  *library = handle;
-  return uv_ok_;
+  return 0;
 }
 
 
-uv_err_t uv_dlclose(uv_lib_t library) {
-  if (!FreeLibrary(library)) {
-    saved_errno = GetLastError();
-    return uv__new_sys_error(saved_errno);
+void uv_dlclose(uv_lib_t* lib) {
+  if (lib->errmsg) {
+    LocalFree((void*)lib->errmsg);
+    lib->errmsg = NULL;
   }
 
-  return uv_ok_;
+  if (lib->handle) {
+    /* Ignore errors. No good way to signal them without leaking memory. */
+    FreeLibrary(lib->handle);
+    lib->handle = NULL;
+  }
 }
 
 
-uv_err_t uv_dlsym(uv_lib_t library, const char* name, void** ptr) {
-  FARPROC proc = GetProcAddress(library, name);
-  if (proc == NULL) {
-    saved_errno = GetLastError();
-    return uv__new_sys_error(saved_errno);
-  }
-
-  *ptr = (void*) proc;
-  return uv_ok_;
+int uv_dlsym(uv_lib_t* lib, const char* name, void** ptr) {
+  *ptr = (void*) GetProcAddress(lib->handle, name);
+  return uv__dlerror(lib, *ptr ? 0 : GetLastError());
 }
 
 
-const char *uv_dlerror(uv_lib_t library) {
-  char* buf = NULL;
-  FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
-                 FORMAT_MESSAGE_IGNORE_INSERTS, NULL, saved_errno,
-                 MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US), (LPSTR)&buf, 0, NULL);
-  return buf;
+const char* uv_dlerror(uv_lib_t* lib) {
+  return lib->errmsg ? lib->errmsg : "no error";
 }
 
 
-void uv_dlerror_free(uv_lib_t library, const char *msg) {
-  LocalFree((LPVOID)msg);
+static int uv__dlerror(uv_lib_t* lib, int errorno) {
+  if (lib->errmsg) {
+    LocalFree((void*)lib->errmsg);
+    lib->errmsg = NULL;
+  }
+
+  if (errorno) {
+    FormatMessageA(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+                   FORMAT_MESSAGE_IGNORE_INSERTS, NULL, errorno,
+                   MAKELANGID(LANG_ENGLISH, SUBLANG_ENGLISH_US),
+                   (LPSTR)&lib->errmsg, 0, NULL);
+  }
+
+  return errorno ? -1 : 0;
 }
diff --git a/test/test-dlerror.c b/test/test-dlerror.c
@@ -23,8 +23,12 @@
 #include "task.h"
 #include <string.h>
 
-const char* path = "test/fixtures/load_error.node";
-const char* msg;
+
+TEST_IMPL(dlerror) {
+  const char* path = "test/fixtures/load_error.node";
+  const char* msg;
+  uv_lib_t lib;
+  int r;
 
 #ifdef __linux__
   const char* dlerror_desc = "file too short";
@@ -36,14 +40,19 @@ const char* msg;
   const char* dlerror_desc = "";
 #endif
 
-uv_lib_t lib;
-uv_err_t r;
-
-TEST_IMPL(dlerror) {
   r = uv_dlopen(path, &lib);
-  msg = uv_dlerror(lib);
+  ASSERT(r == -1);
+
+  msg = uv_dlerror(&lib);
   ASSERT(msg != NULL);
   ASSERT(strstr(msg, dlerror_desc) != NULL);
-  uv_dlerror_free(lib, msg);
+
+  /* Should return the same error twice in a row. */
+  msg = uv_dlerror(&lib);
+  ASSERT(msg != NULL);
+  ASSERT(strstr(msg, dlerror_desc) != NULL);
+
+  uv_dlclose(&lib);
+
   return 0;
 }