docs: S23.22 drop the FatFs sector floor and its porting-guide example - #772
Conversation
|
Caution Review failedAn error occurred during the review process. Please try again later. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
Included review availability: 1 review is currently available. Based on recent review activity, included reviews refill at 2 per hour. WalkthroughThe change revises the block-size validation comment to describe sector-level read-modify-write operations for sub-sector blocks. The ChangesComment quality
Estimated code review effort: 1 (Trivial) | ~2 minutes Merge Risk: ⚪ Minimal · up to This PR corrects documentation for an existing FatFs sector-size check without changing behavior or APIs; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
The check rejected a file-block-size default below one FatFs sector, on the stated grounds that a smaller block cannot back a coherent on-disk record layout. It can. Nothing in Core assumes sector alignment or an atomic block write - records are self-framed and integrity-checked by the SecurityPolicy - and the adapter writes through byte-granular f_write, so the filesystem does the read-modify-write. A sub-sector block is inefficient, not incorrect, and enforcing the integrator's media geometry is not this library's business. It also only ever covered the default: SolidSyslogFileBlockDevice_Create takes blockSize as a runtime parameter that bypasses it. The floor that is load-bearing - one worst-case record - stays in SolidSyslogTunablesDefaults.h, unconditional and for every backend. FreeRTOS+FAT never got the equivalent check because it has no equivalent macro: it takes the sector size as a runtime argument at disk init. Removing this one settles that asymmetry rather than explaining it, and the two FAT adapters now have the same shape. The porting guide's rule for choosing between an #error and a build gate cited this check as its worked example, and no longer has one. The section goes with it: the guide is for porters adding an adapter, not a tutorial on preprocessor directives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
574adde to
64fb695
Compare
DavidCozens
changed the title
☀️ Quality Summary Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
The last loose end on #708. The audit recorded that
Platform/PlusFat/has nocompile-time sector-size check where
Platform/FatFs/has one, and asked whetherthat gap needed closing. Investigating it showed the check itself is the thing to
remove.
Change Description
The check goes. It rejected a file-block-size default below one FatFs sector,
on the stated grounds that a smaller block cannot back a coherent on-disk record
layout. It can. Nothing in
SolidSyslogFileBlockDevice.c,SolidSyslogBlockSequence.corSolidSyslogBlockStore.hassumes sector alignmentor an atomic block write - records are self-framed and integrity-checked by the
SecurityPolicy - and the adapter writes through byte-granular
f_write, so thefilesystem does the read-modify-write. A sub-sector block is inefficient, not
incorrect, and enforcing the integrator's media geometry is not this library's
business.
Two further reasons it was not earning its place:
SolidSyslogFileBlockDevice_CreatetakesblockSizeas a runtime parameter, and any non-zero value bypassed it. The floorthat is load-bearing - one worst-case record - stays in
SolidSyslogTunablesDefaults.h, unconditional and for every backend.#erroroutside that header, and the only one guardingefficiency rather than a value below which the library cannot work.
Plus-FAT needs no equivalent, and now no explanation.
FF_MAX_SSis a FatFsmacro; FreeRTOS+FAT publishes no compile-time sector-size macro at all, taking the
sector size as a runtime argument at disk init, so #527's acceptance criterion was
never expressible for that backend. Removing this check settles the asymmetry
rather than documenting it, and the two FAT adapters now have the same shape.
SolidSyslogTunables.hgoes with it - that translation unit used no othertunable macro. It stays in
SolidSyslogFatFsFileStatic.c, which needs the poolsize.
docs/porting.mdloses a section. Its rule for choosing between an#errorand a build gate cited this check as its worked example, and the tree now has no
platform-side
#errorto put in its place. The guide is for porters adding anadapter rather than a tutorial on preprocessor directives, so the section goes
rather than acquiring a hypothetical. Worth noting that the check failed that
section's own criterion - "the integrator is using the class and their
configuration contradicts it" - which is what prompted looking at it.
Test Evidence
clang-format --dry-run --Werrorover the whole tree: clean.scripts/misra_renumber.py --apply, run after the format check: the 11.3 entryfor this file moves 63 -> 53, one line off its Plus-FAT sibling at 52.
markdownlint-cli2v0.22.1 over all 64 files: 0 errors.preprocessor block and an include with no other use in that translation unit.
Areas Affected
Platform/FatFs/Source/SolidSyslogFatFsFile.c,docs/porting.md,misra_suppressions.txt. No behaviour change and no API change: an integrator whopreviously hit the
#errornow builds and writes sub-sector blocks, which works.Closes #708