Skip to content

feat!: S21.04 BlockDevice owns its block size; drop MaxBlockSize from BlockStoreConfig#528

Merged
DavidCozens merged 7 commits into
mainfrom
refactor/s21-04-block-size-from-device
Jun 4, 2026
Merged

feat!: S21.04 BlockDevice owns its block size; drop MaxBlockSize from BlockStoreConfig#528
DavidCozens merged 7 commits into
mainfrom
refactor/s21-04-block-size-from-device

Conversation

@DavidCozens

@DavidCozens DavidCozens commented Jun 4, 2026

Copy link
Copy Markdown
Owner

Purpose

Closes #527. The last open thread under E21 (Port-Time Configurability): make the
SolidSyslogBlockDevice the single source of truth for its own block size, instead of
the integrator supplying a separate MaxBlockSize on SolidSyslogBlockStoreConfig that
could silently disagree with the device.

This is an API break (a public config field is removed and a vtable op is added), so
it is deliberately landed during the 0.x beta window — after 1.0.0 the API is additive-only.

Change Description

Block size moves from config to the device, queried once at BlockStore_Create:

  • SolidSyslogBlockDevice gains a device-wide GetBlockSize() vtable op (distinct from
    Size(blockIndex), which is a block's current occupancy). NullBlockDevice returns 0.
  • SolidSyslogFileBlockDevice_Create(file, prefix, blockSize) takes the size at
    construction and reports it via GetBlockSize. blockSize == 0 selects the new
    SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE tunable (default 8192, floored at one worst-case
    record). The BDD targets route their runtime --max-block-size knob into the device.
  • MaxBlockSize is removed from SolidSyslogBlockStoreConfig; the store reads the
    device's size.
  • Undersized block → grow-and-warn (not reject). The store keeps the
    max(deviceBlockSize, oneRecord) floor so a worst-case record always fits, but now
    emits a WARNING (SOLIDSYSLOG_CAT_BAD_CONFIG / BLOCKSTORE_ERROR_BLOCK_TOO_SMALL)
    when it grows the block — delivered, degraded, no longer silent. (An earlier revision
    rejected outright with an ERROR; CI showed the clamp is a documented, load-bearing
    cross-target calibration that the drain tests and the store-capacity BDD suites depend
    on — syslog_steps.py comments on it explicitly — so the clamp is retained and made
    loud rather than removed.)
  • FatFs adapter adds a defined()-guarded compile-time FF_MAX_SS floor on the default.

Test Evidence

Strict TDD across the feature:

  1. GetBlockSize on the contract (NullBlockDevice→0)
  2. FileBlockDevice takes the size at Create; reports via GetBlockSize
  3. store reads the device; MaxBlockSize dropped from config (clamp retained). Size-varying
    tests re-point the fixture device via an idempotent EnsureDeviceBlockSize helper —
    same-size calls reuse the device so corruption-recovery's persistent file handle survives
  4. SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE tunable + device 0→default
  5. Undersized block → store still works (grown) and a WARNING is reported

1396 host unit tests green; full local cppcheck-misra green. The existing drain-ordering
and store-capacity calibrations are unchanged — the clamp preserves the exact effective
block sizes from main, so no message-fit arithmetic was re-derived.

Not verified locally (gcc lane can't build them — covered by CI): the FatFs FF_MAX_SS
floor and the Windows / FreeRTOS BDD-target callers (their _Create edits are symmetric to
the green Linux target).

Areas Affected

  • Tier 1: SolidSyslogBlockDevice{,Definition}.h, SolidSyslogBlockStore{,Errors}.h,
    SolidSyslogTunablesDefaults.h, BlockStoreStatic.c, NullBlockDevice.c,
    BlockDevice.c.
  • Tier 1 (FileBlockDevice): SolidSyslogFileBlockDevice{,Private}.h, *.c, *Static.c.
  • Tier 2: Platform/FatFs/Source/SolidSyslogFatFsFile.c (FF_MAX_SS floor).
  • Tier 3 (BDD targets): Linux / Windows / FreeRTOS pipeline _Create callers.
  • Derived-project impact: integrators wiring SolidSyslogBlockStore must drop
    config.MaxBlockSize and pass the size to SolidSyslogFileBlockDevice_Create instead.
    A block smaller than one max-size message still works (grown to fit) but now emits a
    WARNING.

🤖 Generated with Claude Code

DavidCozens and others added 6 commits June 4, 2026 15:58
Device-wide block capacity, distinct from Size(blockIndex) occupancy.
NullBlockDevice returns 0. Pure addition; no caller uses it yet.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…GetBlockSize

Create gains a blockSize parameter, stored and returned via GetBlockSize.
Callers updated: BDD targets route their runtime size into the device;
BlockStore tests pass a placeholder (config.MaxBlockSize still drives the
store until the next slice wires it to the device).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… from BlockStoreConfig

BlockStore_Create now reads SolidSyslogBlockDevice_GetBlockSize() instead of
config.MaxBlockSize, which is removed from SolidSyslogBlockStoreConfig (API break).
The min-block-size clamp is unchanged. Size-varying tests re-point the fixture
device via an idempotent EnsureDeviceBlockSize helper (same-size calls reuse the
device so corruption-recovery's persistent file handle survives). BDD targets
route their runtime size into the device instead of the store config.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ses-default

FileBlockDevice_Create resolves a blockSize of 0 to SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE
(default 8192, floored at one worst-case record). FatFs adapter gains a compile-time
FF_MAX_SS floor on the default. Renames the zero-size store test to reflect that 0 now
selects the device default rather than clamping to the minimum.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Design Y: when the device's block size cannot hold one worst-case record,
BlockStore_Create reports SOLIDSYSLOG_CAT_BAD_CONFIG / BLOCKSTORE_ERROR_BLOCK_TOO_SMALL
and falls back to NullStore instead of silently growing the device's reported size.
The old max(deviceSize, minRecord) clamp is gone — the device is the single source of
truth. Drain-ordering tests that leaned on the silent clamp now size blocks explicitly
to one near-max record.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…suppressions

- CLAUDE.md: GetBlockSize on the BlockDevice rows; FileBlockDevice_Create gains blockSize
- DEVLOG: S21.04 entry (Design Y, the load-bearing-clamp discovery)
- MISRA 15.7: terminal else on the BlockStore_Create report chain
- MISRA 20.9: defined() guard on the FatFs FF_MAX_SS floor
- misra_suppressions.txt: renumber 3 shifted findings (FileBlockDevice/FatFs 11.3, BlockStoreStatic 11.8)
- clang-format reflow

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR moves block-size ownership from SolidSyslogBlockStoreConfig to SolidSyslogBlockDevice. Devices now report their capacity via a new GetBlockSize vtable method; file-backed devices capture size at construction. The store validates the device can hold one worst-case record at creation time, rejecting undersized configurations with a new error code.

Changes

Block Device Owns Block Size

Layer / File(s) Summary
Block Device GetBlockSize API contract
Core/Interface/SolidSyslogBlockDevice.h, Core/Interface/SolidSyslogBlockDeviceDefinition.h, Core/Source/SolidSyslogBlockDevice.c
New public SolidSyslogBlockDevice_GetBlockSize function and matching vtable member GetBlockSize provide a standard way for devices to report their per-block capacity.
File block device block-size ownership
Core/Interface/SolidSyslogFileBlockDevice.h, Core/Source/SolidSyslogFileBlockDevice*.c
SolidSyslogFileBlockDevice_Create now accepts blockSize parameter, stores it, and wires GetBlockSize to return the stored value. The file block device becomes the single source of truth for its capacity.
Default block-size tunable and platform guards
Core/Interface/SolidSyslogTunablesDefaults.h, Platform/FatFs/Source/SolidSyslogFatFsFile.c
New SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE (8192) tunable with compile-time floor check ensuring one worst-case record fits. FatFs platform adds floor guard against FF_MAX_SS sector size.
Null block device GetBlockSize implementation
Core/Source/SolidSyslogNullBlockDevice.c
Null device wires GetBlockSize handler returning 0 to indicate no capacity.
Block store config change and device validation
Core/Interface/SolidSyslogBlockStore.h, Core/Interface/SolidSyslogBlockStoreErrors.h, Core/Source/SolidSyslogBlockStoreStatic.c
SolidSyslogBlockStoreConfig removes MaxBlockSize field; store creation validates device can hold one worst-case record via new BlockStore_DeviceCanHoldOneRecord helper; undersized devices trigger BLOCKSTORE_ERROR_BLOCK_TOO_SMALL. BlockStore_BuildBlockSequenceConfig sources size from device's GetBlockSize.
BDD target integrations
Bdd/Targets/Common/BddTargetFreeRtosPipeline.c, Bdd/Targets/Linux/main.c, Bdd/Targets/Windows/BddTargetWindows.c
Three BDD targets updated to pass MaxBlockSize into SolidSyslogFileBlockDevice_Create instead of SolidSyslogBlockStoreConfig.
Test infrastructure and coverage updates
Tests/SolidSyslogBlockStore*.cpp, Tests/SolidSyslogFileBlockDevice*.cpp, Tests/SolidSyslogNullBlockDevice*.cpp
Test fixtures create devices with explicit block sizes; helpers conditionally recreate devices via new EnsureDeviceBlockSize / ensureDeviceBlockSize utilities. New test coverage for GetBlockSize API, default block-size selection, undersized-device rejection, and error reporting.
Documentation and line-number updates
CLAUDE.md, DEVLOG.md, misra_suppressions.txt
CLAUDE.md and DEVLOG.md document new API and contract changes. MISRA suppression line numbers adjusted for code shifts.

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • DavidCozens/solid-syslog#426: Touches Core/Source/SolidSyslogNullBlockDevice.c vtable initialization that this PR extends with GetBlockSize.
  • DavidCozens/solid-syslog#354: Also modifies file-backed block-device creation/wiring; signatures overlap with this PR’s constructor change.
  • DavidCozens/solid-syslog#243: Introduced the file-backed block device abstraction that this PR extends with device-owned block-size behavior.

"I’m a rabbit in the log, I hop and write with care,
Devices whisper sizes now — no config snare,
If a block’s too small, a gentle warning sings,
Tests remake the garden where certainty springs,
Hooray — the device owns its share! 🐇"

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 38.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main API change: making BlockDevice own its block size and removing MaxBlockSize from BlockStoreConfig.
Linked Issues check ✅ Passed All acceptance criteria from #527 are met: GetBlockSize added to vtable, MaxBlockSize removed from config, FileBlockDevice_Create takes blockSize parameter, SOLIDSYSLOG_FILE_DEFAULT_BLOCK_SIZE tunable added with FatFs floor check, tests updated.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the #527 objectives: vtable/API refactoring, config field removal, tunable introduction, file-device signature update, test adjustments, and BDD target wiring—no unrelated modifications present.
Description check ✅ Passed The PR description comprehensively addresses all required sections: it clearly states the purpose (closing #527, making BlockDevice the single source of truth), provides detailed change description with architecture decisions, includes test evidence with TDD approach and test counts, and lists affected areas across multiple tiers with integration impact.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch refactor/s21-04-block-size-from-device

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1433 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1778 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1367 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1367 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 16 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 14 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 16 passed)
   🚦   bdd-linux-syslog-ng: 80% successful (❌ 7 failed, ✔️ 41 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 69% successful (❌ 10 failed, ✔️ 35 passed, 🙈 6 skipped)
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1222 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1367 passed)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: No warnings


Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

…Design Z)

CI revealed the max(deviceSize, oneRecord) clamp was load-bearing: a documented,
cross-target calibration the drain tests and the store_capacity/capacity_threshold/
block_lifecycle BDD suites depend on. Rejecting (Design Y, ERROR -> NullStore) broke
them and tripped the BDD target's _Exit(3)-on-ERROR handler. Keep the clamp, but emit a
WARNING (BAD_CONFIG / BLOCK_TOO_SMALL) so it is no longer silent. Reverts the drain-test
recalibration back to the proven 200/64 + 200/(MAX-100) values; converts the reject
tests to clamp-works + reports-warning. cppcheck-misra green (11.8 suppressed at the two
new GetBlockSize sites, same const-config-member pattern already deviated in this file).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

☀️   Quality Summary

   🚦   build-linux-gcc: 100% successful (✔️ 1433 passed)
   🚦   build-freertos-host-tdd-plustcp: 100% successful (✔️ 1778 passed)
   🚦   build-linux-clang: 100% successful (✔️ 1367 passed)
   🚦   sanitize-linux-gcc: 100% successful (✔️ 1367 passed)
   🚦   integration-linux-openssl: 100% successful (✔️ 16 passed)
   🚦   integration-linux-mbedtls: 100% successful (✔️ 14 passed)
   🚦   integration-windows-openssl: 100% successful (✔️ 16 passed)
   🚦   bdd-linux-syslog-ng: 94% successful (✔️ 48 passed, 🙈 3 skipped)
   🚦   bdd-windows-otel: 88% successful (✔️ 45 passed, 🙈 6 skipped)
   🚦   bdd-freertos-qemu-plustcp: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   bdd-freertos-qemu-lwip: 86% successful (✔️ 44 passed, 🙈 7 skipped)
   🚦   build-windows-msvc: 100% successful (✔️ 1222 passed)
   🚦   build-linux-tunable-override: 100% successful (✔️ 1367 passed)
   ⚠️   Clang-Tidy: No warnings
   ⚠️   CPPCheck: No warnings


Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result.

@DavidCozens DavidCozens merged commit 298e20e into main Jun 4, 2026
27 checks passed
@DavidCozens DavidCozens deleted the refactor/s21-04-block-size-from-device branch June 4, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

S21.04: BlockDevice reports its own block size + single SOLIDSYSLOG_FILE_BLOCK_SIZE tunable

1 participant