fix(lwip): Avoid dual heap allocations of semaphores by LwIP

This avoids one pointer indirection.

Author: jimmyw

components/esp_netif/lwip/esp_netif_lwip.c

@@ -207,8 +207,8 @@ static void netif_unset_garp_flag(struct netif *netif)
 #endif  // CONFIG_LWIP_GARP_TMR_INTERVAL
 
 #if !LWIP_TCPIP_CORE_LOCKING
-static sys_sem_t api_sync_sem = NULL;
-static sys_sem_t api_lock_sem = NULL;
+static sys_sem_t api_sync_sem = {0};
+static sys_sem_t api_lock_sem = {0};
 #endif
 
 /**
@@ -588,14 +588,14 @@ esp_err_t esp_netif_init(void)
     }
 
 #if !LWIP_TCPIP_CORE_LOCKING
-    if (!api_sync_sem) {
+    if (!sys_sem_valid(&api_sync_sem)) {
         if (ERR_OK != sys_sem_new(&api_sync_sem, 0)) {
             ESP_LOGE(TAG, "esp netif api sync sem init fail");
             return ESP_FAIL;
         }
     }
 
-    if (!api_lock_sem) {
+    if (!sys_sem_valid(&api_lock_sem)) {
         if (ERR_OK != sys_sem_new(&api_lock_sem, 1)) {
             ESP_LOGE(TAG, "esp netif api lock sem init fail");
             return ESP_FAIL;

components/lwip/port/freertos/include/arch/sys_arch.h

@@ -3,7 +3,7 @@
  *
  * SPDX-License-Identifier: BSD-3-Clause
  *
- * SPDX-FileContributor: 2018-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileContributor: 2018-2025 Espressif Systems (Shanghai) CO LTD
  */
 #ifndef __SYS_ARCH_H__
 #define __SYS_ARCH_H__
@@ -18,7 +18,7 @@ extern "C" {
 #endif
 
 
-typedef SemaphoreHandle_t sys_sem_t;
+typedef StaticSemaphore_t sys_sem_t;
 typedef SemaphoreHandle_t sys_mutex_t;
 typedef TaskHandle_t sys_thread_t;
 
@@ -45,9 +45,8 @@ void sys_delay_ms(uint32_t ms);
 #define sys_mbox_valid(mbox)       (*(mbox) != NULL)
 #define sys_mbox_set_invalid(mbox) (*(mbox) = NULL)
 
-#define sys_sem_valid_val(sema)   ((sema) != NULL)
-#define sys_sem_valid(sema)       (((sema) != NULL) && sys_sem_valid_val(*(sema)))
-#define sys_sem_set_invalid(sema) ((*(sema)) = NULL)
+#define sys_sem_valid(sema) ( ( memcmp((sema), &(StaticSemaphore_t){0}, sizeof(StaticSemaphore_t)) == 0) ? pdFALSE : pdTRUE )
+#define sys_sem_set_invalid(sema) memset((sema), 0, sizeof(StaticSemaphore_t))
 
 void sys_delay_ms(uint32_t ms);
 sys_sem_t* sys_thread_sem_init(void);

components/lwip/port/freertos/sys_arch.c

@@ -108,14 +108,14 @@ sys_sem_new(sys_sem_t *sem, u8_t count)
   LWIP_ASSERT("initial_count invalid (neither 0 nor 1)",
              (count == 0) || (count == 1));
 
-  *sem = xSemaphoreCreateBinary();
-  if (*sem == NULL) {
-      LWIP_DEBUGF(SYS_DEBUG, ("sys_sem_new: out of mem\r\n"));
+  QueueHandle_t res = xSemaphoreCreateBinaryStatic(sem);
+  if ((sys_sem_t *)res != sem) {
+      LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sys_sem_new: out of mem\r\n"));
       return ERR_MEM;
   }
 
   if (count == 1) {
-      BaseType_t ret = xSemaphoreGive(*sem);
+      BaseType_t ret = xSemaphoreGive(sem);
       LWIP_ASSERT("sys_sem_new: initial give failed", ret == pdTRUE);
       (void)ret;
   }
@@ -131,7 +131,7 @@ sys_sem_new(sys_sem_t *sem, u8_t count)
 void
 sys_sem_signal(sys_sem_t *sem)
 {
-  BaseType_t ret = xSemaphoreGive(*sem);
+  BaseType_t ret = xSemaphoreGive(sem);
   /* queue full is OK, this is a signal only... */
   LWIP_ASSERT("sys_sem_signal: sane return value",
              (ret == pdTRUE) || (ret == errQUEUE_FULL));
@@ -144,7 +144,7 @@ int
 sys_sem_signal_isr(sys_sem_t *sem)
 {
     BaseType_t woken = pdFALSE;
-    xSemaphoreGiveFromISR(*sem, &woken);
+    xSemaphoreGiveFromISR(sem, &woken);
     return woken == pdTRUE;
 }
 
@@ -162,7 +162,7 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)
 
   if (!timeout) {
     /* wait infinite */
-    ret = xSemaphoreTake(*sem, portMAX_DELAY);
+    ret = xSemaphoreTake((QueueHandle_t)sem, portMAX_DELAY);
     LWIP_ASSERT("taking semaphore failed", ret == pdTRUE);
   } else {
     /* Round up the number of ticks.
@@ -172,7 +172,7 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)
      * 1 tick before triggering a timeout. Thus, we need to pass 2 ticks as a timeout
      * to `xSemaphoreTake`. */
     TickType_t timeout_ticks = ((timeout + portTICK_PERIOD_MS - 1) / portTICK_PERIOD_MS) + 1;
-    ret = xSemaphoreTake(*sem, timeout_ticks);
+    ret = xSemaphoreTake((QueueHandle_t)sem, timeout_ticks);
     if (ret == errQUEUE_EMPTY) {
       /* timed out */
       return SYS_ARCH_TIMEOUT;
@@ -191,8 +191,8 @@ sys_arch_sem_wait(sys_sem_t *sem, u32_t timeout)
 void
 sys_sem_free(sys_sem_t *sem)
 {
-  vSemaphoreDelete(*sem);
-  *sem = NULL;
+  vSemaphoreDelete((QueueHandle_t)sem);
+  memset(sem, 0, sizeof(*sem));
 }
 
 /**
@@ -490,31 +490,33 @@ sys_thread_sem_free(void* data) // destructor for TLS semaphore
 {
   sys_sem_t *sem = (sys_sem_t*)(data);
 
-  if (sem && *sem){
-    LWIP_DEBUGF(SYS_DEBUG, ("sem del, sem=%p\n", *sem));
-    vSemaphoreDelete(*sem);
+  if (sem){
+    LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sem del, sem=%p\n", sem));
+    vSemaphoreDelete((QueueHandle_t)sem);
   }
 
   if (sem) {
-    LWIP_DEBUGF(SYS_DEBUG, ("sem pointer del, sem_p=%p\n", sem));
+    LWIP_DEBUGF(ESP_THREAD_SAFE_DEBUG, ("sem pointer del, sem_p=%p\n", sem));
     mem_free(sem);
   }
 }
 
 sys_sem_t*
 sys_thread_sem_init(void)
 {
-  sys_sem_t *sem = (sys_sem_t*)mem_malloc(sizeof(sys_sem_t*));
+  sys_sem_t *sem = (sys_sem_t*)mem_malloc(sizeof(sys_sem_t));
 
   if (!sem){
     ESP_LOGE(TAG, "thread_sem_init: out of memory");
     return 0;
   }
 
-  *sem = xSemaphoreCreateBinary();
-  if (!(*sem)){
+  // If successful, create a binary semaphore the pointer to the semaphore is returned
+  QueueHandle_t res = xSemaphoreCreateBinaryStatic(sem);
+
+  if (res != (QueueHandle_t)sem){
     mem_free(sem);
-    ESP_LOGE(TAG, "thread_sem_init: out of memory");
+    ESP_LOGE(TAG, "thread_sem_init: unable to create semaphore %p != %p", res, sem);
     return 0;
   }