From cf814b2d341d138c3f4d03bccb34576b7fa41a32 Mon Sep 17 00:00:00 2001
From: Daniel Campora <daniel@wipy.io>
Date: Fri, 10 Jul 2015 11:32:53 +0200
Subject: [PATCH] cc3200: Refactor and clean-up socket closing code.

---
 cc3200/mods/modnetwork.h |  5 +++--
 cc3200/mods/modusocket.c |  2 --
 cc3200/mods/modwlan.c    | 44 ++++------------------------------------
 cc3200/serverstask.c     | 13 +++++++-----
 4 files changed, 15 insertions(+), 49 deletions(-)

diff --git a/cc3200/mods/modnetwork.h b/cc3200/mods/modnetwork.h
index 59034847b..7ecb8becb 100644
--- a/cc3200/mods/modnetwork.h
+++ b/cc3200/mods/modnetwork.h
@@ -43,16 +43,17 @@ typedef struct _mod_network_nic_type_t {
 typedef struct _mod_network_socket_base_t {
     union {
         struct {
+            // this order is important so that fileno gets > 0 once
+            // the socket descriptor is assigned after being created.
             uint8_t domain;
+            int8_t fileno;
             uint8_t type;
             uint8_t proto;
-            int8_t fileno;
         } u_param;
         int16_t sd;
     };
     bool has_timeout;
     bool cert_req;
-    bool closed;
 } mod_network_socket_base_t;
 
 typedef struct _mod_network_socket_obj_t {
diff --git a/cc3200/mods/modusocket.c b/cc3200/mods/modusocket.c
index c7501be43..7dc4a7190 100644
--- a/cc3200/mods/modusocket.c
+++ b/cc3200/mods/modusocket.c
@@ -138,7 +138,6 @@ STATIC mp_obj_t socket_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_
     s->sock_base.u_param.fileno = -1;
     s->sock_base.has_timeout = false;
     s->sock_base.cert_req = false;
-    s->sock_base.closed = false;
 
     if (n_args > 0) {
         s->sock_base.u_param.domain = mp_obj_get_int(args[0]);
@@ -158,7 +157,6 @@ STATIC mp_obj_t socket_make_new(mp_obj_t type_in, mp_uint_t n_args, mp_uint_t n_
     if (wlan_socket_socket(s, &_errno) != 0) {
         nlr_raise(mp_obj_new_exception_arg1(&mp_type_OSError, MP_OBJ_NEW_SMALL_INT(-_errno)));
     }
-
     return s;
 }
 
diff --git a/cc3200/mods/modwlan.c b/cc3200/mods/modwlan.c
index a402cf640..b25bd9968 100644
--- a/cc3200/mods/modwlan.c
+++ b/cc3200/mods/modwlan.c
@@ -1151,25 +1151,20 @@ STATIC const mp_cb_methods_t wlan_cb_methods = {
 int wlan_gethostbyname(const char *name, mp_uint_t len, uint8_t *out_ip, uint8_t family) {
     uint32_t ip;
     int result = sl_NetAppDnsGetHostByName((_i8 *)name, (_u16)len, (_u32*)&ip, (_u8)family);
-
     out_ip[0] = ip;
     out_ip[1] = ip >> 8;
     out_ip[2] = ip >> 16;
     out_ip[3] = ip >> 24;
-
     return result;
 }
 
 int wlan_socket_socket(mod_network_socket_obj_t *s, int *_errno) {
-    // open the socket
     int16_t sd = sl_Socket(s->sock_base.u_param.domain, s->sock_base.u_param.type, s->sock_base.u_param.proto);
-    // save the socket descriptor
-    s->sock_base.sd = sd;
     if (sd < 0) {
         *_errno = sd;
         return -1;
     }
-
+    s->sock_base.sd = sd;
     return 0;
 }
 
@@ -1177,10 +1172,9 @@ void wlan_socket_close(mod_network_socket_obj_t *s) {
     // this is to prevent the finalizer to close a socket that failed when being created
     if (s->sock_base.sd >= 0) {
         modusocket_socket_delete(s->sock_base.sd);
-        // TODO check return value and raise an exception if applicable
         sl_Close(s->sock_base.sd);
+        s->sock_base.sd = -1;
     }
-    s->sock_base.closed = true;
 }
 
 int wlan_socket_bind(mod_network_socket_obj_t *s, byte *ip, mp_uint_t port, int *_errno) {
@@ -1218,7 +1212,6 @@ int wlan_socket_accept(mod_network_socket_obj_t *s, mod_network_socket_obj_t *s2
 
     // return ip and port
     UNPACK_SOCKADDR(addr, ip, *port);
-
     return 0;
 }
 
@@ -1241,38 +1234,15 @@ int wlan_socket_send(mod_network_socket_obj_t *s, const byte *buf, mp_uint_t len
         *_errno = bytes;
         return -1;
     }
-
     return bytes;
 }
 
 int wlan_socket_recv(mod_network_socket_obj_t *s, byte *buf, mp_uint_t len, int *_errno) {
-    // check if the socket is open
-    if (s->sock_base.closed) {
-        // socket is closed, but the there might be data remaining in the buffer, so check
-        fd_set rfds;
-        FD_ZERO(&rfds);
-        FD_SET(s->sock_base.sd, &rfds);
-        timeval tv;
-        tv.tv_sec = 0;
-        tv.tv_usec = 2;
-        int nfds = sl_Select(s->sock_base.sd + 1, &rfds, NULL, NULL, &tv);
-        if (nfds == -1 || !FD_ISSET(s->sock_base.sd, &rfds)) {
-            // no data waiting, so close the socket and return 0 data
-            wlan_socket_close(s);
-            return 0;
-        }
-    }
-
-    // cap length at WLAN_MAX_RX_SIZE
-    len = MIN(len, WLAN_MAX_RX_SIZE);
-
-    // do the recv
-    int ret = sl_Recv(s->sock_base.sd, buf, len, 0);
+    int ret = sl_Recv(s->sock_base.sd, buf, MIN(len, WLAN_MAX_RX_SIZE), 0);
     if (ret < 0) {
         *_errno = ret;
         return -1;
     }
-
     return ret;
 }
 
@@ -1289,7 +1259,7 @@ int wlan_socket_sendto( mod_network_socket_obj_t *s, const byte *buf, mp_uint_t
 int wlan_socket_recvfrom(mod_network_socket_obj_t *s, byte *buf, mp_uint_t len, byte *ip, mp_uint_t *port, int *_errno) {
     sockaddr addr;
     socklen_t addr_len = sizeof(addr);
-    mp_int_t ret = sl_RecvFrom(s->sock_base.sd, buf, len, 0, &addr, &addr_len);
+    mp_int_t ret = sl_RecvFrom(s->sock_base.sd, buf, MIN(len, WLAN_MAX_RX_SIZE), 0, &addr, &addr_len);
     if (ret < 0) {
         *_errno = ret;
         return -1;
@@ -1355,12 +1325,6 @@ int wlan_socket_ioctl (mod_network_socket_obj_t *s, mp_uint_t request, mp_uint_t
         // set fds if needed
         if (flags & MP_IOCTL_POLL_RD) {
             FD_SET(sd, &rfds);
-
-            // A socked that just closed is available for reading.  A call to
-            // recv() returns 0 which is consistent with BSD.
-            if (s->sock_base.closed) {
-                ret |= MP_IOCTL_POLL_RD;
-            }
         }
         if (flags & MP_IOCTL_POLL_WR) {
             FD_SET(sd, &wfds);
diff --git a/cc3200/serverstask.c b/cc3200/serverstask.c
index d20f9bcfa..0b6ad18b4 100644
--- a/cc3200/serverstask.c
+++ b/cc3200/serverstask.c
@@ -107,12 +107,15 @@ void TASK_Servers (void *pvParameters) {
             servers_data.do_disable = false;
             servers_data.enabled = false;
         }
-        else if (servers_data.do_reset && servers_data.enabled) {
-            telnet_reset();
-            ftp_reset();
+        else if (servers_data.do_reset) {
+            // resetting the servers is needed to prevent half-open sockets
             servers_data.do_reset = false;
-            // resetting the servers is needed to preven half-open sockets
-            // and we should also close all user sockets
+            if (servers_data.enabled) {
+                telnet_reset();
+                ftp_reset();
+            }
+            // and we should also close all user sockets. We do it here
+            // for convinience and to save on code size.
             modusocket_close_all_user_sockets();
         }
 
-- 
GitLab