Review: Memory Pool Allocator Written by an AI Assistant

Published 10 Nov, 2026

Your networking team asked an AI coding assistant to replace malloc for packet buffers in the device firmware. It opened this pull request. You are the reviewer, and nothing merges without your approval.

The assistant's description:

Adds a fixed-size block allocator so the network stack never touches the heap. O(1) alloc and free through an intrusive free list, no per-block header, and pool_owns() guards pool_free() against foreign pointers.

Where it runs: STM32H7 (Cortex-M7) running FreeRTOS. The Ethernet receive interrupt calls pool_alloc() for each frame and queues it to the protocol task, which calls pool_free() when done. The pool is created with pool_init(&rx_pool, rx_mem, sizeof rx_mem, sizeof(struct pkt_hdr) + 1522), giving 1538-byte blocks; rx_mem is a static uint8_t array and struct pkt_hdr holds a uint64_t timestamp. The Ethernet DMA needs word-aligned buffers. Several error paths in the stack can free the same packet twice.

Your task

Flag every problem you would block the merge for. Click a line number to comment, and Shift-click to cover a range.
For each one, choose the issue type and severity, and explain what fails and how to fix it.
You are scored on the problems you catch, the right issue type and a clear explanation. Missing a critical problem fails the round.

Aim for 20 minutes, as in a real review round.

Block 1 starts at base + 1538. Is that a valid address for a struct pkt_hdr?

The receive interrupt can fire between any two lines of pool_free(). Pick the worst place.

What does pool_owns() return for base + block_count * block_size? For base + 3?

The same packet is freed twice. What do the next two pool_alloc() calls return?

A traffic burst empties the pool and then someone calls pool_calloc().

The allocator is neat, but it hands out misaligned blocks and its free list is corrupted by the very interrupt it was written for. Either is enough to block the merge.

Blocks are not aligned (lines 25–30, critical). block_size is only raised to the size of a pointer, never rounded up to an alignment, and rx_mem is a byte array with no alignment guarantee. With 1538-byte blocks every other block starts at an address that is 2 mod 8, so the uint64_t timestamp and the free-list pointer are misaligned: undefined behaviour, a fault on LDRD/STRD/LDM, and a buffer the Ethernet DMA cannot use. Round block_size up to _Alignof(max_align_t) or the DMA alignment, and align base (or reject unaligned memory).
Free list is not interrupt-safe (lines 69–72, critical). pool_free() reads free_list, links the block, then writes free_list. If the receive interrupt runs pool_alloc() in between, it takes the head block, and the task then puts it back as b->next: the block is in use and on the free list, so it is handed out twice. in_use++ and in_use-- also lose updates. Mask interrupts around both operations (taskENTER_CRITICAL() in the task, taskENTER_CRITICAL_FROM_ISR() in the ISR). A lock-free stack with compare-and-swap would also need ABA protection.
Double free is not detected (lines 67–68, major). Freeing the same block twice pushes it twice, so the next two allocations return the same buffer and two packets share memory; in_use also wraps below zero. pool_owns() checks range only. Keep a free bitmap or a magic word in debug builds and assert, or at least check that the block is not already on the list.
pool_owns accepts the end and interior pointers (line 45, major). <= accepts the one-past-the-end address, and any pointer inside a block passes too. pool_free() then links memory outside the pool, or the middle of a block, and later allocations overlap other data. Use < and check (c - base) % block_size == 0.
pool_calloc does not handle an empty pool (lines 60–61, major). When the pool is exhausted, pool_alloc() returns NULL and memset(NULL, ...) faults, exactly during the traffic burst that emptied the pool. Return NULL before clearing.

#include <stddef.h>
#include <stdint.h>
#include <string.h>
#include "pool.h"

/*
 * Fixed-size block pool for network packet buffers. No heap, O(1)
 * alloc and free. The Ethernet RX interrupt allocates; the protocol
 * task frees once a packet has been processed.
 */
typedef struct pool_block {
    struct pool_block *next;
} pool_block_t;

typedef struct {
    uint8_t      *base;
    size_t        block_size;
    size_t        block_count;
    pool_block_t *free_list;
    size_t        in_use;
} pool_t;

int pool_init(pool_t *p, void *mem, size_t mem_size, size_t block_size)
{
    if (block_size < sizeof(pool_block_t))
        block_size = sizeof(pool_block_t);

p->base = mem;
    p->block_size = block_size;
    p->block_count = mem_size / block_size;
    p->free_list = NULL;
    p->in_use = 0;

for (size_t i = 0; i < p->block_count; i++) {
        pool_block_t *b = (pool_block_t *)(p->base + i * block_size);
        b->next = p->free_list;
        p->free_list = b;
    }
    return p->block_count ? 0 : -1;
}

int pool_owns(const pool_t *p, const void *ptr)
{
    const uint8_t *c = ptr;
    return c >= p->base && c <= p->base + p->block_count * p->block_size;
}

void *pool_alloc(pool_t *p)
{
    pool_block_t *b = p->free_list;
    if (b) {
        p->free_list = b->next;
        p->in_use++;
    }
    return b;
}

void *pool_calloc(pool_t *p)
{
    void *ptr = pool_alloc(p);
    memset(ptr, 0, p->block_size);
    return ptr;
}

void pool_free(pool_t *p, void *ptr)
{
    if (!pool_owns(p, ptr))
        return;
    pool_block_t *b = ptr;
    b->next = p->free_list;
    p->free_list = b;
    p->in_use--;
}

size_t pool_available(const pool_t *p)
{
    return p->block_count - p->in_use;
}

{"issues":[
{"id":"align","file":"pool.c","lines":[25,30],"category":"bug","severity":"critical","title":"Block size and base are never aligned","explanation":"1538-byte blocks in a uint8_t array leave blocks misaligned for uint64_t and pointer fields; misaligned access is undefined behaviour, faults on multi-word loads and breaks DMA.","keywords":["align","misalign","unaligned","round up","max_align_t","alignof","hardfault","fault","dma"],"accept":["design","edge"],"fix":"Round block_size up to the required alignment and align base (or reject unaligned mem)."},
{"id":"isr","file":"pool.c","lines":[69,72],"category":"concurrency","severity":"critical","title":"Free list updated without masking the RX interrupt","explanation":"If the ISR allocates between the read and write of free_list in pool_free, the same block ends up both allocated and on the free list; in_use updates are lost.","keywords":["interrupt","isr","critical section","race","disable","atomic","reentran","primask","taskenter"],"accept":["bug"],"fix":"Wrap alloc and free in a critical section that masks the Ethernet interrupt."},
{"id":"dfree","file":"pool.c","lines":[67,68],"category":"bug","severity":"major","title":"Double free silently corrupts the free list","explanation":"Freeing a block twice links it twice, so two later allocations return the same buffer and in_use wraps below zero.","keywords":["double free","freed twice","twice","already free","same block","bitmap","magic","assert","underflow"],"accept":["security","edge"],"fix":"Track free state (bitmap or magic word) and assert or ignore a second free."},
{"id":"owns","file":"pool.c","lines":[45,45],"category":"edge","severity":"major","title":"pool_owns off by one and ignores block boundaries","explanation":"<= accepts the one-past-the-end address and interior pointers are accepted, so pool_free can link memory outside or inside a block.","keywords":["<=","off-by-one","off by one","one past","end","boundary","modulo","% block_size","interior"],"accept":["bug"],"fix":"Use < and require (c - base) % block_size == 0."},
{"id":"calloc","file":"pool.c","lines":[60,61],"category":"bug","severity":"major","title":"pool_calloc clears a NULL pointer when the pool is empty","explanation":"pool_alloc returns NULL when exhausted and memset then writes through it.","keywords":["null","empty","exhaust","memset","check","fail"],"accept":["edge"],"fix":"if (!ptr) return NULL; before memset."}
]}

Topics #011 AI Code Review C Concurrency Embedded Medium Memory Management