Embedded C skill for Codex

Done well means: every variable shared with an ISR or hardware is correctly qualified and accessed atomically, nothing allocates after initialisation, the build is warning-free at -Wall -Wextra -Wconversion, flash, RAM and stack use are measured against a budget, and the hardware-independent logic has tests that run on the host.

Inspect first

1. Read AGENTS.md, the build system (CMakeLists.txt, Makefile, IDE project) and the compiler flags actually used. -O0 debug builds hide most of the bugs below.
2. Find the linker script (*.ld) for memory regions, stack and heap size.
3. Build once and keep the .map file and arm-none-eabi-size output as the baseline.

arm-none-eabi-size -A -d build/app.elf
python3 scripts/mem_budget.py build/app.elf --flash 0x08000000:512K --ram 0x20000000:128K

scripts/mem_budget.py reads the ELF program headers directly (standard library only), so it works without binutils. It counts initialised .data against both flash and RAM, lists the largest sections, and exits 1 when usage is over --max-flash-pct (default 90) or --max-ram-pct (default 85). Take region bases and sizes from the linker script.

volatile: what it does and does not do

Situation	Needs volatile	Also needs
Memory-mapped peripheral register	Yes (vendor headers already do it)	Correct access width
Variable written in an ISR, read in main/task	Yes	Atomic access if wider than the bus or read-modify-write
Flag set by DMA completion	Yes	Cache maintenance on cores with D-cache
Variable shared between two RTOS tasks	No, it is not a synchronisation tool	A mutex, queue or atomic
Loop counter for a busy-wait delay	Technically yes	Use a hardware timer instead

volatile stops the compiler from caching or removing accesses. It does not make x++ atomic, does not order non-volatile accesses around it, and does not flush caches.

ISR-shared data

#include <stdint.h>
#include <stdbool.h>

static volatile uint32_t g_ticks;          /* written only in the ISR */

void SysTick_Handler(void) { g_ticks++; }  /* ISR is the single writer: no lock needed */

uint32_t ticks_now(void) {
    return g_ticks;                         /* 32-bit aligned read is atomic on Cortex-M */
}

When main code must read-modify-write shared state, or read a 64-bit value on a 32-bit core, use a critical section that restores the previous state (nesting-safe):

static inline uint32_t irq_save(void) {
    uint32_t primask = __get_PRIMASK();
    __disable_irq();
    return primask;
}
static inline void irq_restore(uint32_t primask) { __set_PRIMASK(primask); }

uint32_t take_events(void) {
    uint32_t s = irq_save();
    uint32_t e = g_events;                  /* g_events is volatile, set by ISRs */
    g_events = 0;
    irq_restore(s);
    return e;
}

• Keep critical sections to a few instructions; measure worst-case interrupt latency.
• Under an RTOS use its API (taskENTER_CRITICAL, irq_lock) instead of raw PRIMASK.
• On AVR and other 8-bit cores even 16-bit reads tear; wrap them.
• C11 <stdatomic.h> works on Cortex-M3 and above (LDREX/STREX); on Cortex-M0 atomics wider than a plain load or store fall back to library calls or do not link.

Single-producer single-consumer ring buffer

#define RB_SIZE 256u                          /* power of two */
_Static_assert((RB_SIZE & (RB_SIZE - 1u)) == 0u, "RB_SIZE must be a power of two");

typedef struct {
    uint8_t buf[RB_SIZE];
    volatile uint32_t head;                   /* written by producer (ISR) only */
    volatile uint32_t tail;                   /* written by consumer only */
} ringbuf_t;

bool rb_put(ringbuf_t *rb, uint8_t b) {       /* call from the ISR */
    uint32_t h = rb->head;
    if (h - rb->tail == RB_SIZE) return false; /* full; count drops */
    rb->buf[h & (RB_SIZE - 1u)] = b;
    __DMB();                                  /* data visible before index moves */
    rb->head = h + 1u;
    return true;
}

bool rb_get(ringbuf_t *rb, uint8_t *out) {    /* call from main/task */
    uint32_t t = rb->tail;
    if (rb->head == t) return false;
    *out = rb->buf[t & (RB_SIZE - 1u)];
    __DMB();
    rb->tail = t + 1u;
    return true;
}

Free-running unsigned indices make full and empty distinguishable without wasting a slot. This is only safe with exactly one producer and one consumer.

Integer and type traps

• Use <stdint.h> types for anything that touches hardware or a wire format.
• Integer promotion: uint8_t a = 200, b = 100; if (a + b > 255) is true because the sum is int. (uint8_t)(a + b) wraps. Be explicit.
• ~ on a small unsigned type promotes to int: (uint8_t)~mask when storing back.
• Shifting 1 << 31 is undefined for signed int; write 1u << 31 or UINT32_C(1) << 31.
• Do not use C bit-fields to map registers or wire formats; layout and access width are implementation-defined. Use masks and shifts.
• Do not cast a byte buffer to a struct pointer for parsing: alignment faults on M0 and strict-aliasing UB everywhere. memcpy into a local, or decode field by field with explicit endianness.
• Floating point on cores without an FPU pulls in kilobytes of soft-float; check the map file for __aeabi_d* and use f suffixes (0.5f) on FPU cores that are single-precision.

Memory discipline

• Allocate everything at init: static buffers, pools, malloc only before the scheduler starts if at all. Heap fragmentation is a field failure that takes weeks to appear.
• const tables go to flash; check the map file that they did (missing const puts them in .data, costing flash and RAM).
• Stack: compile with -fstack-usage (writes .su files) and -Wstack-usage=512 to flag large frames; paint the stack with a pattern at boot and check the watermark at runtime. Recursion and VLAs are banned in firmware for this reason.
• Read the .map file sorted by size to find what grew: printf with floats, C++ exceptions pulled in by one file, or a debug table left in.

Compiler flags baseline

-std=c11 -Wall -Wextra -Wconversion -Wshadow -Wdouble-promotion -Wundef
-ffunction-sections -fdata-sections -Wl,--gc-sections -Wl,-Map=build/app.map
-Os (or -O2) -g3 -fno-common -fstack-usage

Treat warnings as errors in CI. Debug at -Og, not -O0, so behaviour matches release.

Host-side tests

Split each driver into a hardware access layer (register reads/writes behind a small interface) and logic (parsing, state machines, filters, protocol framing). Build the logic for the host with the system compiler and test it with Unity/Ceedling, CMocka or GoogleTest, mocking the access layer. Run under -fsanitize=address,undefined on the host to catch the overflow and UB that the target silently tolerates. Fuzz parsers of external input (fuzz-testing).

Review checklist

• Every ISR-shared variable: volatile, single writer or protected, atomic width
• No blocking, logging or allocation in ISRs; ISR clears its flag early
• No unchecked return values from HAL/driver calls
• Wire formats decoded with explicit endianness and length checks
• Buffer indices bounded; sizeof used, not magic numbers
• No recursion, VLAs or malloc in steady state
• Builds warning-free with the baseline flags at the release optimisation level

Verification

Build release flags, run host tests with sanitizers, run mem_budget.py and compare with the baseline, and check .su output for the deepest call path. When a bug “only happens at -O2”, look for a missing volatile, a data race or UB before blaming the compiler.

Report

**Scope:** uart_rx.c, sensor_task.c, ringbuf.h (STM32G0, GCC 13.3, -Os)
**Findings:** 1 blocker (64-bit timestamp read torn between ISR and task), 2 major (struct cast
on unaligned buffer; 1 << 31 on int), 3 minor
**Memory:** flash 61.3 % -> 59.8 %, RAM 71.0 % -> 71.4 % (mem_budget.py)
**Tests:** 18 host tests for frame parser under ASan/UBSan, all passing