Defined sk_close() which closes the socket safely even if called from

socket hook. Replaces the SK_DELETED hack.

Squashed a couple of bugs in handling of TCP sockets.

3 files changed, 47 insertions, 20 deletions

diff --git a/lib/socket.h b/lib/socket.h
index 65ba9a2..147e5ce 100644
--- a/lib/socket.h
+++ b/lib/socket.h
@@ -38,10 +38,12 @@ typedef struct birdsock {
 
   int fd;				/* System-dependent data */
   node n;
+  int entered;
 } sock;
 
 sock *sk_new(pool *);			/* Allocate new socket */
 int sk_open(sock *);			/* Open socket */
+void sk_close(sock *);			/* Safe close of socket even from socket hook */
 int sk_send(sock *, unsigned len);	/* Send data, <0=err, >0=ok, 0=sleep */
 int sk_send_to(sock *, unsigned len, ip_addr to, unsigned port); /* sk_send to given destination */
 void sk_dump_all(void);
@@ -66,7 +68,7 @@ sk_send_buffer_empty(sock *sk)
 #define SK_MAGIC	7	   /* Internal use by sysdep code */
 #define SK_UNIX_PASSIVE	8
 #define SK_UNIX		9
-#define SK_DELETED	10	   /* Set to this if you want to delete socket from err_hook */
+#define SK_DELETED	10	   /* Internal use by sk_close */
 
 /*
  *  Multicast sockets are slightly different from the other ones:
diff --git a/sysdep/unix/io.c b/sysdep/unix/io.c
index 69f3f06..0925609 100644
--- a/sysdep/unix/io.c
+++ b/sysdep/unix/io.c
@@ -336,7 +336,10 @@ sk_free(resource *r)
   sock *s = (sock *) r;
 
   if (s->fd >= 0)
-    rem_node(&s->n);
+    {
+      close(s->fd);
+      rem_node(&s->n);
+    }
 }
 
 static void
@@ -382,6 +385,7 @@ sk_new(pool *p)
   s->tbsize = 0;
   s->err_hook = NULL;
   s->fd = -1;
+  s->entered = 0;
   return s;
 }
 
@@ -477,9 +481,9 @@ sk_alloc_bufs(sock *s)
 static void
 sk_tcp_connected(sock *s)
 {
-  s->rx_hook(s, 0);
   s->type = SK_TCP;
   sk_alloc_bufs(s);
+  s->tx_hook(s);
 }
 
 static int
@@ -524,6 +528,8 @@ sk_open(sock *s)
   switch (type)
     {
     case SK_TCP_ACTIVE:
+      s->ttx = "";			/* Force s->ttx != s->tpos */
+      /* Fall thru */
     case SK_TCP_PASSIVE:
       fd = socket(BIRD_PF, SOCK_STREAM, IPPROTO_TCP);
       break;
@@ -625,6 +631,7 @@ sk_open(sock *s)
     case SK_MAGIC:
       break;
     default:
+      sk_alloc_bufs(s);
 #ifdef IPV6
 #ifdef IPV6_MTU_DISCOVER
       {
@@ -644,7 +651,6 @@ sk_open(sock *s)
 #endif
     }
 
-  sk_alloc_bufs(s);
   add_tail(&sock_list, &s->n);
   return 0;
 
@@ -686,6 +692,15 @@ bad:
   return -1;
 }
 
+void
+sk_close(sock *s)
+{
+  if (s->entered)
+    s->type = SK_DELETED;
+  else
+    rfree(s);
+}
+
 static int
 sk_maybe_write(sock *s)
 {
@@ -767,19 +782,6 @@ sk_read(sock *s)
 {
   switch (s->type)
     {
-    case SK_TCP_ACTIVE:
-      {
-	sockaddr sa;
-	fill_in_sockaddr(&sa, s->daddr, s->dport);
-	if (connect(s->fd, (struct sockaddr *) &sa, sizeof(sa)) >= 0)
-	  sk_tcp_connected(s);
-	else if (errno != EINTR && errno != EAGAIN && errno != EINPROGRESS)
-	  {
-	    log(L_ERR "connect: %m");
-	    s->err_hook(s, errno);
-	  }
-	return 0;
-      }
     case SK_TCP_PASSIVE:
       {
 	sockaddr sa;
@@ -816,6 +818,8 @@ sk_read(sock *s)
       }
     case SK_MAGIC:
       return s->rx_hook(s, 0);
+    case SK_DELETED:
+      return 0;
     default:
       {
 	sockaddr sa;
@@ -842,8 +846,27 @@ sk_read(sock *s)
 static void
 sk_write(sock *s)
 {
-  while (s->ttx != s->tbuf && sk_maybe_write(s) > 0)
-    s->tx_hook(s);
+  switch (s->type)
+    {
+    case SK_TCP_ACTIVE:
+      {
+	sockaddr sa;
+	fill_in_sockaddr(&sa, s->daddr, s->dport);
+	if (connect(s->fd, (struct sockaddr *) &sa, sizeof(sa)) >= 0)
+	  sk_tcp_connected(s);
+	else if (errno != EINTR && errno != EAGAIN && errno != EINPROGRESS)
+	  {
+	    log(L_ERR "connect: %m");
+	    s->err_hook(s, errno);
+	  }
+	break;
+      }
+    case SK_DELETED:
+      return;
+    default:
+      while (s->ttx != s->tbuf && sk_maybe_write(s) > 0)
+	s->tx_hook(s);
+    }
 }
 
 void
@@ -965,6 +988,7 @@ io_loop(void)
 	  WALK_LIST_DELSAFE(n, p, sock_list)
 	    {
 	      s = SKIP_BACK(sock, n, n);
+	      s->entered = 1;
 	      if (FD_ISSET(s->fd, &rd))
 		{
 		  FD_CLR(s->fd, &rd);
@@ -976,6 +1000,7 @@ io_loop(void)
 		  FD_CLR(s->fd, &wr);
 		  sk_write(s);
 		}
+	      s->entered = 0;
 	      if (s->type == SK_DELETED)
 		rfree(s);
 	    }
diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c
index c6f4893..dcd921f 100644
--- a/sysdep/unix/main.c
+++ b/sysdep/unix/main.c
@@ -236,8 +236,8 @@ cli_err(sock *s, int err)
     log(L_INFO "CLI connection dropped: %s", strerror(err));
   else
     log(L_INFO "CLI connection closed");
-  s->type = SK_DELETED;
   cli_free(s->data);
+  sk_close(s);
 }
 
 static int