esp-protocols/components/mdns/refactoring_details.md

Original Function	Refactored Function	Concerns
mdns_if_from_preset_if	mdns_if_from_preset	The function name was changed from mdns_if_from_preset_if to mdns_if_from_preset, but the logic remains identical. While this appears to be a systematic naming convention change (supported by the evidence that all call sites in the refactored codebase now use the new name), there's a risk that some call sites in the original codebase might not have been updated, or that external code depending on the original function name could break. The functionality itself is preserved, but the interface change could cause linkage or compilation issues if not all dependencies are updated.
esp_netif_from_preset_if	netif_from_preset
_mdns_get_esp_netif	mdns_priv_get_esp_netif	The function esp_netif_from_preset_if() has been replaced with netif_from_preset() in the refactored code. While both functions appear to serve the same purpose of looking up predefined network interfaces, this change could introduce subtle bugs if: 1. The new function has different error handling behavior 2. The new function returns different values for edge cases (e.g., when interfaces are not available) 3. The new function has different performance characteristics that could affect timing-sensitive MDNS operations The original esp_netif_from_preset_if() was an inline function with a switch statement mapping predefined interface types to actual esp_netif instances. If netif_from_preset() has a different implementation or doesn't handle all the same mdns_predef_if_t enum values, this could break MDNS functionality for predefined interfaces like STA, AP, or ETH when they become available after initialization.
_mdns_clean_netif_ptr	mdns_priv_netif_disable
_mdns_get_if_from_esp_netif	get_if_from_netif	The function esp_netif_from_preset_if was renamed to netif_from_preset in the refactored code. While both functions appear to serve the same purpose (converting a predefined interface enum to an esp_netif pointer), this change could introduce subtle bugs if the implementation of netif_from_preset differs from the original esp_netif_from_preset_if function, particularly in error handling, return values for unsupported interface types, or behavioral differences in edge cases.
_str_null_or_empty	mdns_utils_str_null_or_empty	The refactoring introduced a potential compilation issue. The original function _str_null_or_empty was defined as static inline within mdns.c, making it only visible within that translation unit. The refactored version mdns_utils_str_null_or_empty is also static inline but in mdns_utils.h, which means it needs to be properly included in all files that use it. From the search results, I can see that mdns_browser.c, mdns_debug.c, and mdns_responder.c are using the new function, but I cannot verify if the original mdns.c file has been updated to include mdns_utils.h and replace all calls to _str_null_or_empty with mdns_utils_str_null_or_empty. If the original mdns.c still contains calls to the old function name without including the new header, this will cause compilation failures.
_mdns_mangle_name	mangle_name
_mdns_service_match	mdns_utils_service_match	The refactored function adds a null check for the 'srv' parameter (!srv), which changes the behavior from the original function. In the original implementation, callers could potentially pass NULL for 'srv', which would cause a segmentation fault when accessing srv->hostname. The refactored version now explicitly checks for NULL and returns false, which may mask underlying bugs in calling code that should be fixed rather than silently handled. Additionally, the function visibility changed from static to public, which could break encapsulation if this was intended to be an internal helper function.
_mdns_get_service_item	mdns_utils_get_service_item	The refactored function changed from static to non-static visibility and now uses mdns_priv_get_services() instead of directly accessing _mdns_server->services. This could introduce subtle bugs related to: 1. Thread safety - the original function was likely called within MDNS_SERVICE_LOCK/UNLOCK contexts, but the refactored version's access pattern through mdns_priv_get_services() may not provide the same guarantees 2. Encapsulation - making the function non-static exposes internal service lookup logic that was previously hidden 3. Dependency on mdns_utils_service_match having identical behavior to _mdns_service_match, which needs verification 4. Potential null pointer issues if mdns_priv_get_services() returns NULL when _mdns_server->services would not be NULL
_mdns_get_service_item_subtype	get_service_item_subtype
mdns_get_host_item	get_host_item	The refactored get_host_item function introduces a potential NULL return when the hostname matches the server's hostname or is NULL. In the original implementation, &_mdns_self_host was always returned (a guaranteed non-NULL address), but the refactored version calls mdns_priv_get_self_host() which can return NULL if s_server is NULL. This could break callers that assume non-NULL returns for self-host cases, potentially causing segmentation faults or incorrect behavior in service discovery and host resolution.
_mdns_can_add_more_services	can_add_more_services
_mdns_send_rx_action	send_rx_action	The refactored function 'send_rx_action' replaces the direct xQueueSend call with mdns_priv_queue_action, but the implementation of mdns_priv_queue_action is not visible in the provided context. This introduces a potential subtle bug if mdns_priv_queue_action does not properly handle the queue operation with the same blocking/non-blocking behavior (original used (TickType_t)0 timeout) or if it returns incorrect boolean values that don't match the expected ESP_OK/ESP_ERR_NO_MEM semantics. Additionally, the function was changed from global to static scope, which could break external callers if not properly addressed in the refactoring.
_mdns_get_default_instance_name	get_default_instance_name	The refactored function introduces potential subtle bugs due to the separation of concerns. Specifically: 1. The helper functions mdns_priv_get_instance() and mdns_priv_get_global_hostname() may have different NULL-checking behavior than the original direct _mdns_server access 2. There could be thread safety issues if s_server (the refactored equivalent of _mdns_server) is accessed without proper synchronization 3. The behavior when s_server is NULL may not exactly match the original behavior, particularly in edge cases 4. The replacement of _str_null_or_empty with mdns_utils_str_null_or_empty could have different implementations that handle edge cases differently
_mdns_get_service_instance_name	mdns_utils_get_service_instance_name	The refactored function get_default_instance_name() can return NULL when both mdns_priv_get_instance() and mdns_priv_get_global_hostname() return NULL or empty strings, whereas the original _mdns_get_default_instance_name() appears to always return a non-NULL string. This behavioral change could lead to null pointer dereferences in callers that expect a valid string, as evidenced by the original usage patterns where the return value is used directly in string operations without null checks.
_mdns_instance_name_match	mdns_utils_instance_name_match	The refactored function mdns_utils_instance_name_match calls get_default_instance_name() which uses mdns_priv_get_instance() internally, while the original function _mdns_instance_name_match used _mdns_get_default_instance_name() that directly accessed _mdns_server->instance. The behavioral equivalence depends on whether mdns_priv_get_instance() returns the same value as the original direct server access, particularly in edge cases where the server state might be inconsistent or during initialization/teardown phases.
_mdns_service_match_instance	mdns_utils_service_match_instance	The refactored function changed from static to public scope (removed static keyword), which could break encapsulation and allow unintended external access to internal service matching logic. Additionally, the helper function mdns_utils_str_null_or_empty might have different behavior than the original _str_null_or_empty if it was reimplemented during the utility extraction.
_mdns_get_service_item_instance	mdns_utils_get_service_item_instance
_mdns_read_fqdn	mdns_utils_read_fqdn	The refactored function mdns_utils_parse_fqdn uses a static buffer (static char buf[MDNS_NAME_BUF_LEN]) which could cause thread safety issues in a multi-threaded environment or reentrancy problems if the function is called recursively. Additionally, the memory movement operations using memmove in mdns_utils_parse_fqdn appear to be incorrectly calculating the offsets - the destination and source calculations seem to assume the mdns_name_t structure fields are exactly MDNS_NAME_BUF_LEN bytes apart, which may not be correct if the structure has padding or different field sizes.
_mdns_set_u16	???
_mdns_append_u8	mdns_utils_append_u8
_mdns_append_u16	mdns_utils_append_u16
_mdns_append_u32	append_u32	The bounds check in the refactored function uses (*index + sizeof(uint32_t)) >= MDNS_MAX_PACKET_SIZE while the original used (*index + 3) >= MDNS_MAX_PACKET_SIZE. Although sizeof(uint32_t) should typically be 4, this creates a potential portability issue if compiled on a system where uint32_t has a different size. Additionally, the refactored function now depends on mdns_utils_append_u8 which has different error handling - it returns 0 on bounds violation but doesn't prevent the index from being incremented in subsequent calls, potentially causing inconsistent state.
_mdns_append_type	append_type	The refactored function uses inconsistent function naming: mdns_utils_append_u16 and append_u32 (without the mdns_utils_ prefix). This suggests either a missing utility function implementation or inconsistent refactoring that could lead to compilation errors if append_u32 is not properly defined in the same context. Additionally, the buffer size calculation changed from hardcoded 10 to using sizeof operations, which while functionally equivalent, introduces potential maintenance complexity.
_mdns_append_string_with_len	append_string	The refactored function append_string introduces a subtle bug by replacing the explicit length parameter with strlen(string). This breaks compatibility with: 1. Non-null-terminated strings that were previously supported 2. Binary data that might contain embedded null bytes 3. Callers who want to control the exact length written (important for DNS label encoding) 4. Embedded systems use cases where string length might be known without scanning Additionally, the return value behavior has changed from returning the number of bytes written (len + 1) to potentially returning 0 on failure, which may break error handling logic in callers expecting the original return value pattern.
_mdns_append_string	append_string	The refactored function append_single_str has a critical semantic change in its return value. While the original _mdns_append_string returns the length of added data (string length + 1) on success, append_single_str returns the updated packet index. This could break calling code that relies on the original return value semantics for length calculations or error checking. Additionally, append_single_str introduces an extra error check for mdns_utils_append_u8 that wasn't present in the original, which changes the error handling behavior.
append_one_txt_record_entry	mdns_priv_append_one_txt_record_entry	The boundary check condition (*index + len + 1) >= MDNS_MAX_PACKET_SIZE in both original and refactored code may be incorrect. It should likely use > instead of >= to prevent buffer overflow when *index + len + 1 exactly equals MDNS_MAX_PACKET_SIZE. This would write one byte beyond the allocated buffer size. The refactoring preserved this potential off-by-one error rather than fixing it.
append_single_str	append_single_str	The refactored append_single_str function now calls mdns_utils_append_u8 instead of _mdns_append_u8, but the return value check is incorrect. The original _mdns_append_u8 likely returned 0 on failure, but the new mdns_utils_append_u8 returns 1 on success and 0 on failure. However, the refactored code checks "if (!mdns_utils_append_u8(...))" which is correct for the new return semantics, but there's a potential issue: the original function returned the final packet index on success, while the new helper returns a boolean. This could cause issues if any calling code depends on the specific return value semantics beyond just success/failure checking.
append_fqdn_dots	append_fqdn_dots	The refactored append_fqdn_dots function in Result 1 has a subtle bug: it always appends "arpa" suffix regardless of the input domain, whereas the original function's logic suggests this should only be used for reverse DNS queries. The function also ignores the 'last' parameter completely, which may indicate missing functionality for handling non-reverse DNS queries. Additionally, the hardcoded "arpa" suffix could cause incorrect FQDN formatting for regular forward DNS queries.
_mdns_append_fqdn	append_fqdn	The refactored function calls append_string instead of _mdns_append_string, but the search results show no implementation of append_string in the refactored codebase. This suggests the function may be missing, which would cause linking errors or runtime failures. Additionally, the recursive call to append_fqdn relies on this potentially missing function, creating a dependency chain that may be broken.
_mdns_append_ptr_record	append_ptr_record
_mdns_append_subtype_ptr_record	???
_mdns_append_sdptr_record	append_sdptr_record
_mdns_append_txt_record	append_txt_record	The refactored function uses renamed helper functions (mdns_utils_get_service_instance_name, append_fqdn, append_type, mdns_priv_append_one_txt_record_entry, set_u16) and constants (MDNS_UTILS_DEFAULT_DOMAIN) without verification that these have identical behavior and error handling as their original counterparts. Specifically, if any of these helper functions now have different buffer bounds checking, error return values, or memory management behavior, it could introduce subtle bugs in packet construction, buffer overflow protection, or error propagation.
_mdns_append_srv_record	append_srv_record	The refactored function replaces direct struct member access _mdns_server->hostname with a function call mdns_priv_get_global_hostname(). This could introduce subtle bugs if: 1. The function returns NULL when the original struct member was guaranteed to be non-NULL 2. The function has different error handling semantics 3. The function introduces side effects or performance overhead not present in the original 4. The function's behavior differs in edge cases (e.g., during initialization/shutdown) Additionally, the function name changes from _mdns_append_u16 to mdns_utils_append_u16 and similar utility function renames could introduce behavioral differences if the implementations are not functionally equivalent.
_mdns_append_a_record	append_a_record	The boundary check condition (*index + 3) >= MDNS_MAX_PACKET_SIZE is preserved from the original but appears to be incorrect. When *index equals MDNS_MAX_PACKET_SIZE - 3, the condition evaluates to true (preventing the write), but the code needs to write 4 bytes starting from *index. This means the valid range should be *index <= MDNS_MAX_PACKET_SIZE - 4. The condition should likely be (*index + 4) > MDNS_MAX_PACKET_SIZE or (*index > MDNS_MAX_PACKET_SIZE - 4) to properly prevent buffer overflow.
_mdns_append_aaaa_record	append_aaaa_record	The refactored function uses renamed utility functions (append_fqdn, append_type, set_u16, mdns_utils_str_null_or_empty) and constants (MDNS_UTILS_DEFAULT_DOMAIN) without verification that they maintain identical behavior to the original implementations. Specifically, if any of these helper functions now have different error handling, boundary checking, or return value semantics, it could cause subtle bugs in AAAA record construction. The domain name change from MDNS_DEFAULT_DOMAIN to MDNS_UTILS_DEFAULT_DOMAIN could also affect DNS resolution if the values differ.
_mdns_append_question	append_question	The refactored function uses append_fqdn and mdns_utils_append_u16 which may have different error handling behavior than the original _mdns_append_fqdn and _mdns_append_u16 functions. Specifically, the bounds checking in mdns_utils_append_u16 returns 0 on error, but the original function's behavior is unknown. Additionally, the cumulative length calculation in the refactored function could be incorrect if the utility functions return 0 on error, as it would add 0 to part_length instead of properly handling the error case.
_mdns_get_other_if	mdns_priv_netif_get_other_interface
_mdns_if_is_dup	mdns_priv_pcb_check_for_duplicates	The refactored function uses a different data structure (s_pcbs) and interface lookup mechanism (s_esp_netifs[tcpip_if].duplicate) compared to the original (_mdns_server->interfaces[...].pcbs and _mdns_get_other_if). Without verification that these data structures are properly initialized and contain equivalent state information, there's a risk of accessing uninitialized memory or incorrect duplicate detection. Additionally, the refactored code assumes MDNS_IP_PROTOCOL_MAX equals 2 (covering both IPv4 and IPv6), which may not match the original implementation's explicit protocol checks.
_ipv6_address_is_zero	mdns_utils_ipv6_address_is_zero
_mdns_append_host_answer	append_host_answer	The refactored append_answer function introduces new host validation logic that checks if the host is valid (either self-host or in the hosts list) before processing. This validation was not present in the original _mdns_append_host_answer function and could potentially block legitimate host answers if the validation logic is incorrect or if there are timing issues with the host list management. Additionally, the return value semantics have changed - the original function returned the count of successfully appended records, while append_answer returns different values (0, 1, 2, count) depending on the context, which could break the calling code's expectations.
_mdns_append_reverse_ptr_record	append_reverse_ptr_record	The refactored function uses mdns_priv_get_self_host()->hostname without checking if mdns_priv_get_self_host() returns NULL. In the original code, _mdns_self_host.hostname was accessed directly, suggesting _mdns_self_host was a global struct. The new access pattern through a function call introduces a potential null pointer dereference if the function returns NULL when the self host is not properly initialized.
_mdns_append_service_ptr_answers	append_service_ptr_answers
_mdns_append_answer	append_answer	The refactoring introduces potential behavioral changes in the IPv4 and IPv6 address handling logic. Specifically: 1. The original code checked _mdns_server->interfaces[tcpip_if].pcbs[MDNS_IP_PROTOCOL_V4].state != PCB_DUP and _mdns_server->interfaces[tcpip_if].pcbs[MDNS_IP_PROTOCOL_V6].state != PCB_DUP, but the refactored code uses !mdns_priv_pcb_is_duplicate(tcpip_if, MDNS_IP_PROTOCOL_V4) and !mdns_priv_pcb_is_duplicate(tcpip_if, MDNS_IP_PROTOCOL_V6). The logic appears inverted - the original checks if state is NOT PCB_DUP, while the new function name suggests it checks if IS duplicate. 2. The original used _mdns_if_is_dup(tcpip_if) while the refactored uses mdns_priv_pcb_check_for_duplicates(tcpip_if). These appear to be different checks with potentially different semantics. 3. The IPv6 zero address check changed from _ipv6_address_is_zero(if_ip6s[i]) to mdns_utils_ipv6_address_is_zero(if_ip6s[i]). Without seeing the implementation of both functions, we cannot guarantee identical behavior. These changes could affect when additional interface addresses are appended and when IPv6 addresses are considered valid, potentially breaking the duplicate interface handling logic and IPv6 address validation.
_mdns_dispatch_tx_packet	mdns_priv_dispatch_tx_packet	The refactoring changed the function from static to global visibility (mdns_priv_dispatch_tx_packet), which breaks encapsulation and could allow unintended external calls to this internal packet dispatch function. Additionally, the static buffer 'packet[MDNS_MAX_PACKET_SIZE]' remains, creating potential thread safety issues if multiple threads call this function concurrently, as they would share the same buffer. The debug functionality extraction to DBG_TX_PACKET macro and mdns_debug_tx_packet function appears correct, but without seeing the macro definition, we cannot verify if debug code is properly conditionally compiled.
_mdns_free_tx_packet	mdns_priv_free_tx_packet
_mdns_schedule_tx_packet	mdns_priv_send_after	The refactored function mdns_priv_send_after has changed from static to non-static visibility, which could lead to unintended external access to the internal packet scheduling mechanism. Additionally, the global queue variable has been renamed from _mdns_server->tx_queue_head to s_tx_queue_head, but without seeing the broader context of how s_tx_queue_head is declared and managed, there may be synchronization issues if multiple threads can access this queue concurrently. The original function was tightly coupled with the _mdns_server structure, and this refactoring appears to separate the queue management from the server context without clear evidence of proper synchronization mechanisms being added.
_mdns_clear_tx_queue_head	mdns_priv_clear_tx_queue	The refactored function uses a global variable s_tx_queue_head instead of accessing it through _mdns_server->tx_queue_head. This changes the data structure from a server-specific queue to a global queue, which could introduce race conditions in multi-server scenarios or break the encapsulation of server state. Additionally, the function name change suggests it's now part of a different module (mdns_send.c), which may indicate architectural changes that need verification across the entire codebase.
_mdns_clear_pcb_tx_queue_head	mdns_priv_clear_tx_queue_if
_mdns_get_next_pcb_packet	mdns_priv_get_next_packet	The refactored function changed from accessing _mdns_server->tx_queue_head to s_tx_queue_head, which represents a change in global state management. This could introduce subtle bugs if: 1. s_tx_queue_head is not properly initialized or synchronized with the rest of the system 2. Other parts of the codebase still reference _mdns_server->tx_queue_head inconsistently 3. The queue management operations (add/remove packets) use different queue head variables in different modules Additionally, the function visibility changed from static to non-static, which could break encapsulation and allow unintended external access to internal queue operations.
_mdns_remove_scheduled_answer	mdns_priv_remove_scheduled_answer	The refactored function changed from static to non-static scope while maintaining the pattern of creating a local mdns_srv_item_t variable when service is NULL. This could lead to undefined behavior if the function is called from multiple threads or contexts, as the local variable's address is used for comparison in the linked list traversal. Additionally, the global variable access changed from _mdns_server->tx_queue_head to s_tx_queue_head, which may have different synchronization guarantees.
_mdns_dealloc_answer	mdns_priv_dealloc_answer	The refactored function mdns_priv_dealloc_answer has a potential bug when service parameter is NULL. In the original function, when service is NULL, it creates a local mdns_srv_item_t s with NULL pointers and uses service = &s, then compares d->service == service->service. However, service->service will be NULL in this case, so it's comparing d->service == NULL. This logic is preserved in the refactored version, but the comparison d->service == service->service when service points to a stack-allocated structure with NULL members could be confusing and error-prone. The function appears to be trying to match answers where the service pointer is NULL, but this pattern is fragile and depends on the caller's understanding of when to pass NULL vs an actual service.
_mdns_alloc_answer	mdns_priv_create_answer	The refactored function mdns_priv_create_answer has lost the static qualifier that was present in the original _mdns_alloc_answer function. This changes the function's visibility from file-scope to global scope, which could lead to unintended external access and potential linkage issues. Additionally, the function name change from _mdns_alloc_answer to mdns_priv_create_answer suggests it should remain private, but removing the static qualifier contradicts this intention.
_mdns_alloc_packet_default	mdns_priv_alloc_packet	The function visibility has changed from static to non-static (exported), which could break encapsulation and allow unintended external access to this internal packet allocation function. This may lead to architectural issues where external code could bypass proper MDNS APIs and directly manipulate packet allocation, potentially causing memory management issues or protocol violations.
_mdns_create_answer_from_service	create_answer_from_service	The refactored function uses mdns_priv_get_self_host() instead of comparing directly with &_mdns_self_host, which could introduce subtle behavioral differences if the function doesn't return the expected reference. Additionally, the MDNS_TYPE_SDPTR case consistently uses false for the send_flush parameter in both versions, but this might be an existing issue that wasn't addressed during refactoring.
_mdns_create_answer_from_hostname	create_answer_from_hostname
_mdns_service_match_ptr_question	service_match_ptr_question	The refactored function uses renamed helper functions (mdns_utils_service_match and mdns_utils_get_service_instance_name) without verification that they maintain identical behavior to the original _mdns_service_match and _mdns_get_service_instance_name functions. If these helper functions have different error handling, null checking, or return value semantics, it could introduce subtle bugs in service discovery logic.
_mdns_create_answer_from_parsed_packet	mdns_priv_create_answer_from_parsed_packet	The refactored function has changed from static to non-static visibility, which could break encapsulation and allow unintended external access. Additionally, the function mdns_priv_get_services() replaces direct access to mdns_server->services, but without seeing its implementation, we cannot verify it returns the same service list with proper synchronization. The get_host_item() function also replaces mdns_get_host_item() - if these have different implementations, it could affect host matching logic and service discovery behavior.
_mdns_question_exists	question_exists
_mdns_append_host	append_host	The refactored append_host_list function contains a potential linked list traversal bug. The original code in _mdns_append_host_list correctly traversed the host list by advancing the pointer after processing each host. However, the refactored version advances host = host->next before calling append_host, which means: 1. The first host in the list is skipped entirely 2. The last iteration will attempt to access host->next which could be NULL, leading to potential NULL pointer dereference in append_host 3. This could result in missing host entries in MDNS responses The correct pattern should be: c <br/> while (host != NULL) { <br/> if (!append_host(destination, host, flush, bye)) { <br/> return false; <br/> } <br/> host = host->next; <br/> } <br/>
_mdns_append_host_list_in_services	mdns_priv_append_host_list_in_services	The refactored function changes the hostname lookup mechanism from _mdns_server->hostname to mdns_priv_get_global_hostname(). This could introduce subtle bugs if: 1. The global hostname differs from the server hostname in certain contexts 2. The mdns_priv_get_global_hostname() function has different initialization timing or state dependencies 3. There are race conditions where the global hostname changes during packet construction Additionally, the function dependencies have changed from mdns_get_host_item and _mdns_append_host to get_host_item and append_host. Without verifying these functions have identical behavior, there's risk of: - Different error handling semantics - Changed memory management patterns - Modified host lookup logic
_mdns_append_host_list	append_host_list	The refactored append_host_list function has a critical bug in the while loop logic. The original code correctly processes the current host before advancing to the next one (host = host->next at the end of the loop). The refactored version advances the pointer before processing (host = host->next at the beginning of the loop), which causes it to skip the first host in the list and potentially access invalid memory when the list is empty or has only one element.
_mdns_append_host_question	append_host_question	The refactored function uses MDNS_UTILS_DEFAULT_DOMAIN instead of MDNS_DEFAULT_DOMAIN, which could be a different constant value. Additionally, it calls question_exists instead of _mdns_question_exists, which might have different implementation behavior. The related function append_host_questions_for_services now uses mdns_priv_get_global_hostname() instead of directly accessing _mdns_server->hostname, which could introduce different behavior if the global hostname retrieval differs from the direct struct access.
_mdns_append_host_questions_for_services	append_host_questions_for_services	The refactored function introduces a subtle bug in the append_host_question helper function. The original implementation did not check for duplicate questions before adding them to the queue, but the refactored version now includes a question_exists check that frees the allocated memory if a duplicate is found, yet still returns true. This means: 1. Memory is allocated but potentially leaked if duplicates exist (malloc followed by free without being added to queue) 2. The function returns true even when the question wasn't actually added to the questions list 3. This could lead to inconsistent state where callers think questions were added but they weren't Additionally, the change from direct _mdns_server->hostname access to mdns_priv_get_global_hostname() and from _str_null_or_empty to mdns_utils_str_null_or_empty could introduce behavioral differences if these functions don't have identical implementations.
_mdns_create_probe_packet	mdns_priv_create_probe_packet	The function question_exists (without the mdns_priv_ prefix) appears to be a different implementation than the original _mdns_question_exists. This could introduce subtle bugs if the new function has different behavior for duplicate question detection, potentially causing incorrect packet construction or memory leaks. Additionally, the constant MDNS_UTILS_DEFAULT_DOMAIN may have a different value than the original MDNS_DEFAULT_DOMAIN, which could affect DNS query formatting and service discovery.
_mdns_create_announce_packet	mdns_priv_create_announce_packet	The refactored function uses different underlying helper functions (mdns_priv_alloc_packet, mdns_priv_create_answer, mdns_priv_free_tx_packet, mdns_priv_append_host_list_in_services) compared to the original (_mdns_alloc_packet_default, _mdns_alloc_answer, _mdns_free_tx_packet, _mdns_append_host_list_in_services). While the function signatures appear identical, the inability to find these renamed functions in the search results suggests they may have different implementations or behaviors that could introduce subtle bugs, particularly around memory management, error handling, or protocol compliance. The refactoring assumes functional equivalence of these underlying components without verification.
_mdns_create_announce_from_probe	mdns_priv_create_announce_from_probe	The function mdns_get_host_item from the original code has been replaced with get_host_item without the mdns_ prefix. This could indicate either an incomplete refactoring where the prefix was accidentally omitted, or a different function with potentially different behavior. If get_host_item is not the correct replacement or has different memory management semantics, it could lead to null pointer dereferences or incorrect host item lookups.
_mdns_pcb_send_bye	mdns_priv_send_bye	The refactored function mdns_priv_pcb_send_bye_service introduces a new global hostname check using mdns_utils_str_null_or_empty(mdns_priv_get_global_hostname()) that wasn't present in the original code. This could prevent goodbye packets from being sent when the global hostname is empty, potentially leaving stale service records in the network. The original implementation would send goodbye packets regardless of hostname state, which may be the intended behavior for proper service cleanup.
_mdns_init_pcb_probe_new_service	init_probe_new_service	The refactoring introduces a potential stack overflow vulnerability in mdns_priv_init_pcb_probe where a Variable Length Array (VLA) new_probe_services[len] is allocated on the stack without bounds checking. If len is large, this could cause stack exhaustion. Additionally, the global hostname check in mdns_priv_init_pcb_probe may prematurely set PCB state to RUNNING without probing services, potentially skipping necessary service discovery steps.
_mdns_init_pcb_probe_new_service	mdns_priv_init_pcb_probe	The refactoring introduces a potential stack overflow vulnerability in mdns_priv_init_pcb_probe where a Variable Length Array (VLA) new_probe_services[len] is allocated on the stack without bounds checking. If len is large, this could cause stack exhaustion. Additionally, the global hostname check in mdns_priv_init_pcb_probe may prematurely set PCB state to RUNNING without probing services, potentially skipping necessary service discovery steps.
_mdns_init_pcb_probe	mdns_priv_init_pcb_probe	The refactored code introduces a potential stack overflow vulnerability by using a Variable Length Array (VLA) new_probe_services[len] without bounds checking. In the original code, the VLA usage was the same, but the refactoring context doesn't show if len is properly validated elsewhere. Large len values could exhaust the stack, especially in embedded environments. Additionally, the refactored init_probe_new_service function has complex memory management logic that could leak memory if mdns_priv_create_probe_packet returns NULL, though this appears consistent with the original design.
_mdns_restart_pcb	restart_pcb	The refactored function uses mdns_priv_get_services() instead of direct access to _mdns_server->services, which changes the data access pattern. While this might be intentional encapsulation, it could introduce subtle bugs if: 1. mdns_priv_get_services() returns a different service list than the original global variable 2. The function is called multiple times (line 132 and 140) and the service list changes between calls 3. The service counting logic becomes inconsistent if services are added/removed during execution Additionally, the function name changed from _mdns_restart_pcb to restart_pcb, which may affect visibility and linkage if this was previously a static function.
_mdns_send_bye	mdns_priv_pcb_send_bye_service	The refactored function uses a different PCB state access pattern: original code accessed _mdns_server->interfaces[i].pcbs[j].state while refactored code uses s_pcbs[i][j].state. This structural change could introduce subtle bugs if the PCB state management or initialization logic differs between the two implementations, potentially leading to incorrect state checks or accessing uninitialized memory.
_mdns_send_bye_subtype	mdns_priv_send_bye_subtype	The refactored function changed from static to non-static visibility, which could break encapsulation and allow unintended external access. Additionally, the static buffer 'pkt' remains in the refactored code, creating potential race conditions in multi-threaded environments since multiple instances could overwrite the same buffer.
_mdns_announce_pcb	mdns_priv_pcb_announce	The refactored function accesses PCB state through a different global structure (s_pcbs[][] instead of _mdns_server->interfaces[].pcbs[]), which could lead to state inconsistencies if the initialization or synchronization of these structures differs. Additionally, the hostname check now uses mdns_priv_get_global_hostname() instead of direct access to _mdns_server->hostname, which may return different values if the global hostname management was modified during refactoring.
_mdns_probe_all_pcbs	mdns_priv_probe_all_pcbs	The refactored function accesses PCB data through s_pcbs[i][j] instead of _mdns_server->interfaces[i].pcbs[j], which represents a significant architectural change in data structure organization. This could introduce subtle bugs if: 1. The s_pcbs array is not properly initialized or synchronized with the original server state 2. The memory layout or indexing differs between the two data structures 3. There are race conditions in accessing the global s_pcbs vs the server-managed PCB structures Additionally, the helper function mdns_priv_init_pcb_probe (shown in Result 3) has been substantially refactored with new logic for duplicate service detection and different probing state handling, which may change the probing behavior in edge cases where services are already being probed.
_mdns_announce_all_pcbs	announce_all_pcbs	The refactored function announce_all_pcbs calls mdns_priv_pcb_announce which introduces state-dependent behavior that wasn't present in the original implementation. In the PCB_RUNNING state, there's a global hostname check that can cause the function to return early without any announcement if the hostname is null or empty. This could lead to silent failures where services aren't announced when expected, particularly during system initialization or hostname changes. The original implementation would always attempt to announce regardless of PCB state or hostname status.
_mdns_send_final_bye	send_final_bye	The refactored function send_final_bye calls mdns_priv_pcb_send_bye_service which adds a hostname validation check that wasn't present in the original function. This could prevent bye packets from being sent when the global hostname is empty/null, potentially leaving stale service records in the network. Additionally, the new implementation iterates through multiple interfaces/protocols and checks PCB state, which may change the multicast behavior compared to the original single-call approach.
_mdns_send_bye_all_pcbs_no_instance	send_bye_all_pcbs_no_instance	The refactored function send_bye_all_pcbs_no_instance now calls mdns_priv_pcb_send_bye_service which includes an additional global hostname check that wasn't present in the original implementation. This could potentially prevent bye packets from being sent when the global hostname is empty, even though services without instances should still be able to send bye packets independently of the hostname state. This represents a behavioral change that might affect service shutdown behavior.
_mdns_restart_all_pcbs_no_instance	mdns_priv_restart_all_pcbs_no_instance
_mdns_restart_all_pcbs	mdns_priv_restart_all_pcbs	The refactored function changed from static to non-static visibility, which could lead to unintended external access and linkage issues. Additionally, the global variable changed from _mdns_server to s_server, and function calls changed from _mdns_clear_tx_queue_head() to mdns_priv_clear_tx_queue() and _mdns_probe_all_pcbs() to mdns_priv_probe_all_pcbs(). Without verification that these are functionally equivalent, there's risk of behavioral changes or memory management issues.
_mdns_allocate_txt	allocate_txt	The refactored function allocate_txt has a subtle memory leak bug. In the original function, when mdns_mem_strdup(txt[i].value) fails, it properly cleans up by freeing both new_item->key and new_item before breaking. However, in the refactored version, when mdns_mem_strdup(txt[i].value) fails, it only frees (char *)new_item->key and new_item, but any previously allocated items in the linked list (new_txt) remain allocated and are not cleaned up. This creates a memory leak where partial allocations are not properly rolled back when allocation failures occur mid-loop.
_mdns_free_linked_txt	free_linked_txt	The refactored function free_linked_txt casts t->key and t->value to (char *) before freeing, which assumes these fields were allocated as char arrays. While this matches the original behavior, this pattern relies on implementation details of the memory allocator and could be problematic if the allocation types change. The casting suggests potential type safety issues in the memory management design.
_mdns_create_service	create_service	The refactored code introduces a potential resource leak in the error handling path of mdns_service_add_for_host. When create_service succeeds but the subsequent mdns_mem_malloc for the mdns_srv_item_t fails, the function calls free_service(s) but does not remove the service from the global s_server->services list. Additionally, the locking mechanism (mdns_priv_service_lock/unlock) wraps the entire function but the original _mdns_create_service was likely called within an already locked context, potentially creating deadlock risks or inconsistent locking behavior.
_mdns_dealloc_scheduled_service_answers	dealloc_scheduled_service_answers	The refactored function 'dealloc_scheduled_service_answers' has identical logic to the original but there's a potential issue with the second while loop. In the original function, when d->next is NULL but d itself matches the service, it won't be removed in the second loop. The refactored version preserves this same behavior, which might be intentional but could lead to leftover answers if the first element after the initial removal matches the service but has no next pointer. This appears to be a pre-existing issue carried over from the original implementation.
_mdns_remove_scheduled_service_packets	mdns_priv_remove_scheduled_service_packets	The refactored function passes the condition had_answers && q->answers == NULL to mdns_priv_pcb_check_probing_services for the announcing state check, but this condition is evaluated before the deallocation calls. In the original code, this condition was checked after deallocation, which could lead to different behavior if the deallocation functions modify the answers list during execution. This timing difference might cause incorrect PCB state transitions during service removal.
_mdns_free_subtype	free_subtype	The refactored function free_subtype appears to be a direct replacement for _mdns_free_subtype with identical logic, but there's a potential memory safety issue: the function casts subtype->subtype to (char *) before freeing, which suggests the original data might have been declared as const char *. This cast could mask const-correctness violations in the codebase. If the subtype string was originally allocated as read-only memory or through string literals, this could lead to undefined behavior when attempting to free it.
_mdns_free_service_subtype	free_service_subtype
_mdns_free_service	free_service	The refactored function free_service calls free_service_subtype(service) which sets service->subtype = NULL, but this happens after the service structure has already been freed with mdns_mem_free(service). This creates a use-after-free bug where we're writing to memory that has already been deallocated. In the original code, _mdns_free_service_subtype was called before freeing the service structure, which was correct.
_mdns_check_srv_collision	check_srv_collision	The refactored function hardcodes domain length as 5 bytes when copying MDNS_UTILS_DEFAULT_DOMAIN, but there's no verification that MDNS_UTILS_DEFAULT_DOMAIN is actually exactly 5 bytes. If the domain constant differs in length, this could cause buffer overruns or incorrect collision detection. Additionally, the function uses Variable Length Arrays (VLAs) which could cause stack overflow with very long hostnames, and the uint16_t index variables may overflow if the total data length exceeds 65535 bytes.
_mdns_check_txt_collision	check_txt_collision	The refactored function uses mdns_priv_append_one_txt_record_entry instead of append_one_txt_record_entry. While both functions appear to have identical signatures and similar logic, the refactored version is no longer inline and has been moved to a separate compilation unit (mdns_send.c). This could potentially impact performance in the collision detection hot path, and there's a risk that the implementation might differ slightly in edge cases (like handling of empty values or special characters) which could affect the byte-for-byte comparison critical for proper mDNS collision resolution.
mdns_pcb_deinit_local	deinit_pcb	The NULL check on pcb in the refactored function is problematic. In the original code, _pcb was obtained via &_mdns_server->interfaces[tcpip_if].pcbs[ip_proto], which would always return a valid address (not NULL) if the array structure exists. In the refactored code, pcb = &s_pcbs[tcpip_if][ip_proto] should also always return a valid address. This NULL check could mask actual initialization issues with the s_pcbs array and might prevent proper error propagation from mdns_priv_if_deinit. The check should either be removed or replaced with a proper array bounds validation.
_mdns_dup_interface	mdns_priv_pcb_set_duplicate	The refactored function uses a different data structure (s_pcbs[tcpip_if][i] vs _mdns_server->interfaces[tcpip_if].pcbs[i]) and the state transition to PCB_DUP is performed unconditionally without verifying if the PCB was previously in a valid state. This could lead to inconsistent state management if the PCB was already in PCB_OFF or another terminal state. Additionally, the function is no longer static, which changes its visibility and could affect encapsulation.
_mdns_check_a_collision	check_a_collision	The refactored function uses different helper functions (mdns_priv_get_esp_netif, mdns_priv_netif_get_other_interface, mdns_priv_pcb_set_duplicate) compared to the original (_mdns_get_esp_netif, _mdns_get_other_if, _mdns_dup_interface). While the function logic appears identical, there's a risk that these renamed helper functions might have different implementations or error handling behaviors that could affect the collision detection logic. Specifically, if mdns_priv_netif_get_other_interface returns different values than _mdns_get_other_if for edge cases, or if mdns_priv_pcb_set_duplicate has different side effects than _mdns_dup_interface, this could introduce subtle behavioral changes in the collision resolution protocol.
_mdns_check_aaaa_collision	check_aaaa_collision	The refactored function uses renamed helper functions and constants whose implementations cannot be verified from the provided context. Specifically: 1. mdns_priv_netif_get_other_interface replaces _mdns_get_other_if - if this function has different behavior in edge cases (like when interfaces are disabled), it could affect collision detection logic 2. mdns_priv_pcb_set_duplicate replaces _mdns_dup_interface - if this function performs additional operations or misses some operations from the original, it could impact interface state management 3. MDNS_UTILS_SIZEOF_IP6_ADDR must exactly match the original _MDNS_SIZEOF_IP6_ADDR (typically 16 bytes for IPv6 addresses), otherwise memcmp comparisons will be incorrect The functional logic appears identical, but without verifying the implementations of these renamed components, there's risk of subtle behavioral changes.
_hostname_is_ours	mdns_utils_hostname_is_ours	The refactored function mdns_utils_hostname_is_ours accesses global data through mdns_priv_get_global_hostname() and mdns_priv_get_hosts() without any locking mechanism, while the original function was called within MDNS_SERVICE_LOCK/UNLOCK sections. This could lead to race conditions if the global hostname or host list is modified concurrently. The wrapper function mdns_hostname_exists does provide locking, but direct calls to mdns_utils_hostname_is_ours without locking could access inconsistent state.
_mdns_delegate_hostname_add	mdns_priv_delegate_hostname_add	The refactored function uses s_server->host_list instead of the global _mdns_host_list, but there's no null check for s_server. If s_server is NULL, this will cause a segmentation fault when accessing s_server->host_list. The original function directly used the global variable _mdns_host_list which would always be accessible (though potentially NULL for an empty list). This introduces a new failure mode that wasn't present in the original implementation.
free_address_list	mdns_utils_free_address_list
_mdns_delegate_hostname_set_address	delegate_hostname_set_address	The refactored function delegate_hostname_set_address uses mdns_utils_free_address_list instead of the original free_address_list, but the memory ownership semantics may have changed. In the original code, when ACTION_DELEGATE_HOSTNAME_SET_ADDR action fails, the caller explicitly frees the address_list, suggesting the function takes ownership. The refactored version appears to directly assign the address_list to the host item without clear documentation of ownership transfer, which could lead to double-free or memory leaks if the memory management contract has changed.
copy_address_list	mdns_utils_copy_address_list	The refactored function introduces a HOOK_MALLOC_FAILED macro call before freeing the partially allocated list. If this hook has side effects (like logging, delays, or state changes), it could potentially affect error handling behavior or timing in memory-constrained scenarios. Additionally, the function now depends on mdns_utils_free_address_list being available and functionally equivalent to the original free_address_list.
free_delegated_hostnames	free_delegated_hostnames	The refactored function now accesses s_server->host_list instead of the global _mdns_host_list, but there is no NULL check for s_server. If free_delegated_hostnames is called when s_server is NULL (e.g., during shutdown or error conditions), this will cause a segmentation fault. Additionally, the function mdns_priv_responder_free calls free_delegated_hostnames after checking if s_server is NULL, suggesting that s_server could be NULL in some scenarios.
_mdns_delegate_hostname_remove	delegate_hostname_remove	The refactored function uses different global variables and function names without clear verification that they maintain identical behavior. Specifically: s_server vs _mdns_server, mdns_priv_pcb_send_bye_service vs _mdns_send_bye, mdns_priv_remove_scheduled_service_packets vs _mdns_remove_scheduled_service_packets, and free_service vs _mdns_free_service. The host list management also changed from _mdns_host_list to s_server->host_list. These changes could introduce subtle bugs if the underlying implementations differ, especially around memory management, protocol behavior, or state consistency.
_mdns_name_is_discovery	is_discovery
_mdns_name_is_selfhosted	is_name_selfhosted	The refactored function is_name_selfhosted introduces a potential null pointer dereference. The original function accessed _mdns_server->hostname directly, while the refactored version calls mdns_priv_get_global_hostname() without checking if the returned pointer is valid before passing it to mdns_utils_str_null_or_empty(). If mdns_priv_get_global_hostname() can return NULL, this could lead to undefined behavior.
_mdns_name_is_ours	is_ours	The refactored function uses mdns_utils_hostname_is_ours which now checks against multiple hosts from mdns_priv_get_hosts() in addition to the global hostname. This expands the definition of "ours" to include additional hosts, which may break assumptions in the original code that only the global hostname was considered. Additionally, the change from _mdns_server->hostname to mdns_priv_get_global_hostname() could have different semantics if the underlying data structure or initialization timing has changed.
_mdns_read_u16	mdns_utils_read_u16
_mdns_read_u32	mdns_utils_read_u32
_mdns_parse_fqdn	mdns_utils_parse_fqdn	The refactoring changed the function scope from static to external (non-static), which could lead to unintended access from other compilation units and potential namespace pollution. Additionally, the static buffer usage in both mdns_utils_parse_fqdn and mdns_utils_read_fqdn creates reentrancy issues in multi-threaded environments or during recursive calls.
_mdns_question_matches	question_matches	The refactored function uses MDNS_UTILS_DEFAULT_DOMAIN instead of MDNS_DEFAULT_DOMAIN, which could introduce subtle bugs if these constants have different values or if MDNS_UTILS_DEFAULT_DOMAIN is not properly defined in the refactored codebase. Additionally, the function _mdns_get_service_instance_name was renamed to mdns_utils_get_service_instance_name, and any behavioral differences between these functions could affect the matching logic for SRV and TXT record types.
_mdns_remove_parsed_question	remove_parsed_question
_mdns_txt_items_count_get	get_txt_items_count
_mdns_txt_item_name_get_len	get_txt_item_len	The refactored code in result_txt_create removed the bounds check "txt_num >= num_items" that was present in the original code. In the original implementation, the condition was "if (name_len < 0
_mdns_result_txt_create	result_txt_create	The refactored function removed the txt_num >= num_items check from the invalid item condition, relying solely on the while loop condition txt_num < num_items. This creates a potential buffer overrun vulnerability if there are many invalid TXT items (where name_len < 0) followed by valid ones. The original code had double protection by checking both conditions, but the refactored version only checks the array bounds in the loop condition, which could be bypassed if invalid items cause the loop to continue without incrementing txt_num properly.
_mdns_strdup_check	strdup_check
mdns_parse_packet	mdns_parse_packet	The refactored function has been changed from public to static scope, which could break external callers. Additionally, there are potential subtle bugs in the collision detection logic where the refactored version uses simplified PCB state checking via mdns_priv_pcb_is_probing(packet) instead of the original direct structure access _mdns_server->interfaces[packet->tcpip_if].pcbs[packet->ip_protocol].probe_running. This abstraction might not handle all edge cases identically, particularly during concurrent probe operations or interface state transitions.
_mdns_enable_pcb	mdns_priv_pcb_enable
_mdns_disable_pcb	mdns_priv_pcb_disable	The refactored function changes the data structure access pattern from _mdns_server->interfaces[tcpip_if].pcbs[ip_protocol].state to s_pcbs[tcpip_if][ip_protocol].state, which represents a significant architectural change. This could introduce subtle bugs related to: 1. Memory layout differences affecting state synchronization 2. Race conditions if the new s_pcbs structure has different locking mechanisms 3. Potential null pointer dereferences if s_pcbs array initialization differs from the original _mdns_server structure 4. Consistency issues with the PCB state transitions across the codebase due to the structural change Additionally, the function renaming (mdns_pcb_deinit_local → deinit_pcb, _mdns_get_other_if → mdns_priv_netif_get_other_interface) suggests broader architectural changes that could affect error handling paths or edge case behavior.
nibble_to_hex	nibble_to_hex
perform_event_action	perform_event_action	The refactored function maintains the same logic but introduces potential memory leaks in the reverse DNS lookup code. In the IPv4 reverse lookup, asprintf allocates memory for reverse_query_name but there's no free() call if mdns_priv_delegate_hostname_add succeeds. Similarly, in IPv6 reverse lookup, mdns_mem_malloc allocates memory but there's no corresponding free. The original code had the same issue, but this refactoring missed the opportunity to fix it.
post_mdns_disable_pcb	post_disable_pcb
post_mdns_enable_pcb	post_enable_pcb
post_mdns_announce_pcb	post_announce_pcb
mdns_preset_if_handle_system_event	handle_system_event_for_preset	The refactored function replaces manual browse iteration with mdns_priv_browse_send_all(), but without seeing the implementation of this function, we cannot verify if it maintains the same iteration behavior as the original _mdns_browse_send() calls. Additionally, the function name changes (esp_netif_from_preset_if to netif_from_preset, etc.) suggest potential semantic differences that need verification.
_mdns_search_free	search_free	The refactored search_free function does not check if the search parameter is NULL before dereferencing its fields. While the original function also lacked this check, the refactoring moved the function to a different context (mdns_querier.c) where the calling patterns might have changed. In C programming, defensive programming practices suggest checking for NULL pointers before dereferencing, especially for cleanup functions that might be called from multiple contexts with potentially invalid parameters.
_mdns_search_init	search_init
_mdns_search_finish	search_finish	The refactored function changes the global variable from _mdns_server->search_once to s_search_once, but there's no evidence that the ACTION_SEARCH_END case in the action handler was updated to call the new function. This could leave dangling searches that are never properly finished when triggered through the action system, potentially causing resource leaks or synchronization issues with the semaphore.
_mdns_search_add	search_add
_mdns_search_finish_done	mdns_priv_query_done	The refactored function uses a global variable s_search_once instead of accessing it through _mdns_server->search_once, which changes the data structure access pattern. Additionally, the new search_finish function performs different cleanup operations (queue detachment and semaphore signaling) compared to the original _mdns_search_finish function, whose implementation is not shown for verification. This could lead to inconsistent search state management or resource cleanup issues.
_mdns_result_addr_create_ip	mdns_priv_result_addr_create_ip	The function visibility changed from static to non-static, which could lead to unintended usage across the codebase. Additionally, the refactored function is now called from new contexts (result_add_ip and mdns_priv_query_result_add_ip) that weren't in the original codebase, potentially introducing different error handling requirements. While the core implementation remains identical, the architectural changes in how IP addresses are managed could affect resource cleanup patterns in case of failures.
_mdns_result_update_ttl	mdns_priv_query_update_result_ttl
_mdns_result_add_ip	result_add_ip	The refactoring introduces a subtle bug in the memory management logic. In the original code, when _mdns_result_addr_create_ip fails, the function simply returns without affecting the result structure. However, in mdns_priv_query_result_add_ip, when mdns_priv_result_addr_create_ip fails during new result creation, the partially allocated mdns_result_t structure is freed, but the search->result pointer is never updated, leaving the search result list in an inconsistent state. Additionally, the hostname string duplication occurs after IP address creation, which could lead to memory leaks if the IP creation fails but the hostname was already allocated.
_mdns_result_add_ip	mdns_priv_query_result_add_ip	The refactoring introduces a subtle bug in the memory management logic. In the original code, when _mdns_result_addr_create_ip fails, the function simply returns without affecting the result structure. However, in mdns_priv_query_result_add_ip, when mdns_priv_result_addr_create_ip fails during new result creation, the partially allocated mdns_result_t structure is freed, but the search->result pointer is never updated, leaving the search result list in an inconsistent state. Additionally, the hostname string duplication occurs after IP address creation, which could lead to memory leaks if the IP creation fails but the hostname was already allocated.
_mdns_search_result_add_ip	mdns_priv_query_result_add_ip	The function 'result_add_ip' is called without the 'mdns_priv_' prefix, which may indicate either a missing namespace prefix or an undefined function. This could lead to linking errors or incorrect behavior if the intended function is not available. All other helper functions were properly prefixed with 'mdns_priv_' or 'mdns_utils_', making this inconsistency potentially problematic.
_mdns_search_result_add_ptr	mdns_priv_query_result_add_ptr	The refactored function mdns_priv_query_result_add_ptr appears to have a critical bug: callers in the refactored code (mdns_receive.c lines 851-852, 922-923, 1038-1040) do not check the return value for NULL, unlike the original code which explicitly checked for NULL returns (lines 4020-4023, 4132-4135). This could lead to NULL pointer dereferences if the function fails to allocate memory or encounters other errors. The function signature still returns mdns_result_t*, suggesting it should be checked, but the calling code ignores the return value.
_mdns_search_result_add_srv	mdns_priv_query_result_add_srv
_mdns_search_result_add_txt	mdns_priv_query_result_add_txt	The refactoring splits the original unified TXT handling function into two specialized functions (query vs browse) with different signatures. The original parser call site at line 4149 now needs to determine which function to call based on context, but the additional parameters required by mdns_priv_browse_result_add_txt (instance, service, proto) may not be available in all calling contexts where the original function was used. This could lead to either incorrect function selection or missing data when calling the browse version, potentially causing incomplete TXT record processing or memory access issues.
_mdns_search_find_from	mdns_priv_query_find_from	The function has been changed from static to non-static linkage, which could expose internal implementation details and create unintended dependencies. Additionally, the utility function replacements (_mdns_get_esp_netif → mdns_priv_get_esp_netif and _str_null_or_empty → mdns_utils_str_null_or_empty) need verification that they maintain identical behavior in all edge cases, particularly for NULL inputs and network interface mapping.
_mdns_create_search_packet	create_search_packet	The refactored function uses mdns_priv_alloc_packet instead of _mdns_alloc_packet_default. If mdns_priv_alloc_packet doesn't provide the same default packet initialization (like setting default flags, TTL values, or other packet metadata), it could result in malformed mDNS search packets that don't conform to protocol specifications. Additionally, the domain constant change from MDNS_DEFAULT_DOMAIN to MDNS_UTILS_DEFAULT_DOMAIN could affect DNS query formation if the values differ, potentially causing search queries to use the wrong domain.
_mdns_search_send_pcb	mdns_priv_query_send	The refactored function uses mdsn_priv_pcb_is_inited(tcpip_if, ip_protocol) instead of the original complex condition mdns_is_netif_ready(tcpip_if, ip_protocol) && _mdns_server->interfaces[tcpip_if].pcbs[ip_protocol].state > PCB_INIT. This could introduce subtle bugs if the new function doesn't properly replicate both the network interface readiness check AND the PCB state validation. Additionally, there appears to be a potential typo in the function name "mdsn_priv_pcb_is_inited" (missing 'n' in 'mdns').
_mdns_search_send	search_send	The refactored function mdns_priv_query_send (which replaces _mdns_search_send_pcb) immediately frees the packet after dispatch with mdns_priv_free_tx_packet(packet), while the original implementation's packet lifecycle management is unclear. This could cause issues if packet transmission is asynchronous and the packet needs to remain valid until transmission completes. Additionally, the refactored version introduces early return on packet creation failure, which changes the error handling behavior compared to the original loop structure.
_mdns_tx_handle_packet	handle_packet	The refactoring splits the original function's logic across multiple functions, but introduces a potential bug in the PCB_PROBE_3 case. In the original code, when creating an announce packet fails, it breaks from the switch statement after scheduling a retry. However, in the refactored version, the variable 'p' is reassigned to 'a' (the new announce packet) in the success case, but this reassignment only affects the local copy in mdns_priv_pcb_schedule_tx_packet. The calling function handle_packet still holds the original 'p' pointer, which could lead to double-free or use-after-free issues if the packet lifecycle isn't properly managed across the function boundaries.
_mdns_tx_handle_packet	mdns_priv_pcb_schedule_tx_packet	The refactoring splits the original function's logic across multiple functions, but introduces a potential bug in the PCB_PROBE_3 case. In the original code, when creating an announce packet fails, it breaks from the switch statement after scheduling a retry. However, in the refactored version, the variable 'p' is reassigned to 'a' (the new announce packet) in the success case, but this reassignment only affects the local copy in mdns_priv_pcb_schedule_tx_packet. The calling function handle_packet still holds the original 'p' pointer, which could lead to double-free or use-after-free issues if the packet lifecycle isn't properly managed across the function boundaries.
_mdns_remap_self_service_hostname	mdns_priv_remap_self_service_hostname	The function scope change from static to non-static could potentially cause linking conflicts if this function is now accessible from other compilation units. Additionally, the global variable change from _mdns_server to s_server requires verification that s_server is properly initialized and equivalent to the original _mdns_server in all execution contexts.
_mdns_sync_browse_result_link_free	sync_browse_result_link_free	The function name changed from _mdns_sync_browse_result_link_free to sync_browse_result_link_free (removing the leading underscore and "mdns_" prefix). While the core logic remains identical, this naming change could introduce subtle bugs if: 1. Call sites in the refactored codebase haven't been updated to use the new function name 2. The function visibility/scope changed due to the naming convention alteration 3. Other parts of the codebase rely on the specific naming pattern for internal functions The original function was called in ACTION_BROWSE_SYNC cases in mdns.c, and if those call sites weren't properly updated to reference the renamed function, it would result in compilation errors or runtime failures due to undefined function references.
_mdns_free_action	free_action	The refactored free_action function delegates cleanup to external functions (mdns_priv_*_action with ACTION_CLEANUP mode) without visible implementation. This introduces a risk that the specific memory cleanup operations performed in the original function (mdns_mem_free for hostnames, _mdns_search_free for searches, free_address_list for address lists) may not be properly handled, potentially leading to memory leaks. The architectural change from direct cleanup to delegation requires verification that all cleanup functions properly handle their respective resource types.
_mdns_execute_action	execute_action	The refactoring introduces several subtle bugs: 1. Missing ACTION_SYSTEM_EVENT handling: In Result 2 (mdns_engine.c), the ACTION_SYSTEM_EVENT case is completely empty, while the original function called perform_event_action(). This will break system event processing. 2. Missing semaphore operations: The original ACTION_HOSTNAME_SET case called xSemaphoreGive(_mdns_server->action_sema) after completion, but the refactored version delegates to mdns_priv_responder_action without preserving this critical synchronization. 3. Missing memory cleanup in ACTION_BROWSE_SYNC: The original function explicitly called _mdns_sync_browse_result_link_free() for ACTION_BROWSE_SYNC, but the refactored version delegates this to mdns_priv_browse_action without clear evidence this cleanup is preserved. 4. Split responsibility confusion: The introduction of both execute_action and free_action functions suggests the original single responsibility has been split, but it's unclear when each should be called. This could lead to double-free or memory leaks if not properly coordinated. 5. Inconsistent implementations: Having two different execute_action functions (Result 1 vs Result 2) with different behavior for ACTION_SYSTEM_EVENT suggests either incomplete refactoring or platform-specific variations that need verification.
_mdns_execute_action	free_action	The refactoring introduces several subtle bugs: 1. Missing ACTION_SYSTEM_EVENT handling: In Result 2 (mdns_engine.c), the ACTION_SYSTEM_EVENT case is completely empty, while the original function called perform_event_action(). This will break system event processing. 2. Missing semaphore operations: The original ACTION_HOSTNAME_SET case called xSemaphoreGive(_mdns_server->action_sema) after completion, but the refactored version delegates to mdns_priv_responder_action without preserving this critical synchronization. 3. Missing memory cleanup in ACTION_BROWSE_SYNC: The original function explicitly called _mdns_sync_browse_result_link_free() for ACTION_BROWSE_SYNC, but the refactored version delegates this to mdns_priv_browse_action without clear evidence this cleanup is preserved. 4. Split responsibility confusion: The introduction of both execute_action and free_action functions suggests the original single responsibility has been split, but it's unclear when each should be called. This could lead to double-free or memory leaks if not properly coordinated. 5. Inconsistent implementations: Having two different execute_action functions (Result 1 vs Result 2) with different behavior for ACTION_SYSTEM_EVENT suggests either incomplete refactoring or platform-specific variations that need verification.
_mdns_send_search_action	send_search_action	The refactored function replaces direct queue access xQueueSend(_mdns_server->action_queue, &action, (TickType_t)0) with a wrapper function mdns_priv_queue_action(action). Without seeing the implementation of mdns_priv_queue_action, we cannot verify if it maintains the same non-blocking behavior (0 timeout) and return value semantics. If mdns_priv_queue_action uses blocking behavior or has different error handling, it could introduce timing issues or resource management problems in the MDNS search functionality.
_mdns_scheduler_run	mdns_priv_send_packets	The refactored function uses mdns_priv_queue_action(action) instead of directly calling xQueueSend. While this appears to be a wrapper function, there's a potential subtle bug: if mdns_priv_queue_action has different timeout behavior than the original (TickType_t)0 (non-blocking), it could introduce blocking behavior that wasn't present in the original implementation. Additionally, the error handling logic assumes mdns_priv_queue_action returns false on failure (matching the original xQueueSend != pdPASS), but this needs verification to ensure the same cleanup behavior is maintained.
_mdns_search_run	mdns_priv_query_start_stop	The refactored function mdns_priv_query_start_stop introduces a subtle bug in error handling. In the original code, when _mdns_send_search_action(ACTION_SEARCH_END, s) fails, the search state is restored to SEARCH_RUNNING, allowing retry in the next iteration. However, in the refactored version, when send_search_action(ACTION_SEARCH_END, s) fails, the state is also restored to SEARCH_RUNNING, but the timeout condition (now > (s->started_at + s->timeout)) will remain true in subsequent iterations, causing the search to immediately try to end again in an infinite loop until the send succeeds. This could lead to excessive retries and potential resource exhaustion.
_mdns_service_task	service_task	The refactored function replaces the direct pointer check _mdns_server && _mdns_server->action_queue with a function call mdns_priv_is_server_init() and a global variable s_action_queue. This introduces potential issues: 1. The function call mdns_priv_is_server_init() may have different timing/side effects compared to the simple pointer check 2. The global s_action_queue is not protected by the same locking mechanism as the original server structure 3. There's a potential race condition if s_action_queue is modified while the service task is running 4. The separation of server initialization check from queue access could lead to inconsistent state detection
_mdns_timer_cb	timer_cb	The refactoring changes the core functionality of the timer callback from scheduler/search operations to packet sending/query operations. While the structural mapping is clear (timer_cb replaces _mdns_timer_cb and start_timer replaces _mdns_start_timer), the functional equivalence cannot be verified without understanding what mdns_priv_send_packets and mdns_priv_query_start_stop do compared to the original _mdns_scheduler_run and _mdns_search_run. This could introduce subtle bugs in MDNS protocol timing, packet scheduling, or service discovery behavior.
_mdns_timer_cb	start_timer	The refactoring changes the core functionality of the timer callback from scheduler/search operations to packet sending/query operations. While the structural mapping is clear (timer_cb replaces _mdns_timer_cb and start_timer replaces _mdns_start_timer), the functional equivalence cannot be verified without understanding what mdns_priv_send_packets and mdns_priv_query_start_stop do compared to the original _mdns_scheduler_run and _mdns_search_run. This could introduce subtle bugs in MDNS protocol timing, packet scheduling, or service discovery behavior.
_mdns_start_timer	start_timer	The refactored start_timer function uses a global variable s_timer_handle instead of the original _mdns_server->timer_handle, but there's no evidence that s_timer_handle is properly initialized to NULL. This could lead to double-creation of timers if the function is called multiple times, as the original code would have stored the handle in a server structure that gets reinitialized. Additionally, the callback function was renamed from _mdns_timer_cb to timer_cb without verification that the implementation remains equivalent.
_mdns_stop_timer	stop_timer
_mdns_task_create_with_caps	create_task_with_caps	The refactored function introduces a new error handling path with 'alloc_failed' label that calls HOOK_MALLOC_FAILED before proceeding to the 'err' label. This changes the control flow and could potentially cause issues if HOOK_MALLOC_FAILED has side effects or if the order of cleanup operations matters. In the original function, memory allocation failure went directly to the cleanup code, but now there's an intermediate step that might interfere with proper resource management.
_mdns_service_task_start	service_task_start
_mdns_service_task_stop	service_task_stop	The refactored function maintains the same potential resource management issue as the original: if xQueueSend fails, the semaphore gets deleted immediately while the task might still be running and accessing it. However, there's an additional subtle change - the refactored version uses different timer functions (esp_timer_stop/delete vs original _mdns_stop_timer) which could have different error handling behavior. The original _mdns_stop_timer might have handled timer state differently than the new stop_timer function.
mdns_post_custom_action_tcpip_if	post_custom_action	The refactored function inverts the queue operation logic: original uses xQueueSend(...) != pdPASS for failure case, while refactored uses !mdns_priv_queue_action(action) for failure case. This suggests mdns_priv_queue_action returns true on success, but without seeing its implementation, we cannot verify if the queue behavior is equivalent. Additionally, the server initialization check changed from direct !_mdns_server to !mdns_priv_is_server_init() which may have different semantics.
set_default_duplicated_interfaces	set_default_duplicated_interfaces	The function appears to be identical in both implementations (100% similarity), but there's a potential architectural concern. The refactored codebase contains mdns_priv_pcb_set_duplicate which handles PCB-level duplication with additional operations (clearing TX queues, deinitializing PCBs, setting PCB_DUP state), while the original netif-level duplication logic remains unchanged. This creates a potential inconsistency where netif-level duplication is established but PCB-level duplication handling may not be properly synchronized. The bidirectional relationship between interfaces is managed differently in these two approaches, which could lead to race conditions or inconsistent state between netif and PCB layers.
unregister_predefined_handlers	mdns_priv_netif_unregister_predefined_handlers	The refactored function uses a different event handler function name: 'handle_system_event_for_preset' instead of 'mdns_preset_if_handle_system_event'. While the conditional compilation logic is identical, this change could introduce subtle bugs if: 1. The new handler function has different behavior or implementation 2. The new handler function has a different signature that's not compatible with the event system 3. The original handler function 'mdns_preset_if_handle_system_event' is still registered elsewhere and not properly cleaned up 4. The new handler function doesn't exist or has different error handling The search shows 'mdns_preset_if_handle_system_event' exists in the original codebase with a specific implementation, but no matches were found for 'handle_system_event_for_preset' in the refactored codebase, suggesting it might be missing or renamed differently.
mdns_netif_action	mdns_netif_action	The refactored post_custom_action function returns ESP_OK even when mdns_priv_queue_action fails (returns false), which occurs when memory allocation for the action fails. This masks memory allocation failures and changes the error behavior from the original implementation. The original mdns_post_custom_action_tcpip_if likely returned ESP_ERR_NO_MEM in this scenario, but the refactored version returns success, potentially hiding critical resource constraints.
mdns_register_netif	mdns_register_netif	The refactored mdns_unregister_netif function in Result 2 has a critical bug where it calls mdns_priv_service_lock() twice at the end instead of unlocking. This will cause a deadlock situation where the service lock is never released, blocking all subsequent mDNS operations that require the lock. The correct pattern should be mdns_priv_service_unlock() before returning.
mdns_unregister_netif	mdns_unregister_netif	The refactored mdns_unregister_netif function has a critical bug where it calls mdns_priv_service_lock() twice instead of calling mdns_priv_service_unlock() to release the lock. This will cause a deadlock situation as the service lock is never released, preventing other threads from accessing the shared resource and potentially hanging the system.
mdns_init	mdns_init	The refactored mdns_init function has a subtle bug in error handling: when mdns_priv_netif_init() fails, the code jumps to free_queue label which only deletes the action queue, but does not delete the semaphore created in mdns_priv_responder_init(). In the original code, both the queue and semaphore were properly cleaned up in the error path. This could lead to a resource leak of the semaphore.
mdns_free	mdns_free	The refactored mdns_free function introduces a subtle bug in the cleanup sequence. In the original code, free_delegated_hostnames() was called before mdns_mem_free((char *)_mdns_server->hostname) and mdns_mem_free((char *)_mdns_server->instance). However, in the refactored version, free_delegated_hostnames() is called inside mdns_priv_responder_free() AFTER the hostname and instance have already been freed. This could lead to accessing freed memory if free_delegated_hostnames() references the server's hostname or instance fields.
mdns_hostname_set	mdns_hostname_set	The refactored function replaces direct xQueueSend with mdns_priv_queue_action, but there's a potential synchronization bug. The original code used xQueueSend with 0 timeout (non-blocking) and returned ESP_ERR_NO_MEM on queue full. The new mdns_priv_queue_action function's implementation is unknown - if it uses a different timeout or blocking behavior, it could cause deadlocks or different memory error handling. Additionally, the semaphore take operation (xSemaphoreTake) remains but the queue mechanism changed, which could lead to inconsistent state if the new queue function doesn't properly coordinate with the semaphore signaling.
mdns_hostname_get	mdns_hostname_get
mdns_delegate_hostname_add	mdns_delegate_hostname_add	The refactored function uses mdns_priv_queue_action() instead of xQueueSend() with (TickType_t)0 timeout. If mdns_priv_queue_action() has different queue semantics (e.g., different timeout behavior or error handling), it could introduce subtle bugs in memory management or action queuing reliability. Additionally, the replacement of copy_address_list() with mdns_utils_copy_address_list() requires verification that the memory allocation and cleanup behavior remains identical, especially in error scenarios.
mdns_delegate_hostname_remove	mdns_delegate_hostname_remove	The refactored function shows inconsistent synchronization behavior compared to similar functions in the same codebase. While mdns_hostname_set and mdns_delegate_hostname_add both call xSemaphoreTake(s_server->action_sema, portMAX_DELAY) after queueing their actions, mdns_delegate_hostname_remove does not. This inconsistency could lead to race conditions where callers of mdns_delegate_hostname_remove assume the operation is complete when it may still be pending in the action queue. The original codebase also lacked this synchronization, but the refactoring introduced this pattern inconsistently across similar functions.
mdns_delegate_hostname_set_address	mdns_delegate_hostname_set_address	The refactored function removed the semaphore synchronization (xSemaphoreTake) that was present in similar functions (mdns_delegate_hostname_add and mdns_hostname_set), while the queue mechanism changed from direct xQueueSend to mdns_priv_queue_action. This creates inconsistent synchronization behavior across similar API functions and could lead to race conditions if the action handler expects synchronous completion. The original implementation used immediate queue sending (timeout 0) without waiting, but the refactored similar functions now wait for semaphore, suggesting this function may have missed a required synchronization update.
mdns_hostname_exists	mdns_hostname_exists
mdns_instance_name_set	mdns_instance_name_set	The refactored function replaces direct xQueueSend with mdns_priv_queue_action, but the timeout behavior may have changed. The original used (TickType_t)0 timeout (non-blocking), while the helper function's timeout semantics are unclear. Additionally, the refactored version lacks the explicit semaphore synchronization present in mdns_hostname_set (xSemaphoreTake), potentially creating inconsistent behavior between similar operations.
mdns_service_add_for_host	mdns_service_add_for_host	The refactored function has a critical bug in the linked list insertion logic. The original code correctly sets up the linked list by assigning the new item to the head of the services list. However, the refactored code has a logical error: it sets item->next = NULL and then immediately overwrites it with item->next = s_server->services, which is correct, but then assigns s_server->services = item. This appears correct at first glance, but there's a subtle issue - the original code properly maintains the linked list structure while the refactored version might have different behavior if the helper functions (like mdns_utils_get_service_item_instance and create_service) have different implementations than their original counterparts. Additionally, the memory cleanup function changed from _mdns_free_service to free_service - if these have different implementations, it could lead to memory leaks or corruption in error paths.
mdns_service_add	mdns_service_add	The refactored mdns_service_add function appears functionally equivalent to the original, but there are potential issues in its dependency chain. The function itself only changed the global variable name from _mdns_server to s_server, but the mdns_service_add_for_host function it calls has undergone significant refactoring with new locking mechanisms, service limit checks, and different error handling patterns. This could introduce subtle bugs like: 1. Deadlocks if the new mdns_priv_service_lock/unlock functions are not properly balanced 2. Different service validation behavior through mdns_utils_get_service_item_instance 3. Changed memory allocation patterns with create_service and HOOK_MALLOC_FAILED 4. Potential race conditions if the locking scope differs from the original implementation
mdns_service_exists	mdns_service_exists
mdns_service_exists_with_instance	mdns_service_exists_with_instance
_copy_mdns_txt_items	copy_txt_items	The error handling cleanup loop condition y < ret_index + 1 in both original and refactored code appears to have a potential off-by-one error. When ret_index represents the number of successfully allocated items, accessing ret[ret_index] (which would be the ret_index + 1-th element) could read/write beyond the allocated memory bounds if an allocation failure occurs early in the second loop. The condition should likely be y < ret_index to only clean up the items that were actually allocated.
_copy_delegated_host_address_list	copy_delegated_host_address_list
_mdns_lookup_service	lookup_service
mdns_service_port_set_for_host	mdns_service_port_set_for_host
mdns_service_port_set	mdns_service_port_set
mdns_service_txt_set_for_host	mdns_service_txt_set_for_host	The refactored function has the same subtle bug as the original: when memory allocation fails (allocate_txt returns NULL), the function returns ESP_ERR_NO_MEM directly without releasing the service lock (mdns_priv_service_unlock). This creates a potential deadlock scenario where the service lock remains held indefinitely after a memory allocation failure.
mdns_service_txt_set	mdns_service_txt_set	The refactored function uses s_server instead of _mdns_server for the server state check. While both appear to serve the same purpose (global MDNS server state), the refactored code introduces a potential NULL pointer dereference in mdns_priv_responder_free() at line 64 where it accesses s_server->action_sema without first checking if s_server is NULL, unlike the original code which consistently checked _mdns_server before accessing its members.
mdns_service_txt_item_set_for_host_with_explicit_value_len	mdns_service_txt_item_set_for_host_with_explicit_value_len	The refactored function uses renamed helper functions (mdns_utils_str_null_or_empty, mdns_utils_get_service_item_instance, announce_all_pcbs, mdns_priv_service_lock/unlock) and a renamed global variable (s_server). While the core logic appears identical, there's a risk that these renamed functions may have different implementations or behaviors compared to their original counterparts. Specifically, the memory allocation error handling in the out_of_mem label still uses HOOK_MALLOC_FAILED and mdns_mem_free, but if the renamed locking functions don't properly handle error states or if the global s_server initialization differs from _mdns_server, it could lead to subtle bugs like memory leaks, race conditions, or incorrect service discovery behavior.
mdns_service_txt_item_set_for_host	mdns_service_txt_item_set_for_host	The refactored function appears identical to the original, but there are two related functions (mdns_service_txt_item_set and mdns_service_txt_item_set_with_explicit_value_len) that now include state checking for s_server, while the original mdns_service_txt_item_set_for_host does not. This creates an inconsistency where some TXT item setting functions check server state while others don't. Additionally, the global variable has been renamed from _mdns_server to s_server, which could indicate broader changes in the codebase that might affect error handling consistency.
mdns_service_txt_item_set	mdns_service_txt_item_set
mdns_service_txt_item_set_with_explicit_value_len	mdns_service_txt_item_set_with_explicit_value_len
mdns_service_txt_item_remove_for_host	mdns_service_txt_item_remove_for_host	The refactored function calls announce_all_pcbs(&s, 1, false) but there's no evidence that announce_all_pcbs exists in the refactored codebase. The search for "announce_all_pcbs" returned no matches, suggesting this function may be missing or renamed differently than expected. This could lead to a compilation error or runtime failure when trying to announce the TXT item removal.
mdns_service_txt_item_remove	mdns_service_txt_item_remove	The refactored mdns_service_txt_item_remove_for_host function has several potential issues: 1. The complex ESP_GOTO_ON_FALSE conditional combines multiple error checks (server state, service/proto/key validation) into a single error path, which may mask specific error conditions that were handled separately in the original implementation 2. The memory management uses mdns_mem_free() instead of the original free() calls, which could indicate inconsistent memory management strategy across the codebase 3. The announce_all_pcbs(&s, 1, false) call appears to be new functionality that wasn't present in the original TXT removal logic, potentially causing unnecessary network announcements 4. The error handling with HOOK_MALLOC_FAILED at the end seems misplaced since no memory allocation occurs in the main logic path of this function 5. The function returns ESP_OK even when no TXT item was found to remove (the function completes without setting an error when the key isn't found)
_mdns_service_subtype_remove_for_host	service_subtype_remove_for_host	The refactored mdns_service_subtype_remove_for_host function has a potential memory leak in the error path. When memory allocation fails at remove_subtypes->subtype = mdns_mem_strdup(subtype), the code jumps to out_of_mem label which frees remove_subtypes but does not free the already allocated remove_subtypes structure itself. This creates a scenario where remove_subtypes is allocated but its subtype field allocation fails, leaving the remove_subtypes structure leaked. The original function had no such memory allocation, so this is a new bug introduced during refactoring.
_mdns_service_subtype_remove_for_host	mdns_service_subtype_remove_for_host	The refactored mdns_service_subtype_remove_for_host function has a potential memory leak in the error path. When memory allocation fails at remove_subtypes->subtype = mdns_mem_strdup(subtype), the code jumps to out_of_mem label which frees remove_subtypes but does not free the already allocated remove_subtypes structure itself. This creates a scenario where remove_subtypes is allocated but its subtype field allocation fails, leaving the remove_subtypes structure leaked. The original function had no such memory allocation, so this is a new bug introduced during refactoring.
mdns_service_subtype_remove_for_host	mdns_service_subtype_remove_for_host	The refactored function uses free_subtype(remove_subtypes) instead of _mdns_free_subtype(remove_subtypes). If free_subtype doesn't properly handle the case where remove_subtypes->subtype is NULL (due to failed mdns_mem_strdup), this could lead to memory leaks or undefined behavior. Additionally, the refactored version calls free_subtype even when mdns_priv_send_bye_subtype might have taken ownership of the subtypes, potentially causing double-free issues if the ownership semantics changed.
_mdns_service_subtype_add_for_host	service_subtype_add_for_host	The refactored function 'mdns_service_subtype_add_multiple_items_for_host' has inconsistent error handling compared to the original. In the original code, when _mdns_service_subtype_add_for_host returns ESP_ERR_INVALID_ARG (subtype already exists), the calling code continues processing other subtypes. However, in the refactored version, any error other than ESP_OK or ESP_ERR_NO_MEM causes immediate termination with goto exit, preventing processing of remaining subtypes and potentially leaving the service in an inconsistent state. Additionally, the memory cleanup rollback only occurs for ESP_ERR_NO_MEM cases, not for other error types.
mdns_service_subtype_add_multiple_items_for_host	mdns_service_subtype_add_multiple_items_for_host	The refactored function in Result 1 appears to be the direct replacement, but there are several subtle concerns: 1. Missing validation for num_items > 0: In the original code, the ESP_GOTO_ON_FALSE check includes (num_items > 0), but in Result 1, this validation appears to be missing from the condition, which could allow calling the function with num_items=0 and bypass proper error handling. 2. Function signature inconsistency: The refactored function uses parameter name 'service' while the original uses 'service_type' in the header declaration. This could cause compilation issues if the header wasn't updated consistently. 3. Resource management in error path: The cleanup loop in the error path uses service_subtype_remove_for_host which may have different memory management behavior than the original _mdns_service_subtype_remove_for_host. If the memory allocation strategy changed (as seen in Result 3), the removal function might not properly handle the new allocation pattern. 4. Potential double-free in update function: Result 2 shows free_subtype(goodbye_subtype) followed by free_service_subtype(s->service) - if these functions have overlapping cleanup responsibilities, this could lead to double-free issues.
mdns_service_subtype_add_for_host	mdns_service_subtype_add_for_host	The refactoring introduced a subtle bug in the error handling logic. In the original implementation, when _mdns_service_subtype_add_for_host returns ESP_ERR_NO_MEM, the calling function mdns_service_subtype_add_multiple_items_for_host would continue processing other subtypes but eventually roll back all successful additions. However, in the refactored version, service_subtype_add_for_host immediately returns ESP_ERR_NO_MEM without proper cleanup of the allocated subtype_item, causing a memory leak. Additionally, the refactored version uses different error labels (err vs out_of_mem) and the memory cleanup path in service_subtype_add_for_host returns the same error code regardless of which allocation failed, losing error specificity.
mdns_service_subtype_add_for_host	service_subtype_add_for_host	The refactoring introduced a subtle bug in the error handling logic. In the original implementation, when _mdns_service_subtype_add_for_host returns ESP_ERR_NO_MEM, the calling function mdns_service_subtype_add_multiple_items_for_host would continue processing other subtypes but eventually roll back all successful additions. However, in the refactored version, service_subtype_add_for_host immediately returns ESP_ERR_NO_MEM without proper cleanup of the allocated subtype_item, causing a memory leak. Additionally, the refactored version uses different error labels (err vs out_of_mem) and the memory cleanup path in service_subtype_add_for_host returns the same error code regardless of which allocation failed, losing error specificity.
mdns_service_subtype_add_for_host	mdns_service_subtype_add_multiple_items_for_host	The refactoring introduced a subtle bug in the error handling logic. In the original implementation, when _mdns_service_subtype_add_for_host returns ESP_ERR_NO_MEM, the calling function mdns_service_subtype_add_multiple_items_for_host would continue processing other subtypes but eventually roll back all successful additions. However, in the refactored version, service_subtype_add_for_host immediately returns ESP_ERR_NO_MEM without proper cleanup of the allocated subtype_item, causing a memory leak. Additionally, the refactored version uses different error labels (err vs out_of_mem) and the memory cleanup path in service_subtype_add_for_host returns the same error code regardless of which allocation failed, losing error specificity.
_mdns_service_find_subtype_needed_sendbye	service_find_subtype_needed_sendbye
mdns_service_subtype_update_multiple_items_for_host	mdns_service_subtype_update_multiple_items_for_host	The refactored function calls free_service_subtype(s->service) which completely clears all subtypes from the service before adding new ones in the loop. This differs from the original behavior where _mdns_free_service_subtype might have been more selective. This could cause temporary service unavailability and potential race conditions where subtypes are briefly unavailable between the free and add operations. Additionally, the refactored version uses different locking functions (mdns_priv_service_lock/unlock vs MDNS_SERVICE_LOCK/UNLOCK) which may have different thread safety guarantees.
mdns_service_instance_name_set_for_host	mdns_service_instance_name_set_for_host
mdns_service_instance_name_set	mdns_service_instance_name_set	The refactored function in Result 1 appears to be functionally equivalent to the original, but there's a subtle variable name change from _mdns_server to s_server. While this is likely just a naming convention change, it could indicate broader architectural changes that might affect other parts of the codebase if the global server variable semantics have changed. The function maintains the same parameter signature and delegation pattern to mdns_service_instance_name_set_for_host. However, the presence of mdns_instance_name_set (Result 2) is concerning as it suggests a parallel API with different semantics - it only takes an instance parameter and implements the functionality directly rather than delegating. This could lead to confusion about which function to use and potential inconsistencies in service instance management.
mdns_service_remove_for_host	mdns_service_remove_for_host	The refactored function calls free_service(a->service) and mdns_mem_free(a) after removing the service from the linked list, but the original function used _mdns_free_service(a->service). Without seeing the implementation of free_service vs _mdns_free_service, there's a risk that the refactored version may have different memory management behavior, potentially leading to memory leaks or double-free issues if the implementations differ.
mdns_service_remove	mdns_service_remove
mdns_service_remove_all	mdns_service_remove_all
mdns_query_results_free	mdns_query_results_free
_mdns_query_results_free	mdns_priv_query_results_free	The refactoring changes the locking mechanism from MDNS_SERVICE_LOCK/UNLOCK to mdns_priv_service_lock/unlock without clear evidence of functional equivalence. This could introduce subtle synchronization issues, deadlock risks, or race conditions if the new locking functions have different semantics, scope, or implementation than the original ones. The change from static to public function scope (mdns_priv_query_results_free) also increases the risk of improper direct usage bypassing necessary locking.
_mdns_query_results_free	mdns_query_results_free	The refactoring changes the locking mechanism from MDNS_SERVICE_LOCK/UNLOCK to mdns_priv_service_lock/unlock without clear evidence of functional equivalence. This could introduce subtle synchronization issues, deadlock risks, or race conditions if the new locking functions have different semantics, scope, or implementation than the original ones. The change from static to public function scope (mdns_priv_query_results_free) also increases the risk of improper direct usage bypassing necessary locking.
mdns_query_async_delete	mdns_query_async_delete
mdns_query_async_get_results	mdns_query_async_get_results	The function implementation remains identical, but the relocation from mdns.c to mdns_querier.c introduces potential architectural risks. The main concerns are: 1) Header file dependencies - mdns.h must properly declare the function and include necessary types from the new module, 2) Structure definition consistency - mdns_search_once_t must have identical layout in both compilation units, particularly the done_semaphore and result fields, 3) Build system integration - mdns_querier.c must be properly compiled and linked, and 4) Cross-module access - other functions in mdns.c that previously accessed this function directly may now require proper inter-module function calls.
mdns_query_async_new	mdns_query_async_new	The refactored search_init function now uses uint8_t for max_results parameter instead of size_t, which could cause truncation issues if max_results values exceed 255. This is a potential data loss bug since the original function accepted size_t and could handle larger values. Additionally, the search_init function creates a semaphore that needs verification of proper cleanup in all error paths through search_free.
mdns_query_generic	mdns_query_generic	The refactored function uses renamed helper functions (mdns_priv_is_server_init, mdns_utils_str_null_or_empty, search_init, send_search_action, search_free) without clear verification that these maintain the exact same behavior and error handling semantics as their original counterparts. Specifically: 1. The condition mdns_utils_str_null_or_empty(service) != mdns_utils_str_null_or_empty(proto) relies on the renamed string function returning consistent boolean values 2. The search initialization and action functions may have different memory management or error propagation behaviors 3. The semaphore wait on search->done_semaphore assumes the internal search structure and synchronization mechanism remains unchanged 4. Without seeing the implementations of these renamed functions, we cannot verify that resource cleanup (search_free) properly handles the results pointer assignment that occurs before freeing These renamed functions could introduce subtle behavioral changes in error cases, memory management, or protocol compliance.
mdns_query	mdns_query	The refactored mdns_query function appears identical to the original, but its dependency on mdns_query_generic has been significantly changed, introducing potential subtle bugs: 1. Timeout handling regression: The refactored mdns_query_generic uses portMAX_DELAY instead of the original timeout parameter, making queries potentially block indefinitely rather than respecting the user-specified timeout. 2. Error code inconsistency: The refactored version returns ESP_ERR_NO_MEM when send_search_action fails, but this function could fail for other reasons (network errors, internal state issues) that should return different error codes. 3. Parameter validation change: The validation logic for service/proto parameters has been altered from the original implementation, potentially allowing invalid parameter combinations that were previously rejected. 4. Function dependency changes: The refactored code uses search_init, send_search_action, and search_free instead of the original _mdns_search_init and internal mechanisms, raising concerns about functional equivalence and proper resource management. While the mdns_query wrapper function itself is unchanged, these underlying changes in mdns_query_generic could lead to unexpected blocking behavior, incorrect error reporting, and different parameter validation semantics.
mdns_query_ptr	mdns_query_ptr
mdns_query_srv	mdns_query_srv
mdns_query_txt	mdns_query_txt
mdns_lookup_delegated_service	mdns_lookup_delegated_service	The refactored lookup_service function in mdns_console.c (Result 2) creates a circular dependency: it calls mdns_lookup_delegated_service/mdns_lookup_selfhosted_service, which in turn call the lookup_service function in mdns_responder.c. This appears to be a naming collision where two different functions with the same name exist in different compilation units, which could lead to linker errors or incorrect function resolution depending on the build configuration.
mdns_lookup_selfhosted_service	mdns_lookup_selfhosted_service	The refactored code introduces a potential circular dependency. The function lookup_service in mdns_console.c calls mdns_lookup_selfhosted_service, which in turn calls lookup_service (the internal implementation). This creates infinite recursion. The original code called _mdns_lookup_service directly without this circular call pattern. Additionally, the parameter order in the lookup_service wrapper function appears inconsistent - it takes a delegated parameter but the actual implementation functions don't have this parameter in their signatures.
mdns_query_a	mdns_query_a	The refactored function changes the validation function from _str_null_or_empty to mdns_utils_str_null_or_empty. While this appears to be a simple renaming for better organization, there's a risk that the new function might have different behavior (e.g., different empty string definition, different return values, or different error handling) that could affect the input validation logic. The original codebase search didn't find references to mdns_utils_str_null_or_empty, suggesting this might be a newly introduced utility function whose implementation should be verified for functional equivalence.
mdns_query_aaaa	mdns_query_aaaa
mdns_debug_packet	dbg_packet	The refactored function changes the timestamp type from uint32_t to uint64_t but uses the same format string logic. In embedded systems with 32-bit architectures, this could cause issues with printf formatting and stack alignment. Additionally, the static mdns_name_t variable could cause thread-safety issues in multi-threaded environments, and the new mdns_dbg_flush() call at the end might introduce blocking behavior that wasn't present in the original function.
_mdns_sync_browse_action	mdns_priv_browse_sync	The refactored function mdns_priv_browse_sync hardcodes the action type to ACTION_BROWSE_SYNC, removing the type parameter from the original function. This could be problematic if the original function was ever called with action types other than ACTION_BROWSE_SYNC. While the search shows only ACTION_BROWSE_SYNC usage in the original codebase, this change reduces flexibility and could break future extensions that might use different browse sync action types.
_mdns_send_browse_action	send_browse_action	The refactored function uses !mdns_priv_queue_action(action) instead of the original xQueueSend(_mdns_server->action_queue, &action, (TickType_t)0) != pdPASS. This boolean logic inversion suggests that mdns_priv_queue_action returns true for success and false for failure, while xQueueSend returns pdPASS (typically 1) for success and other values for failure. If mdns_priv_queue_action doesn't properly replicate the exact queueing semantics (including the zero timeout behavior), this could introduce subtle behavioral changes in the action queueing mechanism, potentially affecting the MDNS browsing functionality under high load conditions.
_mdns_browse_item_free	browse_item_free
_mdns_browse_init	browse_init	The refactored browse_init function has inconsistent error handling for memory allocation failures. When browse->service allocation fails, HOOK_MALLOC_FAILED is called twice (once for browse malloc and once for service strndup), but when browse->proto allocation fails, HOOK_MALLOC_FAILED is only called once (for browse malloc). This inconsistency could lead to incorrect memory failure tracking or logging. The original function had consistent behavior where HOOK_MALLOC_FAILED was only called for the initial browse allocation failure.
mdns_browse_new	mdns_browse_new	The refactored mdns_browse_new function has a critical logic error in the server initialization check. The original code returned NULL when !_mdns_server (server not initialized), but the refactored code returns NULL when mdns_priv_is_server_init() (server IS initialized). This logical inversion would prevent browsing from working when the MDNS server is properly initialized, making the function always return NULL in normal operation.
mdns_browse_delete	mdns_browse_delete
_mdns_browse_finish	browse_finish	The refactored function uses a global variable 's_browse' instead of accessing '_mdns_server->browse'. This structural change could introduce subtle bugs if: 1. 's_browse' is not properly synchronized with the server state in other parts of the system 2. There are concurrent access issues since global variables are more prone to race conditions 3. Other functions that previously operated on '_mdns_server->browse' might not be updated to use 's_browse' consistently 4. The initialization and lifetime management of 's_browse' might differ from the server-based approach
_mdns_browse_add	browse_add	The refactored function uses a global variable 's_browse' instead of accessing it through '_mdns_server->browse'. This structural change could introduce subtle bugs if: 1. 's_browse' is not properly initialized or synchronized with the original server state 2. Other parts of the system still expect to access browse data through '_mdns_server->browse' 3. There are thread safety issues since global 's_browse' might not have the same protection as the original server structure 4. The browse queue management is now split between different data structures, potentially causing inconsistencies
_mdns_browse_send	browse_send	The refactored function replaces _mdns_search_send_pcb with mdns_priv_query_send, which changes the PCB readiness check from mdns_is_netif_ready(tcpip_if, ip_protocol) && _mdns_server->interfaces[tcpip_if].pcbs[ip_protocol].state > PCB_INIT to mdsn_priv_pcb_is_inited(tcpip_if, ip_protocol). This could change when queries are sent and there's a potential typo in "mdsn_priv_pcb_is_inited" (should likely be "mdns_priv_pcb_is_inited"). Additionally, the packet management in mdns_priv_query_send (create, dispatch, immediate free) may have different error handling characteristics than the original implementation.
_mdns_add_browse_result	add_browse_result	The function visibility has changed from static to potentially non-static (the refactored code snippet doesn't show the static keyword). This could lead to linkage issues or unintended external access. Additionally, all call sites in the original codebase need to be verified to ensure they now call the renamed function in the new file location (mdns_browser.c vs mdns.c).
_mdns_browse_result_add_ip	mdns_priv_browse_result_add_ip	The refactored function replaces _mdns_add_browse_result with add_browse_result without the _mdns_ prefix, which may indicate a missing namespace or different function implementation. Additionally, the TTL update logic changed from _mdns_result_update_ttl to mdns_priv_query_update_result_ttl, which could have different behavior for TTL calculations. The search results show no matches for these renamed functions in the refactored codebase, suggesting they may be missing or have different signatures/implementations.
_mdns_browse_find_from	mdns_priv_browse_find	The refactored function changed from taking a browse list parameter to using a global variable 's_browse', which could introduce thread safety issues and makes the function dependent on global state. Additionally, the original function was called with '_mdns_server->browse' as parameter, but the refactored version directly uses 's_browse' without clear evidence that these are equivalent. This change in data access pattern could lead to incorrect browse list traversal if the global variable is not properly synchronized with the original server state.
is_txt_item_in_list	is_txt_item_in_list	The refactored function appears to be identical to the original (100% similarity) but has been moved from mdns.c to mdns_browser.c. This change in location could introduce subtle issues if: 1. The function now has different visibility/scope in the new file 2. Callers in the original location (mdns.c) cannot access it if it remains static 3. The function assumes different context or dependencies in the browser component vs responder component 4. The TXT item comparison logic might need different behavior between browser and responder contexts
_mdns_browse_result_add_txt	???
_mdns_copy_address_in_previous_result	copy_address_in_previous_result
_mdns_browse_result_add_srv	mdns_priv_browse_result_add_srv	The refactored function calls two helper functions without the expected 'mdns_priv_' prefix: 'copy_address_in_previous_result' and 'add_browse_result'. This naming inconsistency suggests these functions may not be properly declared or implemented in the refactored codebase, potentially leading to linker errors or incorrect function resolution. Additionally, the final explicit 'return' statement was removed, which could affect code clarity but likely doesn't impact functionality.
_mdns_browse_sync	browse_sync
_debug_printf_result	dbg_printf_result	The refactored function dbg_printf_result has been changed from non-static to static, which may break external calls if the function was used outside its compilation unit. Additionally, the addition of mdns_dbg_flush() at the end changes the debug output behavior - it now forces immediate flushing which could impact performance in debug-heavy scenarios or embedded systems with constrained I/O resources.
debug_printf_browse_result	mdns_debug_printf_browse_result	The refactored function adds an unconditional call to mdns_dbg_flush() which was not present in the original function. This could potentially impact performance in debug mode, especially if called frequently, as it may force immediate output flushing that wasn't happening before. Additionally, the debug macro names have changed from _mdns_dbg_printf/_debug_printf_result to dbg_printf/dbg_printf_result, which could have different formatting or behavior if these macros are not functionally equivalent.
debug_printf_browse_result_all	mdns_debug_printf_browse_result_all	The refactored function adds a call to mdns_dbg_flush() at the end. While this might be intentional to ensure debug output is flushed, it could introduce timing differences in debugging scenarios or potentially block if the flush operation has significant overhead. In embedded systems or real-time environments, this could affect system behavior during debugging sessions. The original function did not have any flush operation, so output buffering behavior may have changed.