From 5ab58af1736d7aaf2d129200ace0bfc3f8fa1370 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Tue, 23 Apr 2019 15:20:04 +0200 Subject: [PATCH 01/10] fix udp value of remote addresses --- .../ESP8266WiFi/src/include/UdpContext.h | 76 ++++++++++++++----- 1 file changed, 57 insertions(+), 19 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index b08e410954..806d8fd9ab 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -231,11 +231,25 @@ class UdpContext return _pcb->local_port; } + void current_addresses_setup () + { + // _rx_buf is an address helper + current_addresses = *((addresshelper_s*)_rx_buf->payload); + + // swallow helper pbuf + auto head = _rx_buf; + _rx_buf = _rx_buf->next; + _rx_buf_offset = 0; + pbuf_free(head); + } + bool next() { if (!_rx_buf) return false; + current_addresses_setup(); + if (!_first_buf_taken) { _first_buf_taken = true; @@ -425,37 +439,57 @@ class UdpContext (void) upcb; (void) addr; (void) port; + + // --> Arduino's UDP is a stream but UDP is not <-- + // When IPv6 is enabled, we store addresses from here + // because lwIP's macro are valid only in this callback + // (there's no easy way to safely guess whether packet + // is from v4 or v6 when we have only access to payload) + // Because of this stream-ed way this is inacurate when + // user does not swallow data quickly enough + // + // fixing this by an intermediate chained pbuf containing + // addrhelper_s + + // allocate new pbuf to store addresses/ports + pbuf* helper = pbuf_alloc(PBUF_RAW, sizeof(addrhelper_s), PBUF_RAM); + if (!helper) + { + // memory issue - discard received data + pbuf_free(pb); + return; + } + // construct in place + IPAddress* srcaddr = &((addrhelper_s*)(pbuf->payload()))->srcaddr; + IPAddress* dstaddr = &((addrhelper_s*)(pbuf->payload()))->dstaddr; +#if LWIP_VERSION_MAJOR == 1 + new(srcaddr) IPAddress(current_iphdr_src); + new(dstaddr) IPAddress(current_iphdr_dest); +#else + new(srcaddr) IPAddress(ip_data.current_iphdr_src); + new(dstaddr) IPAddress(ip_data.current_iphdr_dest); +#endif + ((addrhelper_s*)(pbuf->payload()))->srcport = upcb->remote_port; + ((addrhelper_s*)(pbuf->payload()))->dstport = upcb->local_port; + // chain this helper pbuf first if (_rx_buf) { // there is some unread data // chain the new pbuf to the existing one DEBUGV(":urch %d, %d\r\n", _rx_buf->tot_len, pb->tot_len); - pbuf_cat(_rx_buf, pb); + pbuf_cat(_rx_buf, helper); } else { DEBUGV(":urn %d\r\n", pb->tot_len); _first_buf_taken = false; - _rx_buf = pb; + _rx_buf = helper; _rx_buf_offset = 0; } - // --> Arduino's UDP is a stream but UDP is not <-- - // When IPv6 is enabled, we store addresses from here - // because lwIP's macro are valid only in this callback - // (there's no easy way to safely guess whether packet - // is from v4 or v6 when we have only access to payload) - // Because of this stream-ed way this is inacurate when - // user does not swallow data quickly enough (the former - // IPv4-only way suffers from the exact same issue. - -#if LWIP_VERSION_MAJOR == 1 - _src_addr = current_iphdr_src; - _dst_addr = current_iphdr_dest; -#else - _src_addr = ip_data.current_iphdr_src; - _dst_addr = ip_data.current_iphdr_dest; -#endif + // now chain data + // chain the new pbuf to the existing one + pbuf_cat(_rx_buf, pb); if (_on_rx) { _on_rx(); @@ -483,7 +517,11 @@ class UdpContext #ifdef LWIP_MAYBE_XCC uint16_t _mcast_ttl; #endif - IPAddress _src_addr, _dst_addr; + struct addrhelper_s + { + IPAddress srcaddr, dstaddr; + int16_t srcport, dstport; + } current_addr; }; From 79ceb03d8594cfb9cb66c27bb29abe3bd97551bf Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 00:03:34 +0200 Subject: [PATCH 02/10] wip --- .../ESP8266WiFi/src/include/UdpContext.h | 99 ++++++++++--------- 1 file changed, 50 insertions(+), 49 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index 806d8fd9ab..2faadb79ea 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -32,7 +32,7 @@ void esp_schedule(); #include -#define GET_UDP_HDR(pb) (reinterpret_cast(((uint8_t*)((pb)->payload)) - UDP_HLEN)) +#define ALIGNER(x) ((void*)((((intptr_t)(x))+3)&~3)) class UdpContext { @@ -207,21 +207,17 @@ class UdpContext CONST IPAddress& getRemoteAddress() CONST { - return _src_addr; + return current_addr.srcaddr; } uint16_t getRemotePort() const { - if (!_rx_buf) - return 0; - - udp_hdr* udphdr = GET_UDP_HDR(_rx_buf); - return lwip_ntohs(udphdr->src); + return current_addr.srcport; } const IPAddress& getDestAddress() const { - return _dst_addr; + return current_addr.dstaddr; } uint16_t getLocalPort() const @@ -234,12 +230,11 @@ class UdpContext void current_addresses_setup () { // _rx_buf is an address helper - current_addresses = *((addresshelper_s*)_rx_buf->payload); + current_addr = *((addrhelper_s*)ALIGNER(_rx_buf->payload)); // swallow helper pbuf auto head = _rx_buf; _rx_buf = _rx_buf->next; - _rx_buf_offset = 0; pbuf_free(head); } @@ -248,8 +243,6 @@ class UdpContext if (!_rx_buf) return false; - current_addresses_setup(); - if (!_first_buf_taken) { _first_buf_taken = true; @@ -262,6 +255,7 @@ class UdpContext if (_rx_buf) { + current_addresses_setup(); pbuf_ref(_rx_buf); } pbuf_free(head); @@ -440,56 +434,63 @@ class UdpContext (void) addr; (void) port; - // --> Arduino's UDP is a stream but UDP is not <-- - // When IPv6 is enabled, we store addresses from here - // because lwIP's macro are valid only in this callback - // (there's no easy way to safely guess whether packet - // is from v4 or v6 when we have only access to payload) - // Because of this stream-ed way this is inacurate when - // user does not swallow data quickly enough - // - // fixing this by an intermediate chained pbuf containing - // addrhelper_s - - // allocate new pbuf to store addresses/ports - pbuf* helper = pbuf_alloc(PBUF_RAW, sizeof(addrhelper_s), PBUF_RAM); - if (!helper) - { - // memory issue - discard received data - pbuf_free(pb); - return; - } - // construct in place - IPAddress* srcaddr = &((addrhelper_s*)(pbuf->payload()))->srcaddr; - IPAddress* dstaddr = &((addrhelper_s*)(pbuf->payload()))->dstaddr; -#if LWIP_VERSION_MAJOR == 1 - new(srcaddr) IPAddress(current_iphdr_src); - new(dstaddr) IPAddress(current_iphdr_dest); -#else - new(srcaddr) IPAddress(ip_data.current_iphdr_src); - new(dstaddr) IPAddress(ip_data.current_iphdr_dest); -#endif - ((addrhelper_s*)(pbuf->payload()))->srcport = upcb->remote_port; - ((addrhelper_s*)(pbuf->payload()))->dstport = upcb->local_port; + addrhelper_s* helper; + // chain this helper pbuf first if (_rx_buf) { // there is some unread data + // chain pbuf + + // Addresses/ports are stored from this callback because lwIP's + // macro are valid only now. + // + // When peeking data from before payload start (like it was done + // before IPv6), there's no easy way to safely guess whether + // packet is from v4 or v6. + // + // Now storing data in an intermediate chained pbuf containing + // addrhelper_s + + // allocate new pbuf to store addresses/ports + pbuf* pb_helper = pbuf_alloc(PBUF_RAW, sizeof(addrhelper_s) + /*alignment shift*/4, PBUF_RAM); + if (!pb_helper) + { + // memory issue - discard received data + pbuf_free(pb); + return; + } + + helper = (addrhelper_s*)ALIGNER(pb_helper->payload); + // construct in place + new(&helper->srcaddr) IPAddress(); + new(&helper->dstaddr) IPAddress(); + pbuf_cat(_rx_buf, pb_helper); + + // now chain effective data // chain the new pbuf to the existing one DEBUGV(":urch %d, %d\r\n", _rx_buf->tot_len, pb->tot_len); - pbuf_cat(_rx_buf, helper); + pbuf_cat(_rx_buf, pb); } else { + helper = ¤t_addr; + DEBUGV(":urn %d\r\n", pb->tot_len); _first_buf_taken = false; - _rx_buf = helper; + _rx_buf = pb; _rx_buf_offset = 0; } - // now chain data - // chain the new pbuf to the existing one - pbuf_cat(_rx_buf, pb); + // fill addresses and port +#if LWIP_VERSION_MAJOR == 1 + *helper->srcaddr = current_iphdr_src; + *helper->dstaddr = current_iphdr_dest; +#else + *helper->srcaddr = *ip_current_src_addr(); + *helper->dstaddr = *ip_current_dest_addr(); +#endif + helper->srcport = port; if (_on_rx) { _on_rx(); @@ -520,7 +521,7 @@ class UdpContext struct addrhelper_s { IPAddress srcaddr, dstaddr; - int16_t srcport, dstport; + int16_t srcport; } current_addr; }; From f181d4c751435d0ee60cb8d27a1219d89cfbb0fe Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 02:12:43 +0200 Subject: [PATCH 03/10] test OK with multiple senders and bufferization --- .../ESP8266WiFi/src/include/UdpContext.h | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index 2faadb79ea..31577f5ce3 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -227,17 +227,6 @@ class UdpContext return _pcb->local_port; } - void current_addresses_setup () - { - // _rx_buf is an address helper - current_addr = *((addrhelper_s*)ALIGNER(_rx_buf->payload)); - - // swallow helper pbuf - auto head = _rx_buf; - _rx_buf = _rx_buf->next; - pbuf_free(head); - } - bool next() { if (!_rx_buf) @@ -255,7 +244,16 @@ class UdpContext if (_rx_buf) { - current_addresses_setup(); + pbuf_ref(_rx_buf); + + // _rx_buf is an address helper + current_addr = *((addrhelper_s*)ALIGNER(_rx_buf->payload)); + + // swallow helper pbuf + auto head = _rx_buf; + _rx_buf = _rx_buf->next; + pbuf_free(head); + pbuf_ref(_rx_buf); } pbuf_free(head); From 725caa45180368efe060433cc8d4947b2e2d5982 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 02:13:24 +0200 Subject: [PATCH 04/10] end of buffer fix in udp example --- libraries/ESP8266WiFi/examples/Udp/Udp.ino | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/libraries/ESP8266WiFi/examples/Udp/Udp.ino b/libraries/ESP8266WiFi/examples/Udp/Udp.ino index c413d3cea2..8668d264b2 100644 --- a/libraries/ESP8266WiFi/examples/Udp/Udp.ino +++ b/libraries/ESP8266WiFi/examples/Udp/Udp.ino @@ -49,21 +49,15 @@ void loop() { // if there's data available, read a packet int packetSize = Udp.parsePacket(); if (packetSize) { - Serial.print("Received packet of size "); - Serial.println(packetSize); - Serial.print("From "); - IPAddress remote = Udp.remoteIP(); - for (int i = 0; i < 4; i++) { - Serial.print(remote[i], DEC); - if (i < 3) { - Serial.print("."); - } - } - Serial.print(", port "); - Serial.println(Udp.remotePort()); + Serial.printf("Received packet of size %d from %s:%d\n (to %s:%d, free heap = %d B)\n", + packetSize, + Udp.remoteIP().toString().c_str(), Udp.remotePort(), + Udp.destinationIP().toString().c_str(), Udp.localPort(), + ESP.getFreeHeap()); // read the packet into packetBufffer Udp.read(packetBuffer, UDP_TX_PACKET_MAX_SIZE); + packetBuffer[packetSize] = 0; Serial.println("Contents:"); Serial.println(packetBuffer); @@ -72,7 +66,7 @@ void loop() { Udp.write(ReplyBuffer); Udp.endPacket(); } - delay(10); + } /* From 34911aee5ec2a9c0acc780927f7170c05365345e Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 02:32:00 +0200 Subject: [PATCH 05/10] udp example: fix buffer management --- libraries/ESP8266WiFi/examples/Udp/Udp.ino | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/ESP8266WiFi/examples/Udp/Udp.ino b/libraries/ESP8266WiFi/examples/Udp/Udp.ino index 8668d264b2..f1000477d7 100644 --- a/libraries/ESP8266WiFi/examples/Udp/Udp.ino +++ b/libraries/ESP8266WiFi/examples/Udp/Udp.ino @@ -26,7 +26,7 @@ unsigned int localPort = 8888; // local port to listen on // buffers for receiving and sending data -char packetBuffer[UDP_TX_PACKET_MAX_SIZE]; //buffer to hold incoming packet, +char packetBuffer[UDP_TX_PACKET_MAX_SIZE+1]; //buffer to hold incoming packet, char ReplyBuffer[] = "acknowledged\r\n"; // a string to send back WiFiUDP Udp; @@ -56,8 +56,8 @@ void loop() { ESP.getFreeHeap()); // read the packet into packetBufffer - Udp.read(packetBuffer, UDP_TX_PACKET_MAX_SIZE); - packetBuffer[packetSize] = 0; + int n = Udp.read(packetBuffer, UDP_TX_PACKET_MAX_SIZE); + packetBuffer[n] = 0; Serial.println("Contents:"); Serial.println(packetBuffer); From bc5158c6682348756cd652449c59e5633132429d Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 23:06:32 +0200 Subject: [PATCH 06/10] fix IPAddress handling --- libraries/ESP8266WiFi/src/include/UdpContext.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index 31577f5ce3..f1e0132fab 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -482,11 +482,11 @@ class UdpContext // fill addresses and port #if LWIP_VERSION_MAJOR == 1 - *helper->srcaddr = current_iphdr_src; - *helper->dstaddr = current_iphdr_dest; + helper->srcaddr = current_iphdr_src; + helper->dstaddr = current_iphdr_dest; #else - *helper->srcaddr = *ip_current_src_addr(); - *helper->dstaddr = *ip_current_dest_addr(); + helper->srcaddr = *ip_current_src_addr(); + helper->dstaddr = *ip_current_dest_addr(); #endif helper->srcport = port; From f62c0bcbe088d25b94d25b9530700349638e0a3d Mon Sep 17 00:00:00 2001 From: david gauchard Date: Wed, 24 Apr 2019 23:08:22 +0200 Subject: [PATCH 07/10] example style --- libraries/ESP8266WiFi/examples/Udp/Udp.ino | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/ESP8266WiFi/examples/Udp/Udp.ino b/libraries/ESP8266WiFi/examples/Udp/Udp.ino index f1000477d7..7ec287b392 100644 --- a/libraries/ESP8266WiFi/examples/Udp/Udp.ino +++ b/libraries/ESP8266WiFi/examples/Udp/Udp.ino @@ -26,7 +26,7 @@ unsigned int localPort = 8888; // local port to listen on // buffers for receiving and sending data -char packetBuffer[UDP_TX_PACKET_MAX_SIZE+1]; //buffer to hold incoming packet, +char packetBuffer[UDP_TX_PACKET_MAX_SIZE + 1]; //buffer to hold incoming packet, char ReplyBuffer[] = "acknowledged\r\n"; // a string to send back WiFiUDP Udp; From 2a484867168bb6cf88a51baa363b445391c7ad9f Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 25 Apr 2019 02:45:23 +0200 Subject: [PATCH 08/10] fix per review: - separate variable declaration from struct declaration - cammelCase - in-place constructor: avoid empty constructor then initialisation - add in-place call to destructor also improved pbuf usage --- .../ESP8266WiFi/src/include/UdpContext.h | 101 ++++++++++-------- 1 file changed, 57 insertions(+), 44 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index f1e0132fab..12b25faff2 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -207,17 +207,17 @@ class UdpContext CONST IPAddress& getRemoteAddress() CONST { - return current_addr.srcaddr; + return _currentAddr.srcaddr; } uint16_t getRemotePort() const { - return current_addr.srcport; + return _currentAddr.srcport; } const IPAddress& getDestAddress() const { - return current_addr.dstaddr; + return _currentAddr.dstaddr; } uint16_t getLocalPort() const @@ -231,33 +231,41 @@ class UdpContext { if (!_rx_buf) return false; - if (!_first_buf_taken) { _first_buf_taken = true; return true; } - auto head = _rx_buf; + auto deleteme = _rx_buf; _rx_buf = _rx_buf->next; - _rx_buf_offset = 0; if (_rx_buf) { - pbuf_ref(_rx_buf); - - // _rx_buf is an address helper - current_addr = *((addrhelper_s*)ALIGNER(_rx_buf->payload)); - - // swallow helper pbuf - auto head = _rx_buf; + // first rx_buf contains an address helper, + // copy it to "current address" + auto helper = (AddrHelper*)ALIGNER(_rx_buf->payload); + _currentAddr = *helper; + + // destroy helper +#if 0 // constructor not called in _recv, see #if + helper->~AddrHelper(); +#else + helper->srcaddr.~IPAddress(); + helper->dstaddr.~IPAddress(); +#endif + // forwarding in rx_buf list, next one is effective data + // current (not ref'ed) one will be pbuf_free'd with deleteme _rx_buf = _rx_buf->next; - pbuf_free(head); + // this rx_buf is not nullptr by construction + // ref'ing it to prevent release from the below pbuf_free(deleteme) pbuf_ref(_rx_buf); } - pbuf_free(head); - return _rx_buf != 0; + pbuf_free(deleteme); + + _rx_buf_offset = 0; + return _rx_buf != nullptr; } int read() @@ -426,13 +434,15 @@ class UdpContext } void _recv(udp_pcb *upcb, pbuf *pb, - const ip_addr_t *addr, u16_t port) + const ip_addr_t *srcaddr, u16_t srcport) { (void) upcb; - (void) addr; - (void) port; - addrhelper_s* helper; +#if LWIP_VERSION_MAJOR == 1 + #define TEMPDSTADDR (¤t_iphdr_dest) +#else + #define TEMPDSTADDR (ip_current_dest_addr()) +#endif // chain this helper pbuf first if (_rx_buf) @@ -448,31 +458,37 @@ class UdpContext // packet is from v4 or v6. // // Now storing data in an intermediate chained pbuf containing - // addrhelper_s + // AddrHelper // allocate new pbuf to store addresses/ports - pbuf* pb_helper = pbuf_alloc(PBUF_RAW, sizeof(addrhelper_s) + /*alignment shift*/4, PBUF_RAM); + pbuf* pb_helper = pbuf_alloc(PBUF_RAW, sizeof(AddrHelper) + /*alignment shift*/4, PBUF_RAM); if (!pb_helper) { // memory issue - discard received data pbuf_free(pb); return; } - - helper = (addrhelper_s*)ALIGNER(pb_helper->payload); // construct in place - new(&helper->srcaddr) IPAddress(); - new(&helper->dstaddr) IPAddress(); + AddrHelper* helper = (AddrHelper*)ALIGNER(pb_helper->payload); +#if 0 // should work but does not + new(&helper) AddrHelper(srcaddr, TEMPDSTADDR, srcport); +#else + new(&helper->srcaddr) IPAddress(srcaddr); + new(&helper->dstaddr) IPAddress(TEMPDSTADDR); + helper->srcport = srcport; +#endif + // chain it pbuf_cat(_rx_buf, pb_helper); - // now chain effective data - // chain the new pbuf to the existing one + // now chain the new data pbuf DEBUGV(":urch %d, %d\r\n", _rx_buf->tot_len, pb->tot_len); pbuf_cat(_rx_buf, pb); } else { - helper = ¤t_addr; + _currentAddr.srcaddr = srcaddr; + _currentAddr.dstaddr = TEMPDSTADDR; + _currentAddr.srcport = srcport; DEBUGV(":urn %d\r\n", pb->tot_len); _first_buf_taken = false; @@ -480,27 +496,19 @@ class UdpContext _rx_buf_offset = 0; } - // fill addresses and port -#if LWIP_VERSION_MAJOR == 1 - helper->srcaddr = current_iphdr_src; - helper->dstaddr = current_iphdr_dest; -#else - helper->srcaddr = *ip_current_src_addr(); - helper->dstaddr = *ip_current_dest_addr(); -#endif - helper->srcport = port; - if (_on_rx) { _on_rx(); } - } + #undef TEMPDSTADDR + + } static void _s_recv(void *arg, udp_pcb *upcb, pbuf *p, - CONST ip_addr_t *addr, u16_t port) + CONST ip_addr_t *srcaddr, u16_t srcport) { - reinterpret_cast(arg)->_recv(upcb, p, addr, port); + reinterpret_cast(arg)->_recv(upcb, p, srcaddr, srcport); } private: @@ -516,11 +524,16 @@ class UdpContext #ifdef LWIP_MAYBE_XCC uint16_t _mcast_ttl; #endif - struct addrhelper_s + struct AddrHelper { IPAddress srcaddr, dstaddr; int16_t srcport; - } current_addr; + + AddrHelper() { } + AddrHelper(const ip_addr_t* src, const ip_addr_t* dst, uint16_t srcport): + srcaddr(src), dstaddr(dst), srcport(srcport) { } + }; + AddrHelper _currentAddr; }; From 061669082b6c21c170f0a3a4e81b94ef7c83d3bc Mon Sep 17 00:00:00 2001 From: david gauchard Date: Thu, 25 Apr 2019 10:52:46 +0200 Subject: [PATCH 09/10] fix use of pointer for in place constructor --- libraries/ESP8266WiFi/src/include/UdpContext.h | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index 12b25faff2..f5d83d236b 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -247,13 +247,9 @@ class UdpContext auto helper = (AddrHelper*)ALIGNER(_rx_buf->payload); _currentAddr = *helper; - // destroy helper -#if 0 // constructor not called in _recv, see #if + // destroy the helper helper->~AddrHelper(); -#else - helper->srcaddr.~IPAddress(); - helper->dstaddr.~IPAddress(); -#endif + // forwarding in rx_buf list, next one is effective data // current (not ref'ed) one will be pbuf_free'd with deleteme _rx_buf = _rx_buf->next; @@ -469,14 +465,7 @@ class UdpContext return; } // construct in place - AddrHelper* helper = (AddrHelper*)ALIGNER(pb_helper->payload); -#if 0 // should work but does not - new(&helper) AddrHelper(srcaddr, TEMPDSTADDR, srcport); -#else - new(&helper->srcaddr) IPAddress(srcaddr); - new(&helper->dstaddr) IPAddress(TEMPDSTADDR); - helper->srcport = srcport; -#endif + new(ALIGNER(pb_helper->payload)) AddrHelper(srcaddr, TEMPDSTADDR, srcport); // chain it pbuf_cat(_rx_buf, pb_helper); From 03e9298fb6e78cb717f2604acbf23546b95620c2 Mon Sep 17 00:00:00 2001 From: david gauchard Date: Fri, 26 Apr 2019 18:56:08 +0200 Subject: [PATCH 10/10] add comments --- .../ESP8266WiFi/src/include/UdpContext.h | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/libraries/ESP8266WiFi/src/include/UdpContext.h b/libraries/ESP8266WiFi/src/include/UdpContext.h index f5d83d236b..8198ccea61 100644 --- a/libraries/ESP8266WiFi/src/include/UdpContext.h +++ b/libraries/ESP8266WiFi/src/include/UdpContext.h @@ -32,7 +32,8 @@ void esp_schedule(); #include -#define ALIGNER(x) ((void*)((((intptr_t)(x))+3)&~3)) +#define PBUF_ALIGNER_ADJUST 4 +#define PBUF_ALIGNER(x) ((void*)((((intptr_t)(x))+3)&~3)) class UdpContext { @@ -242,19 +243,23 @@ class UdpContext if (_rx_buf) { - // first rx_buf contains an address helper, + // we have interleaved informations on addresses within reception pbuf chain: + // before: (data-pbuf) -> (data-pbuf) -> (data-pbuf) -> ... in the receiving order + // now: (address-info-pbuf -> data-pbuf) -> (address-info-pbuf -> data-pbuf) -> ... + + // so the first rx_buf contains an address helper, // copy it to "current address" - auto helper = (AddrHelper*)ALIGNER(_rx_buf->payload); + auto helper = (AddrHelper*)PBUF_ALIGNER(_rx_buf->payload); _currentAddr = *helper; - // destroy the helper + // destroy the helper in the about-to-be-released pbuf helper->~AddrHelper(); - // forwarding in rx_buf list, next one is effective data + // forward in rx_buf list, next one is effective data // current (not ref'ed) one will be pbuf_free'd with deleteme _rx_buf = _rx_buf->next; - // this rx_buf is not nullptr by construction + // this rx_buf is not nullptr by construction, // ref'ing it to prevent release from the below pbuf_free(deleteme) pbuf_ref(_rx_buf); } @@ -457,7 +462,7 @@ class UdpContext // AddrHelper // allocate new pbuf to store addresses/ports - pbuf* pb_helper = pbuf_alloc(PBUF_RAW, sizeof(AddrHelper) + /*alignment shift*/4, PBUF_RAM); + pbuf* pb_helper = pbuf_alloc(PBUF_RAW, sizeof(AddrHelper) + PBUF_ALIGNER_ADJUST, PBUF_RAM); if (!pb_helper) { // memory issue - discard received data @@ -465,7 +470,7 @@ class UdpContext return; } // construct in place - new(ALIGNER(pb_helper->payload)) AddrHelper(srcaddr, TEMPDSTADDR, srcport); + new(PBUF_ALIGNER(pb_helper->payload)) AddrHelper(srcaddr, TEMPDSTADDR, srcport); // chain it pbuf_cat(_rx_buf, pb_helper);