S11.02: Extract SolidSyslogPoolAllocator and migrate CircularBuffer onto it

[DavidCozens]

Parent epic: #29

Context

S11.01 (#392, merged via #394) landed the canonical pattern for E11 — a library-internal static pool of class instances, with SolidSyslog_LockConfig injection wrapping each slot probe and a Fallback singleton on pool exhaustion. The pilot is SolidSyslogCircularBuffer.

Looking at the final Core/Source/SolidSyslogCircularBufferStatic.c (12 functions + a struct + two static globals + a vtable pair), it's clear every other E11 class is going to copy the same shape:

• static struct X Pool[N];
• static bool InUse[N]; (currently embedded in a struct Slot wrapper alongside X)
• AcquireFirstFree() — walk pool, lock-per-iteration, claim first free slot
• AcquireIfFree(i) — single-slot lock + check + claim
• Acquire(i) — mark-in-use + return handle
• MarkInUse(i) / MarkFree(i) — flag mutations
• PoolItemIsFree(i) / PoolItemIsInUse(i) — flag reads (chained pair)
• HandleFromIndex(i) — return &Pool[i].Object.Base;
• HandleIsValid(p) — return p != &Fallback;
• IndexFromHandle(base) — reverse-lookup walk
• PoolIndexIsValid(i) — i < POOL_SIZE
• FreeIfInUse(i) — locked Cleanup + MarkFree

That's a big DRY violation about to be replicated across BlockStore, FileBlockDevice, StreamSender, every Platform Mutex / Stream / Datagram / File, the Atomic counters, TLS, FatFs, … Worth extracting before the second E11 class lands.

Goal

A new TU-internal helper, SolidSyslogPoolAllocator, that owns the slot-walk and the per-iteration LockConfig/UnlockConfig over a caller-supplied bool[]. Each E11 class drops from ~13 pool-management functions to ~3 (_Create, _Destroy, IndexFromHandle + a 3-line cleanup callback).

Decisions locked in (do not relitigate)

These were agreed at the end of the S11.01 session — they're load-bearing:

Helper API — exactly this shape

/* Core/Source/SolidSyslogPoolAllocator.h — TU-internal, NOT in Core/Interface/ */
struct SolidSyslogPoolAllocator
{
    bool*  inUse;     /* caller-owned array, length == count           */
    size_t count;     /* number of slots                               */
};

/* Returns index of first free slot (now marked in-use), or count on
 * exhaustion. LockConfig wraps EACH per-slot probe -- not lock-once-
 * around-loop. Initialise of the concrete object runs OUTSIDE the lock,
 * after this returns. */
size_t SolidSyslogPoolAllocator_AcquireFirstFree(struct SolidSyslogPoolAllocator* allocator);

typedef void (*SolidSyslogPoolCleanup)(size_t index, void* context);

/* Returns true if the slot was in-use and is now free. The cleanup
 * callback (if non-NULL) fires INSIDE the per-slot lock, BEFORE InUse
 * is cleared. The lock-held-during-Cleanup invariant is the whole
 * reason for the callback signature -- doing Cleanup after the helper
 * unlocks would let a concurrent AcquireFirstFree grab the slot and
 * race Initialise vs Cleanup on the same memory. */
bool SolidSyslogPoolAllocator_FreeIfInUse(
    struct SolidSyslogPoolAllocator* allocator,
    size_t                           index,
    SolidSyslogPoolCleanup           cleanup,
    void*                            context
);

static inline bool SolidSyslogPoolAllocator_IndexIsValid(
    const struct SolidSyslogPoolAllocator* allocator,
    size_t                                  index)
{
    return index < allocator->count;
}

Caller-supplied storage (option B, not macro-instantiated)

The helper does not own the bool[]. The caller declares static bool ClassInUse[N]; and threads it through. Reasons rejected for the alternative (macro-instantiated helper with internal bool inUse[N]):

• Macro-instantiated headers get ugly fast and surprise readers.
• Caller-supplied storage matches the project's established pattern (cf. SolidSyslogFormatterStorage, SolidSyslogPosixMutexStorage, …).
• Allows pool size to remain a per-class tunable in SolidSyslogTunablesDefaults.h.

Initialise outside lock, Cleanup inside lock

Same invariant S11.01 established:

• Acquire side: helper returns index, caller runs Initialise lock-free. The slot is reserved by InUse=true; no other task can see it because they have no handle yet.
• Free side: helper runs Cleanup via callback while still holding the lock, then marks InUse=false. Releasing the lock before Cleanup would let a concurrent AcquireFirstFree grab the slot and race Initialise vs Cleanup on the same memory.

This asymmetry is the right answer; do not flatten it into a single shape.

No new MISRA suppressions / NOLINTs

The E11 invariant. The helper introduces:

• Function pointer typedef — fine (existing pattern, see SolidSyslogStoreFullCallback).
• void* context parameter — passed through to the cleanup callback. CircularBuffer passes NULL and the callback ignores it. No casts.
• A bool* field in the helper struct — no casts.

Zero new 11.3 / 11.5 / 11.8 hazards. If a cast appears anywhere in the helper or in the migrated class, stop and reshape — that's the E11 invariant.

Three-TU split survives unchanged

CircularBuffer.c and CircularBufferPrivate.h do not change. CircularBufferStatic.c rewrites against the helper. The pattern memory project_e11_three_tu_split.md still applies — the new helper is not a fourth TU per class; it's one shared TU pair used by every *Static.c.

Reference shape after migration

Core/Source/SolidSyslogCircularBufferStatic.c post-migration:

#include "SolidSyslogCircularBuffer.h"

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

#include "SolidSyslogBufferDefinition.h"
#include "SolidSyslogCircularBufferPrivate.h"
#include "SolidSyslogError.h"
#include "SolidSyslogErrorMessages.h"
#include "SolidSyslogPoolAllocator.h"
#include "SolidSyslogPrival.h"
#include "SolidSyslogTunables.h"

struct SolidSyslogMutex;

static bool                              Fallback_Read(struct SolidSyslogBuffer*, void*, size_t, size_t*);
static void                              Fallback_Write(struct SolidSyslogBuffer*, const void*, size_t);
static size_t                            CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base);
static void                              CircularBuffer_CleanupAtIndex(size_t index, void* context);

static bool                              InUse[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE];
static struct SolidSyslogCircularBuffer  Pool[SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE];
static struct SolidSyslogBuffer          Fallback   = {Fallback_Write, Fallback_Read};
static struct SolidSyslogPoolAllocator   Allocator  = {InUse, SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE};

struct SolidSyslogBuffer* SolidSyslogCircularBuffer_Create(
    struct SolidSyslogMutex* mutex, uint8_t* ring, size_t ringBytes)
{
    size_t index = SolidSyslogPoolAllocator_AcquireFirstFree(&Allocator);
    struct SolidSyslogBuffer* handle = &Fallback;
    if (SolidSyslogPoolAllocator_IndexIsValid(&Allocator, index))
    {
        CircularBuffer_Initialise(&Pool[index].Base, mutex, ring, ringBytes);
        handle = &Pool[index].Base;
    }
    else
    {
        SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_ERROR, SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_POOL_EXHAUSTED);
    }
    return handle;
}

void SolidSyslogCircularBuffer_Destroy(struct SolidSyslogBuffer* base)
{
    size_t index = CircularBuffer_IndexFromHandle(base);
    bool released = SolidSyslogPoolAllocator_IndexIsValid(&Allocator, index)
                 && SolidSyslogPoolAllocator_FreeIfInUse(&Allocator, index, CircularBuffer_CleanupAtIndex, NULL);
    if (!released)
    {
        SolidSyslog_Error(SOLIDSYSLOG_SEVERITY_WARNING, SOLIDSYSLOG_ERROR_MSG_CIRCULARBUFFER_UNKNOWN_DESTROY);
    }
}

static void CircularBuffer_CleanupAtIndex(size_t index, void* context)
{
    (void) context;
    CircularBuffer_Cleanup(&Pool[index].Base);
}

static size_t CircularBuffer_IndexFromHandle(const struct SolidSyslogBuffer* base)
{
    size_t result = SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE;
    for (size_t poolIndex = 0; poolIndex < SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE; poolIndex++)
    {
        if (base == &Pool[poolIndex].Base)
        {
            result = poolIndex;
            break;
        }
    }
    return result;
}

/* Fallback_Read / Fallback_Write unchanged */

Notice:

• struct Slot wrapper is gone — Pool is now an array of bare struct SolidSyslogCircularBuffer, InUse is its own array. The wrapper survived from the index-pair pattern; the helper makes it redundant.
• 13 helper functions collapse to 3 (IndexFromHandle, CleanupAtIndex, the two Fallback_* vtable entries — really 2 helpers + 2 vtable entries).
• Direct field access (&Pool[index].Base) replaces HandleFromIndex(index) — that helper added zero value once struct Slot was retired.

Acceptance criteria

1. New Core/Source/SolidSyslogPoolAllocator.{h,c} exists. .h is TU-internal (referenced only from Core/Source/).
2. SolidSyslogCircularBufferStatic.c uses the helper; the 10 file-scope helpers listed in Context are gone.
3. struct Slot is retired — Pool and InUse are separate arrays.
4. New Tests/SolidSyslogPoolAllocatorTest.cpp exercises the helper directly via ConfigLockFake and an in-test cleanup-callback spy. Aim for 100% line / function / branch coverage like CircularBufferStatic.c achieved.
5. The existing Tests/SolidSyslogCircularBufferTest.cpp mostly survives unchanged — the Pool TEST_GROUP tests the public API which still works. The lock-count assertions still hold:
   • CreateAcquiresAndReleasesConfigLockOnFirstFreeSlot → 1 lock acquisition (one slot probed).
   • CreateLocksOncePerSlotProbedWhenPoolIsFull → POOL_SIZE lock acquisitions.
   • DestroyOfPooledHandleLocksOnce → 1.
   • DestroyOfUnknownHandleDoesNotLock → 0 (the helper's IndexFromHandle finds no match, PoolAllocator_FreeIfInUse is never reached).
6. All gates green: debug, clang-debug, sanitize, tidy, cppcheck, coverage, clang-format, cppcheck-misra. No new suppressions / NOLINTs.
7. Full suite passes at SOLIDSYSLOG_CIRCULAR_BUFFER_POOL_SIZE = 3 (the validation step David asked for at end of S11.01).
8. cppcheck-misra count stays ≤ 87 (current main baseline after S11.01).
9. DEVLOG entry on the work branch BEFORE the PR.

Out of scope

• Do not migrate any other class yet. The first migrated class (CircularBuffer) is the proof. BlockStore and friends come in S11.03+.
• Do not move the helper to Core/Interface/. It's a library-internal abstraction; integrator code must not see it.
• Do not expose inUse or count outside the helper struct in operations. All access via the three operations + IndexIsValid.
• Do not change the public CircularBuffer API. _Create(mutex, ring, ringBytes) and _Destroy(base) stay byte-identical.
• Do not add SolidSyslogPoolAllocatorStorage-style external sizing. The struct is small enough to live as a static instance in each consumer's TU.

TDD discipline (per project memory feedback_tdd_and_review_style.md)

One TDD cycle at a time. The helper has only three operations + the predicate — build it test-first:

1. IndexIsValid (one line) — drive both true and false cases.
2. AcquireFirstFree happy — empty pool, returns 0, marks slot 0.
3. AcquireFirstFree walk — slot 0 in use, returns 1.
4. AcquireFirstFree exhausted — all slots in use, returns count.
5. AcquireFirstFree locks per probe — ConfigLockFake count == probed slots (1 for empty, N for full).
6. FreeIfInUse happy — marks slot free, returns true, invokes cleanup callback with right index.
7. FreeIfInUse already-free — returns false, does NOT invoke cleanup.
8. FreeIfInUse invokes cleanup INSIDE the lock — fake cleanup checks LockCallCount at invocation time vs UnlockCallCount (one more lock than unlock at the moment of cleanup).
9. FreeIfInUse NULL cleanup callback — skips the call, still marks free.
10. FreeIfInUse locks exactly once — fake count == 1.

Then migrate CircularBufferStatic.c in a separate commit. Existing CircularBuffer tests must continue to pass through the migration — that's the regression net.

Sequencing on the work branch

Recommended commit shape on feat/s11-02-pool-allocator:

1. feat: S11.02 add SolidSyslogPoolAllocator helper — new helper + tests. Not yet wired into CircularBuffer.
2. refactor: S11.02 migrate CircularBufferStatic onto PoolAllocator — drops the 10 helper functions, drops struct Slot, retains 100% coverage and all existing test assertions.
3. docs: update DEVLOG for S11.02

Squash-merge target like S11.01: feat: S11.02 extract PoolAllocator helper (E11 pre-sweep).

Memory pointers for the next session

Read these before touching anything:

• CLAUDE.md (project root) + SKILL.md
• MEMORY.md (project memory index) — particularly:
  • project_e11_static_pool_design.md — pool design agreements
  • project_e11_three_tu_split.md — three-TU split shape (still applies; this story does NOT change it)
  • project_configlock_injection.md — SolidSyslog_SetConfigLock shape
  • project_allocation_epic.md — future dynamic-allocation epic context
  • feedback_storage_pattern.md — caller-supplied storage convention
  • feedback_tdd_and_review_style.md — pacing
  • feedback_drive_arg_values_in_same_test.md — TDD assertion bundling
  • feedback_devlog_in_pr.md — DEVLOG commit goes on the work branch
  • feedback_no_git_commit_dash_s.md — never -S
  • feedback_no_merge.md — never merge; David merges PRs himself
• DEVLOG entry for S11.01 (commits 2–N) — captures the locked-in shape this story builds on.
• Issue S11.01: Pilot — CircularBuffer pool migration (E11 canonical pattern) #392 (S11.01) and PR feat: S11.01 CircularBuffer pool migration (E11 pilot) #394 — full pilot context and the discussion that led to this extraction.

Open questions for the next session to surface

• Whether the helper should report its own SolidSyslog_Error on exhaustion instead of returning count and leaving the caller to do it. Argument for: removes a per-class duplicated error-report. Argument against: error messages are class-specific (_CIRCULARBUFFER_POOL_EXHAUSTED vs _BLOCKSTORE_POOL_EXHAUSTED etc), so each class wants its own text. Default: leave error reporting to the caller (status quo) unless the next session sees a clean way to parameterise.
• Whether IndexFromHandle should also be hoisted. Probably not — the per-class reverse-lookup is the one bit that's genuinely typed and would need either a const void* cast or a per-class function pointer. The five lines per class aren't worth the abstraction overhead. Default: leave per-class.
• Whether PoolAllocator deserves an entry in Core/Interface/ audience table in CLAUDE.md. No — it's TU-internal, not an integration point. Default: no doc surface.