[erlang-bugs] driver_entry init() return value ignored

[Vance Shipley]

The driver_entry structure is a C struct that all erlang drivers
define.  It contains entry points for the erlang driver that are 
called by the erlang emulator when erlang code accesses the driver.

The first of the function pointers is init() which is called at
system startup for sdtaically linked drivers and after loading
for dynamically loaded drivers.  The init() function is documented
in erts-5.5.5/doc/html/driver_entry.html as:

   int init(void)

      This is called directly after the driver has been loaded by
      erl_ddll:load_driver/2. (Actually when the driver is added 
      to the driver list.) The driver should return 0, or if the
      driver can't initialize, -1.

In the R11B-5 implementation init() is actually defined as returning
void, the return value of the driver's init() callback is ignored.

I discovered this during development of a linked in driver which
was failing to initialize due to library path problems.  Instead
of erl_ddll:load_driver/1 failing with an appropriate error it was
succeeding and the problem wasn't recognized until much later in
the startup process.  This violates the principal of least astonishment.

The attached patch seems to resolve the problem and I have tested
it with my dynamically linked drivers.  I have not considered what
impact this change would have on a staticaly linked driver failing
however.

	-Vance

-------------- next part --------------
--- ./erts/emulator/beam/io.c	Mon Jun 11 08:52:02 2007
+++ ./erts/emulator/beam/io.c	Sat Aug  4 15:28:08 2007
@@ -4514,8 +4514,9 @@
     erts_add_driver_entry(drv,rec_lock != NULL);
 }
 
-void erts_add_driver_entry(ErlDrvEntry *drv, int driver_list_locked)
+int erts_add_driver_entry(ErlDrvEntry *drv, int driver_list_locked)
 {
+    int res;
     DE_List *p = erts_alloc(ERTS_ALC_T_DRV_ENTRY_LIST, sizeof(DE_List));
 
     if (!driver_list_locked) {
@@ -4534,12 +4535,13 @@
 	erts_smp_tsd_set(driver_list_lock_status_key, (void *) 1);
     }
     if (drv->init != NULL) {
-	(*drv->init)();
+	res = (*drv->init)();
     }
     if (!driver_list_locked) {
 	erts_smp_tsd_set(driver_list_lock_status_key, NULL);
 	erts_smp_mtx_unlock(&erts_driver_list_lock);
     }
+    return(res);
 }
 
 /* Not allowed for dynamic drivers */


--- ./erts/emulator/beam/erl_bif_ddll.c	Thu Mar  8 03:37:26 2007
+++ ./erts/emulator/beam/erl_bif_ddll.c	Sat Aug  4 15:36:24 2007
@@ -1606,9 +1606,10 @@
     dh->status = ERL_DE_OK;
     dp->handle = dh;
 
-    erts_add_driver_entry(dp,1); /* io.c */
-
-    return ERL_DE_NO_ERROR;
+    if (erts_add_driver_entry(dp,1) != 0)  /* io.c */
+	return ERL_DE_LOAD_ERROR_FAILED_INIT;
+    else
+	return ERL_DE_NO_ERROR;
 }
 
 static int do_unload_driver_entry(DE_Handle *dh, Eterm *save_name)


--- ./erts/emulator/drivers/common/ddll_drv.c	Mon Jan 29 07:53:37 2007
+++ ./erts/emulator/drivers/common/ddll_drv.c	Sat Aug  4 15:37:54 2007
@@ -252,9 +252,10 @@
     dp->handle = dh;
 
     /* insert driver */
-    add_driver_entry(dp);
-
-    return reply(erlang_port, 'o', "");
+    if(add_driver_entry(dp)!= 0)
+	return reply(erlang_port, 'e', "driver_init_failed");
+    else
+	return reply(erlang_port, 'o', "");
 }