Make the NIF stateless!

Need to explicitly pass in socket info to the procket NIF, rather than it tracking the data internally. This should make it much safer and reliable. Changes to the interface: open/2 -> open/1 : vestigal protocol arg removed, stick to streams. Erlang module changed to match (along with the bizarre passing in of the port as a protocol, who did that? o_O) Returns the socket descriptor listening on the Unix socket: {ok, FD} poll/0 -> poll/1 : takes the socket descriptor close/0 -> close/2 : close(SocketPath, SocketDescriptor), closes the socket descriptor and deletes the socket path.
msantos · Jan 11, 2010 · 00214e3 · 00214e3
1 parent 6a15ed0
commit 00214e3
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 81 deletions.
diff --git a/c_src/procket.c b/c_src/procket.c
@@ -33,124 +33,99 @@
 #include "ancillary.h"
 #include "procket.h"
 
-typedef struct {
-    int fd;
-    char *path;
-} PRIVDATA;
+#define BACKLOG     5
 
 static ERL_NIF_TERM error_tuple(ErlNifEnv *env, char *atom, char *err);
 static int my_enif_get_string(ErlNifEnv *env, ERL_NIF_TERM list, char *buf, size_t buflen);
 static ERL_NIF_TERM error_message(ErlNifEnv *env, char *atom, char *err, char *msg);
-static ERL_NIF_TERM sock_close(ErlNifEnv *env);
 
 
     static int
 load(ErlNifEnv *env, void **priv, ERL_NIF_TERM load_info)
 {
-    PRIVDATA *data = NULL;
-
-
-    data = (PRIVDATA *)enif_alloc(env, sizeof(PRIVDATA));
-    if (data == NULL)
-        return (-1);
-
-    (void)memset(data, '\0', sizeof(PRIVDATA));
-    *priv = data;
-
     return (0);
 }   
 
 
     static int
 reload(ErlNifEnv *env, void **priv, ERL_NIF_TERM load_info)
 {
-    (void)sock_close(env);
-    enif_free(env, ((PRIVDATA *)*priv)->path);
-    enif_free(env, *priv);
-
     return load(env, priv, load_info);
 }
 
 
     static ERL_NIF_TERM
-sock_open(ErlNifEnv *env, ERL_NIF_TERM path, ERL_NIF_TERM protocol)
+sock_open(ErlNifEnv *env, ERL_NIF_TERM path)
 {
-    PRIVDATA *data = NULL;
-    int proto = 0;
+    char *sock_path = NULL;
+    int sock_fd = -1;
+
     struct sockaddr_un sa = { 0 };
     int flags = 0;
 
 
-    data = (PRIVDATA *)enif_get_data(env);
-
-    if (data->path != NULL)
-        return error_tuple(env, "error", "socket_already_open");
-
-    data->path = (char *)enif_alloc(env, UNIX_PATH_MAX);
-    if (data->path == NULL)
+    sock_path = (char *)enif_alloc(env, UNIX_PATH_MAX);
+    if (sock_path == NULL)
         return error_tuple(env, "error", "memory_allocation_failure");
 
-    if (!my_enif_get_string(env, path, data->path, UNIX_PATH_MAX))
-        return enif_make_badarg(env);
-
-    if (strlen(data->path) == 0)
+    if (!my_enif_get_string(env, path, sock_path, UNIX_PATH_MAX))
         return enif_make_badarg(env);
 
-    if (!enif_get_int(env, protocol, &proto))
+    if (strlen(sock_path) == 0)
         return enif_make_badarg(env);
 
     sa.sun_family = PF_LOCAL;
-    (void)memcpy(sa.sun_path, data->path, sizeof(sa.sun_path)-1);
+    (void)memcpy(sa.sun_path, sock_path, sizeof(sa.sun_path)-1);
 
     errno = 0;
-    data->fd = socket(PF_LOCAL, SOCK_STREAM, 0);
-    if (data->fd < 0)
+    sock_fd = socket(PF_LOCAL, SOCK_STREAM, 0);
+    if (sock_fd < 0)
         return error_message(env, "error", "socket", strerror(errno));
 
-    flags = fcntl(data->fd, F_GETFL, 0);
+    flags = fcntl(sock_fd, F_GETFL, 0);
     flags |= O_NONBLOCK;
-    (void)fcntl(data->fd, F_SETFL, flags);
+    (void)fcntl(sock_fd, F_SETFL, flags);
 
     errno = 0;
-    if (bind(data->fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
+    if (bind(sock_fd, (struct sockaddr *)&sa, sizeof(sa)) < 0)
         return error_message(env, "error", "bind", strerror(errno));
 
     errno = 0;
-    if (listen(data->fd, 5) < 0)
+    if (listen(sock_fd, BACKLOG) < 0)
         return error_message(env, "error", "listen", strerror(errno));
 
-    return enif_make_atom(env, "ok");
+    enif_free(env, sock_path);
+    return enif_make_tuple(env, 2,
+           enif_make_atom(env, "ok"),
+           enif_make_int(env, sock_fd));
 }
 
 
     static ERL_NIF_TERM
-poll(ErlNifEnv *env)
+poll(ErlNifEnv *env, ERL_NIF_TERM socket)
 {
-    PRIVDATA *data = NULL;
-    int s= 0;
-    int pipefd = 0;
+    int sock_fd = -1;        /* listening socket */
+    int fd = -1;             /* connected socket */
+    int s = -1;              /* socket received from pipe */
     struct sockaddr_un sa = { 0 };
     socklen_t socklen = 0;
 
 
-    data = (PRIVDATA *)enif_get_data(env);
-
-    if (data->path == NULL)
-        return error_tuple(env, "error", "no_socket");
+    if (!enif_get_int(env, socket, &sock_fd))
+        return enif_make_badarg(env);
 
     errno = 0;
-    pipefd = accept(data->fd, (struct sockaddr *)&sa, &socklen);
-    if (pipefd < 0)
+    fd = accept(sock_fd, (struct sockaddr *)&sa, &socklen);
+    if (fd < 0)
         return error_message(env, "error", "accept", strerror(errno));
 
     errno = 0;
-    if (ancil_recv_fd(pipefd, &s) < 0) {
-        (void)close (pipefd);
+    if (ancil_recv_fd(fd, &s) < 0) {
+        (void)close (fd);
         return error_message(env, "error", "recvmsg", strerror(errno));
     }
 
-    (void)close (pipefd);
-    (void)sock_close(env);
+    (void)close (fd);
 
     return enif_make_tuple(env, 2,
             enif_make_atom(env, "ok"),
@@ -159,28 +134,29 @@ poll(ErlNifEnv *env)
 
 
     static ERL_NIF_TERM
-sock_close(ErlNifEnv *env)
+sock_close(ErlNifEnv *env, ERL_NIF_TERM path, ERL_NIF_TERM fd)
 {
-    PRIVDATA *data = NULL;
-
+    char *sock_path = NULL;
+    int sockfd = -1;
 
-    data = (PRIVDATA *)enif_get_data(env);
 
-    if (data->path == NULL)
-        return error_tuple(env, "error", "no_socket");
+    sock_path = (char *)enif_alloc(env, UNIX_PATH_MAX);
+    if (sock_path == NULL)
+        return error_tuple(env, "error", "memory_allocation_failure");
 
-    (void)close(data->fd);
+    if (!my_enif_get_string(env, path, sock_path, UNIX_PATH_MAX))
+        return enif_make_badarg(env);
 
-    errno = 0;
-    if (unlink(data->path) < 0)
-        return error_message(env, "error", "unlink", strerror(errno));
+    if (!enif_get_int(env, fd, &sockfd))
+        return enif_make_badarg(env);
 
-    if (data->path) {
-        enif_free(env, data->path);
-        data->path = NULL;
+    if (strlen(sock_path) != 0) {
+        errno = 0;
+        if (unlink(sock_path) < 0)
+            return error_message(env, "error", "unlink", strerror(errno));
     }
 
-    data->fd = -1;
+    (void)close(sockfd);
 
     return enif_make_atom(env, "ok");
 }
@@ -231,9 +207,9 @@ my_enif_get_string(ErlNifEnv *env, ERL_NIF_TERM list, char *buf, size_t buflen)
 }
 
 static ErlNifFunc nif_funcs[] = {
-    {"open", 2, sock_open},
-    {"poll", 0, poll},
-    {"close", 0, sock_close}
+    {"open", 1, sock_open},
+    {"poll", 1, poll},
+    {"close", 2, sock_close}
 };
 
 ERL_NIF_INIT(procket, nif_funcs, load, reload, NULL, NULL)

diff --git a/c_src/procket_cmd.c b/c_src/procket_cmd.c
@@ -203,7 +203,7 @@ procket_pipe(PROCKET_STATE *ps)
 
     (void)memcpy(sa.sun_path, ps->path, sizeof(sa.sun_path)-1);
 
-    sa.sun_family = AF_UNIX;
+    sa.sun_family = PF_LOCAL;
 
     IS_LTZERO(s = socket(PF_LOCAL, SOCK_STREAM, 0));
     if (connect(s, (struct sockaddr *)&sa, sizeof(sa)) < 0)

diff --git a/src/procket.erl b/src/procket.erl
@@ -30,7 +30,7 @@
 %% POSSIBILITY OF SUCH DAMAGE.
 -module(procket).
 
--export([init/0,open/2,poll/0,close/0,listen/1,listen/2]).
+-export([init/0,open/1,poll/1,close/2,listen/1,listen/2]).
 -export([make_args/2]).
 
 -define(PROGNAME, "priv/procket").
@@ -44,13 +44,13 @@ on_load() ->
     ok = erlang:load_nif("priv/procket", []),
     true.
 
-open(_,_) ->
+open(_) ->
     erlang:error(not_implemented).
 
-poll() ->
+poll(_) ->
     erlang:error(not_implemented).
 
-close() ->
+close(_,_) ->
     erlang:error(not_implemented).
 
 listen(Port) ->
@@ -60,13 +60,15 @@ listen(Port, Options) when is_integer(Port), is_list(Options) ->
         none -> Options ++ [{pipe, mktmp:dir() ++ "/sock"}];
         _ -> Options
     end,
-    ok = open(proplists:get_value(pipe, Opt), Port),
+    {ok, Sockfd} = open(proplists:get_value(pipe, Opt)),
     Cmd = make_args(Port, Opt),
     case os:cmd(Cmd) of
         [] ->
-            poll();
+            FD = poll(Sockfd),
+            close(proplists:get_value(pipe, Opt), Sockfd),
+            FD;
         Error ->
-            {error, procket_cmd, Error}
+            {error, {procket_cmd, Error}}
     end.
 
 make_args(Port, Options) ->