Skip to content

Commit

Permalink
Fix incorrect preprocessor usage of LwIP PBUF_POOL (project-chip#29232)
Browse files Browse the repository at this point in the history
Configuration allowed specifying an LwIP `pbuf_type` value for
`System::PacketBuffer` allocations, and code tested this configuration
in preprocessor conditions; however, the LwIP names are enum constants,
not preprocessor defintions.

This change replaces the configuration macro with a new one selecting
between `PBUF_POOL` and `PBUF_RAM`. (From PR#14257 that introduced the
configuration, it does not appear that other options were ever used.)

Fixes project-chip#29208
  • Loading branch information
kpschoedel authored Sep 13, 2023
1 parent 9ebd18a commit ba85422
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 6 deletions.
14 changes: 10 additions & 4 deletions src/system/SystemConfig.h
Original file line number Diff line number Diff line change
Expand Up @@ -359,15 +359,21 @@
#endif /* CHIP_SYSTEM_CONFIG_PACKETBUFFER_POOL_SIZE */

/**
* @def CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE
* @def CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM
*
* @brief
* LwIP @pbuf_type for System::PacketBuffer allocations.
* In LwIP builds, this selects whether to use LwIP @pbuf_type @PBUF_RAM (1)
* or @PBUF_POOL (0) for System::PacketBuffer allocations.
*
* Note that this does not affect allocations by LwIP itself, e.g. the normal receive path.
*/
#ifndef CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE
#define CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE PBUF_POOL
#ifndef CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM
#define CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM 0
#endif /* CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM */

// Catch configurations attempting to use the former method (see issue #29208).
#ifdef CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE
#error "See CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM"
#endif /* CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE */

/**
Expand Down
2 changes: 1 addition & 1 deletion src/system/SystemPacketBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,7 @@ PacketBufferHandle PacketBufferHandle::New(size_t aAvailableSize, uint16_t aRese
#if CHIP_SYSTEM_CONFIG_USE_LWIP

lPacket = static_cast<PacketBuffer *>(
pbuf_alloc(PBUF_RAW, static_cast<uint16_t>(lAllocSize), CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE));
pbuf_alloc(PBUF_RAW, static_cast<uint16_t>(lAllocSize), CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE));

SYSTEM_STATS_UPDATE_LWIP_PBUF_COUNTS();

Expand Down
19 changes: 18 additions & 1 deletion src/system/SystemPacketBufferInternal.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,25 @@
*
* True if packet buffers are allocated from an LwIP pool (either standard or custom).
*/
#if CHIP_SYSTEM_CONFIG_USE_LWIP && (CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_TYPE == PBUF_POOL)
#if CHIP_SYSTEM_CONFIG_USE_LWIP && !CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM
#define CHIP_SYSTEM_PACKETBUFFER_FROM_LWIP_POOL 1
#else
#define CHIP_SYSTEM_PACKETBUFFER_FROM_LWIP_POOL 0
#endif

/**
* CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE
*
* LwIP @pbuf_type for System::PacketBuffer allocations.
*/
#if CHIP_SYSTEM_CONFIG_USE_LWIP
#if CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM
#define CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE PBUF_RAM
#else
#define CHIP_SYSTEM_PACKETBUFFER_LWIP_PBUF_TYPE PBUF_POOL
#endif
#endif

/**
* CHIP_SYSTEM_PACKETBUFFER_FROM_LWIP_STANDARD_POOL
*
Expand Down Expand Up @@ -114,3 +127,7 @@
CHIP_SYSTEM_PACKETBUFFER_FROM_LWIP_POOL
#error "Inconsistent PacketBuffer LwIP pool configuration"
#endif

#if (CHIP_SYSTEM_PACKETBUFFER_FROM_LWIP_POOL + CHIP_SYSTEM_CONFIG_PACKETBUFFER_LWIP_PBUF_RAM) > 1
#error "Inconsistent PacketBuffer LwIP pbuf_type configuration"
#endif

0 comments on commit ba85422

Please sign in to comment.