From 31e1d51dbd82a966e31d1f027fc24d3b08123c97 Mon Sep 17 00:00:00 2001 From: Piotr Szulc Date: Thu, 5 Sep 2024 15:54:04 +0200 Subject: [PATCH] [common] Cleanup mDNS code, cache service records (#263) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Handle possible failure of mdns_resp_add_service * One more place where mdns_resp_add_service may fail * addServiceImpl should always store services * Register in services when new netif is added * Refactored handling of cached services. --------- Co-authored-by: Kuba Szczodrzyński --- .../libraries/common/mDNS/LwIPmDNS.cpp | 129 +++++++++++------- 1 file changed, 78 insertions(+), 51 deletions(-) diff --git a/cores/common/arduino/libraries/common/mDNS/LwIPmDNS.cpp b/cores/common/arduino/libraries/common/mDNS/LwIPmDNS.cpp index 6b0a47616..21e25e8a5 100644 --- a/cores/common/arduino/libraries/common/mDNS/LwIPmDNS.cpp +++ b/cores/common/arduino/libraries/common/mDNS/LwIPmDNS.cpp @@ -20,23 +20,51 @@ extern "C" { #if LWIP_MDNS_RESPONDER -static std::vector services_name; -static std::vector services; -static std::vector protos; -static std::vector ports; -static std::vector> records; +struct CachedService { + CachedService(const char *_name, const char *_service, mdns_sd_proto _proto, uint16_t _port) + : name(strdup(_name)), service(strdup(_service)), proto(_proto), port(_port) {} + + CachedService(const CachedService &) = delete; + CachedService &operator=(const CachedService &) = delete; + + CachedService(CachedService &&other) + : name(other.name), service(other.service), proto(other.proto), port(other.port), + records(std::move(other.records)) { + other.name = nullptr; + other.service = nullptr; + other.records.clear(); + } + + ~CachedService() { + if (name) { + free(name); + } + + if (service) { + free(service); + } + + for (auto &str : records) { + if (str) { + free(str); + } + } + } + + char *name; + char *service; + mdns_sd_proto proto; + uint16_t port; + std::vector records; +}; + +static std::vector sCachedServices; static const char *hostName; #ifdef LWIP_NETIF_EXT_STATUS_CALLBACK NETIF_DECLARE_EXT_CALLBACK(netif_callback) #endif -static inline void freeAllocatedStrings(const std::vector &strings) { - for (auto &str : strings) { - free(str); - } -} - mDNS::mDNS() {} mDNS::~mDNS() { @@ -44,14 +72,7 @@ mDNS::~mDNS() { } void mDNS::cleanup() { - freeAllocatedStrings(services_name); - services_name.clear(); - freeAllocatedStrings(services); - services.clear(); - for (auto &record : records) { - freeAllocatedStrings(record); - } - records.clear(); + sCachedServices.clear(); free((void *)hostName); hostName = NULL; @@ -62,10 +83,10 @@ void mDNS::cleanup() { static void mdnsTxtCallback(struct mdns_service *service, void *userdata) { size_t index = (size_t)userdata; - if (index >= records.size()) + if (index >= sCachedServices.size()) return; - for (const auto record : records[index]) { + for (const auto &record : sCachedServices[index].records) { err_t err = mdns_resp_add_service_txtitem(service, record, strlen(record)); if (err != ERR_OK) { LT_DM(MDNS, "Error %d while adding txt record: %s", err, record); @@ -85,28 +106,33 @@ static void mdnsStatusCallback(struct netif *netif, uint8_t result, int8_t slot) #ifdef LWIP_NETIF_EXT_STATUS_CALLBACK static void addServices(struct netif *netif) { - for (uint8_t i = 0; i < services.size(); i++) { + for (uint8_t i = 0; i < sCachedServices.size(); i++) { + const auto &cachedService = sCachedServices[i]; LT_DM( MDNS, "Add service: netif %u / %s / %s / %u / %u", netif->num, - services_name[i], - services[i], - protos[i], - ports[i] + cachedService.name, + cachedService.service, + cachedService.proto, + cachedService.port ); - mdns_resp_add_service( + s8_t slot = mdns_resp_add_service( netif, - services_name[i], - services[i], - (mdns_sd_proto)protos[i], - ports[i], + cachedService.name, + cachedService.service, + cachedService.proto, + cachedService.port, #if LWIP_VERSION_SIMPLE < 20200 // TTL removed in LwIP commit 62fb2fd749b (2.2.0 release) 255, #endif mdnsTxtCallback, reinterpret_cast(i) // index of newly added service ); + + if (slot < 0) { + LT_DM(MDNS, "mdns_resp_add_service returned error %d", slot); + } } } #endif @@ -145,9 +171,9 @@ mdns_netif_ext_status_callback(struct netif *netif, netif_nsc_reason_t reason, c if (reason & LWIP_NSC_NETIF_REMOVED) { LT_DM(MDNS, "Netif removed, stopping mDNS on netif %u", netif->num); mdns_resp_remove_netif(netif); - } else if (reason & LWIP_NSC_STATUS_CHANGED) { - LT_DM(MDNS, "Netif changed, starting mDNS on netif %u", netif->num); - if (enableMDNS(netif) && services.size() > 0) { + } else if ((reason & LWIP_NSC_STATUS_CHANGED) || (reason & LWIP_NSC_NETIF_ADDED)) { + LT_DM(MDNS, "Netif changed/added, starting mDNS on netif %u", netif->num); + if (enableMDNS(netif) && sCachedServices.size() > 0) { LT_DM(MDNS, "Adding services to netif %u", netif->num); addServices(netif); } @@ -191,12 +217,17 @@ void mDNS::end() { bool mDNS::addServiceImpl(const char *name, const char *service, uint8_t proto, uint16_t port) { bool added = false; struct netif *netif = netif_list; + + std::size_t serviceIndex = sCachedServices.size(); + // add the service to TXT record arrays + sCachedServices.emplace_back(name, service, (mdns_sd_proto)proto, port); + while (netif != NULL) { if (netif_is_up(netif)) { // register TXT callback; // pass service index as userdata parameter LT_DM(MDNS, "Add service: netif %u / %s / %s / %u / %u", netif->num, name, service, proto, port); - mdns_resp_add_service( + s8_t slot = mdns_resp_add_service( netif, name, service, @@ -206,38 +237,34 @@ bool mDNS::addServiceImpl(const char *name, const char *service, uint8_t proto, 255, #endif mdnsTxtCallback, - (void *)services.size() // index of newly added service + (void *)serviceIndex // index of newly added service ); + + if (slot < 0) { + LT_DM(MDNS, "mdns_resp_add_service returned error %d", slot); + } + added = true; } netif = netif->next; } - if (!added) - return false; - - // add the service to TXT record arrays - services_name.push_back(strdup(name)); - services.push_back(strdup(service)); - protos.push_back(proto); - ports.push_back(port); - records.emplace_back(); - - return true; + return added; } bool mDNS::addServiceTxtImpl(const char *service, uint8_t proto, const char *item) { uint8_t i; - for (i = 0; i < services.size(); i++) { + for (i = 0; i < sCachedServices.size(); i++) { + const auto &cachedService = sCachedServices[i]; // find a matching service - if (strcmp(services[i], service) == 0 && protos[i] == proto) { + if (strcmp(cachedService.service, service) == 0 && cachedService.proto == proto) { break; } } - if (i == services.size()) + if (i == sCachedServices.size()) return false; - records[i].push_back(strdup(item)); + sCachedServices[i].records.push_back(strdup(item)); return true; }