From f4036e8352a466d7fc1b1e7c323bd671eadc1428 Mon Sep 17 00:00:00 2001
From: goldsimon <goldsimon>
Date: Wed, 25 Jul 2007 19:24:27 +0000
Subject: [PATCH] Another fix for bug #20021: by not returning an error if
 tcp_output fails in tcp_close, the code in do_close_internal gets simpler
 (tcp_output is called again later from tcp timers).

---
 CHANGELOG         |  5 ++++
 src/api/api_msg.c | 72 +++++++++++++++++------------------------------
 src/core/tcp.c    |  6 +++-
 3 files changed, 36 insertions(+), 47 deletions(-)

diff --git a/CHANGELOG b/CHANGELOG
index 26f7b303..5437417e 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -256,6 +256,11 @@ HISTORY
 
   ++ Bug fixes:
 
+  2007-07-25 Simon Goldschmidt
+  * api_msg.c, tcp.c: Another fix for bug #20021: by not returning an error if
+    tcp_output fails in tcp_close, the code in do_close_internal gets simpler
+    (tcp_output is called again later from tcp timers).
+
   2007-07-25 Simon Goldschmidt
   * ip_frag.c: Fixed bug #20429: use the new pbuf_copy_partial instead of the old
     copy_from_pbuf, which illegally modified the given pbuf.
diff --git a/src/api/api_msg.c b/src/api/api_msg.c
index 8fb56161..8a2107c7 100644
--- a/src/api/api_msg.c
+++ b/src/api/api_msg.c
@@ -207,14 +207,17 @@ sent_tcp(void *arg, struct tcp_pcb *pcb, u16_t len)
   LWIP_ASSERT("conn != NULL", (conn != NULL));
 
   if (conn->state == NETCONN_WRITE) {
+    LWIP_ASSERT("conn->pcb.tcp != NULL", conn->pcb.tcp != NULL);
     do_writemore(conn);
   } else if (conn->state == NETCONN_CLOSE) {
     do_close_internal(conn);
   }
 
-  if (conn && conn->callback)
-    if (tcp_sndbuf(conn->pcb.tcp) > TCP_SNDLOWAT)
+  if (conn && conn->callback) {
+    if ((conn->pcb.tcp != NULL) && (tcp_sndbuf(conn->pcb.tcp) > TCP_SNDLOWAT)) {
       (*conn->callback)(conn, NETCONN_EVT_SENDPLUS, len);
+    }
+  }
   
   return ERR_OK;
 }
@@ -421,62 +424,39 @@ do_newconn(struct api_msg_msg *msg)
 static void
 do_close_internal(struct netconn *conn)
 {
+  err_t err;
+
   LWIP_ASSERT("invalid conn", (conn != NULL));
   LWIP_ASSERT("this is for tcp netconns only", (conn->type == NETCONN_TCP));
   LWIP_ASSERT("conn must be in state NETCONN_CLOSE", (conn->state == NETCONN_CLOSE));
   LWIP_ASSERT("pcb already closed", (conn->pcb.tcp != NULL));
 
+  /* Set back some callback pointers */
   if (conn->pcb.tcp->state == LISTEN) {
     tcp_arg(conn->pcb.tcp, NULL);
     tcp_accept(conn->pcb.tcp, NULL);
-    /* for state==LISTEN, tcp_close can't fail */
-    tcp_close(conn->pcb.tcp);
+  } else {
+    tcp_recv(conn->pcb.tcp, NULL);
+  }
+  /* Try to close the connection */
+  err = tcp_close(conn->pcb.tcp);
+  if (err == ERR_OK) {
+    /* Closing succeeded */
     conn->state = NETCONN_NONE;
     conn->pcb.tcp = NULL;
-    conn->err = ERR_OK;
-    sys_mbox_post(conn->mbox, NULL);
-  } else {
-    err_t err;
-    enum tcp_state old_state = conn->pcb.tcp->state;
-    if ((old_state == SYN_RCVD) || (old_state == ESTABLISHED) ||
-      (old_state == CLOSE_WAIT)) {
-      tcp_arg(conn->pcb.tcp, NULL);
-      tcp_sent(conn->pcb.tcp, NULL);
-      tcp_recv(conn->pcb.tcp, NULL);
-      tcp_poll(conn->pcb.tcp, NULL, 0);
-      tcp_err(conn->pcb.tcp, NULL);
-      err = tcp_close(conn->pcb.tcp);
-      switch (err) {
-        default:
-          /* any other error: abort! */
-          tcp_abort(conn->pcb.tcp);
-          /* fall through */
-        case (ERR_OK):
-          /* ERR_OK: fall through */
-        case (ERR_BUF):
-          /* ERR_BUF: tcp_output failed,
-             will be called again by internal tcp timers */
-          conn->state = NETCONN_NONE;
-          conn->pcb.tcp = NULL;
-          conn->err = err;
-          /* Trigger select() in socket layer */
-          if (conn->callback) {
-              /* this should send something else so the errorfd is set,
-                 not the read and write fd! */
-              (*conn->callback)(conn, NETCONN_EVT_RCVPLUS, 0);
-              (*conn->callback)(conn, NETCONN_EVT_SENDPLUS, 0);
-          }
-          /* wake up the application task */
-          sys_mbox_post(conn->mbox, NULL);
-          break;
-        case (ERR_MEM):
-          /* ERR_MEM: error sending FIN?
-             try again in sent_tcp or poll_tcp */
-          /* stay in state NETCONN_CLOSE */
-          break;
-      }
+    conn->err = err;
+    /* Trigger select() in socket layer */
+    if (conn->callback) {
+        /* this should send something else so the errorfd is set,
+           not the read and write fd! */
+        (*conn->callback)(conn, NETCONN_EVT_RCVPLUS, 0);
+        (*conn->callback)(conn, NETCONN_EVT_SENDPLUS, 0);
     }
+    /* wake up the application task */
+    sys_mbox_post(conn->mbox, NULL);
   }
+  /* If closing didn't succeed, we get called again either
+     from poll_tcp or from sent_tcp */
 }
 
 /**
diff --git a/src/core/tcp.c b/src/core/tcp.c
index 15055a47..dd063b4c 100644
--- a/src/core/tcp.c
+++ b/src/core/tcp.c
@@ -190,7 +190,11 @@ tcp_close(struct tcp_pcb *pcb)
   }
 
   if (pcb != NULL && err == ERR_OK) {
-    err = tcp_output(pcb);
+    /* @todo: to ensure all has been sent when tcp_close returns, we have to
+       make sure tcp_output doesn't fail.
+       For now (and as long as we don't need this (no LINGER)), it's enough
+       to let the TCP timers deal with this. */
+    tcp_output(pcb);
   }
   return err;
 }