fix: correct eight bugs found in STM32Ethernet sources

src/EthernetClient.cpp

@@ -57,6 +57,7 @@ int EthernetClient::connect(IPAddress ip, uint16_t port)
     if (_tcp_client == NULL) {
       return 0;
     }
+    _owns_tcp_client = true;
   }
 
   /* Creates a new TCP protocol control block */
@@ -200,7 +201,10 @@ void EthernetClient::stop()
         tcp_connection_close(_tcp_client->pcb, _tcp_client);
       }
     }
-    mem_free(_tcp_client);
+    if (_owns_tcp_client) {
+      mem_free(_tcp_client);
+      _owns_tcp_client = false;
+    }
     _tcp_client = NULL;
   }
 }
@@ -227,7 +231,11 @@ EthernetClient::operator bool()
 
 bool EthernetClient::operator==(const EthernetClient &rhs)
 {
-  return _tcp_client == rhs._tcp_client && _tcp_client->pcb == rhs._tcp_client->pcb;
+  bool equal = _tcp_client == rhs._tcp_client;
+  if (_tcp_client != NULL && rhs._tcp_client != NULL) {
+    equal = equal && (_tcp_client->pcb == rhs._tcp_client->pcb);
+  }
+  return equal;
 }
 
 uint8_t EthernetClient::getSocketNumber()

src/EthernetClient.h

@@ -131,7 +131,9 @@ class EthernetClient : public Client {
      * @brief Closes the connection and releases the client structure.
      *
      * The connection is closed unless it is already in TCP_CLOSING state. The
-     * structure is freed and the client is left with no connection.
+     * structure is freed only if this client allocated it in connect(). A
+     * structure shared with EthernetServer is left for the server to free, and
+     * the client is left with no connection.
      */
     virtual void stop();
     /**
@@ -176,8 +178,8 @@ class EthernetClient : public Client {
     /**
      * @brief Tests whether two clients refer to the same connection.
      *
-     * Two clients with no connection structure compare equal, and the function
-     * then reads a NULL pointer. Only compare clients that have a connection.
+     * Two clients with no connection structure compare equal. A client with no
+     * connection never equals a client that has one.
      *
      * @param rhs Client to compare with.
      * @return true if both clients use the same connection structure and the
@@ -253,6 +255,7 @@ class EthernetClient : public Client {
 
   private:
     struct tcp_struct *_tcp_client;
+    bool _owns_tcp_client = false; // true if connect() allocated _tcp_client
     uint16_t _connectionTimeout = 10000;
 };
 

src/EthernetUdp.cpp

@@ -40,7 +40,10 @@
 #include "lwip/igmp.h"
 #include "lwip/ip_addr.h"
 
-EthernetUDP::EthernetUDP() {}
+EthernetUDP::EthernetUDP()
+  : _port(0), _remotePort(0), _sendtoPort(0), _data(NULL), _udp(), _remaining(0)
+{
+}
 
 uint8_t EthernetUDP::begin(uint16_t port)
 {
@@ -76,6 +79,7 @@ uint8_t EthernetUDP::begin(IPAddress ip, uint16_t port, bool multicast)
 
 #if LWIP_IGMP
   if ((multicast) && (ERR_OK != igmp_joingroup(IP_ADDR_ANY, &ipaddr))) {
+    stop();
     return 0;
   }
 #endif
@@ -161,10 +165,11 @@ size_t EthernetUDP::write(uint8_t byte)
 
 size_t EthernetUDP::write(const uint8_t *buffer, size_t size)
 {
-  _data = stm32_new_data(_data, buffer, size);
-  if (_data == NULL) {
+  struct pbuf *data = stm32_new_data(_data, buffer, size);
+  if (data == NULL) {
     return 0;
   }
+  _data = data;
 
   return size;
 }

src/EthernetUdp.h

@@ -161,7 +161,7 @@ class EthernetUDP : public UDP {
      * @brief Appends a block of bytes to the outgoing packet.
      *
      * Data accumulates until endPacket() sends it. On failure, the data
-     * already buffered for the packet is discarded.
+     * already buffered for the packet is kept and the new bytes are not added.
      *
      * @param buffer Bytes to append.
      * @param size Number of bytes in buffer.

src/utility/stm32_eth.cpp

@@ -112,9 +112,7 @@ static err_t tcp_recv_callback(void *arg, struct tcp_pcb *tpcb, struct pbuf *p,
 static err_t tcp_sent_callback(void *arg, struct tcp_pcb *tpcb, u16_t len);
 static void tcp_err_callback(void *arg, err_t err);
 static void TIM_scheduler_Config(void);
-#if defined(STM32_CORE_VERSION) && (STM32_CORE_VERSION  > 0x01060100)
-  void _stm32_eth_scheduler(void);
-#endif
+void _stm32_eth_scheduler(void);
 
 /**
   * @brief  Configure the network interface.
@@ -286,27 +284,28 @@ uint8_t stm32_eth_link_up(void)
   return netif_is_link_up(&gnetif);
 }
 
-#if defined(STM32_CORE_VERSION) && (STM32_CORE_VERSION  > 0x01060100)
-/* refresh() generates a timer update event, which runs _stm32_eth_scheduler()
-   from the timer interrupt. Does nothing until the timer has been created. */
 void stm32_eth_scheduler(void)
 {
+#if defined(STM32_CORE_VERSION) && (STM32_CORE_VERSION  > 0x01060100)
+  /* refresh() generates a timer update event, which runs _stm32_eth_scheduler()
+   from the timer interrupt. Does nothing until the timer has been created. */
   if (EthTim != NULL) {
     EthTim->refresh();
   }
+#else
+  _stm32_eth_scheduler();
+#endif
 }
 
 /**
-  * @brief  Run one LwIP scheduler pass. Called only from the timer callback.
+  * @brief  Run one LwIP scheduler pass.
   *
   * Reads received packets (unless ETH_INPUT_USE_IT is defined), checks the
   * link status every TIME_CHECK_ETH_LINK_STATE ms, handles LwIP timeouts and
-  * runs the periodic DHCP processing.
+  * runs the periodic DHCP processing. Called by the timer callback and, on
+  * older cores, by stm32_eth_scheduler().
   */
 void _stm32_eth_scheduler(void)
-#else
-void stm32_eth_scheduler(void)
-#endif
 {
   /* Read a received packet from the Ethernet buffers and send it
   to the lwIP for handling */
@@ -770,7 +769,7 @@ err_t tcp_connected_callback(void *arg, struct tcp_pcb *tpcb, err_t err)
 err_t tcp_accept_callback(void *arg, struct tcp_pcb *newpcb, err_t err)
 {
   err_t ret_err;
-  uint8_t accepted;
+  bool accepted = false;
   struct tcp_struct **tcpClient = (struct tcp_struct **)arg;
 
   /* set priority for the newly accepted tcp connection newpcb */
@@ -789,7 +788,7 @@ err_t tcp_accept_callback(void *arg, struct tcp_pcb *newpcb, err_t err)
       for (uint16_t i = 0; i < MAX_CLIENT; i++) {
         if (tcpClient[i] == NULL) {
           tcpClient[i] = client;
-          accepted = 1;
+          accepted = true;
           break;
         }
       }
@@ -818,8 +817,7 @@ err_t tcp_accept_callback(void *arg, struct tcp_pcb *newpcb, err_t err)
       }
     } else {
       /*  close tcp connection */
-      tcp_connection_close(newpcb, client);
-      mem_free(client);
+      tcp_close(newpcb);
 
       /* return memory error */
       ret_err = ERR_MEM;