python/swig test: add nvme.i consistency check and fix violations#3308
Open
martin-belanger wants to merge 1 commit intolinux-nvme:masterfrom
Open
python/swig test: add nvme.i consistency check and fix violations#3308martin-belanger wants to merge 1 commit intolinux-nvme:masterfrom
martin-belanger wants to merge 1 commit intolinux-nvme:masterfrom
Conversation
dwsuse
reviewed
Apr 22, 2026
Add tools/check-nvme-i-consistency.py, a static analysis script that
verifies nvme.i stays consistent with the accessor annotations in
private.h and private-fabrics.h. It enforces several rules:
Rule 1 (error): !accessors:readonly fields exposed in nvme.i must
be declared %immutable.
Rule 2 (error): fields with auto-generated or bridged accessors
must be in %extend{} so SWIG invokes the getter
instead of accessing the struct member directly.
Rule 3 (error): fields in %extend{} must have a getter bridge
whose target is declared in a public header.
Check 4 (error): bridge target not found in any public header.
Check 5 (error): %immutable declaration for a field that does not
exist in the corresponding C struct.
Check 6 (warning): field has an accessor but is not exposed in nvme.i.
Check 7 (warning): field type in nvme.i differs from private header.
The check is registered as a meson test in libnvme/libnvme/meson.build
inside the 'if want_python' block. It performs pure static analysis and
does not depend on the built SWIG module.
Fix all Rule 2 violations found by the new check by moving the
offending fields into %extend{} blocks (using existing blocks where
possible, creating one for libnvme_ns). Remove stale comments on
cntrltype, dctype, and discovered that incorrectly claimed no getter
method was available.
Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Sonnet 4.6 <[email protected]>
b454be1 to
514511f
Compare
Collaborator
|
The only nitpick I have this should be three patches: One for .gitingore, one for the nvme.i change and one for the check added. Rest looks good. Thanks! |
Author
|
@dwsuse - Actually, let's hold off on this PR. This may not be required following my proposal to generate most of the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
python: add nvme.i consistency check and fix violations
Background
The SWIG interface file
libnvme/libnvme/nvme.iexposes libnvme's internal C structs to Python. Struct fields are meant to be accessed through getter/setter accessor functions (e.g.,libnvme_ctrl_get_name()) rather than by direct struct member access (e.g.,ctrl->name). This is enforced through a combination of SWIG mechanisms:%extend {}block cause SWIG to generate calls to the accessor functions instead of accessing the struct member directly.!accessors:readonlyin the private headers must be declared%immutableinnvme.ito prevent SWIG from generating a setter.Keeping
nvme.iconsistent with the accessor annotations inprivate.handprivate-fabrics.his error-prone and easy to get wrong. Every time a new field is added to a!generate-accessorsstruct, or an annotation changes,nvme.imust be updated in sync — and there was previously no automated check to catch mistakes.What this PR adds
libnvme/tools/check-nvme-i-consistency.pyA new static analysis script that validates
nvme.iagainst the accessor annotations in the private headers. It is pure Python (stdlib only) and performs no compilation — it parses source files with regex + brace counting.The script enforces the following rules:
!accessors:readonlythat is exposed innvme.imust be declared%immutable.%extend {}block, not as a direct struct member. Direct access bypasses the public accessor API.%extend {}must have a getter bridge (#define libnvme_STRUCT_FIELD_get ACTUAL_FUNC) whose target function is declared in at least one public header.%immutable FIELDdeclaration innvme.irefers to a field that does not exist in the corresponding C struct.nvme.iat all (may be intentional, but surfaces accidental omissions).nvme.idiffers from its type in the private header (e.g.,char *vsconst char *). A built-in equivalence table suppresses false positives from kernel-style typedefs (__u32↔unsigned int,__u8↔uint8_t, etc.).The script bridges from two sources:
nvme-swig-accessors.i— auto-generated#definebridges for all accessor-generated functions%{ ... %}block innvme.i— hand-written bridges for hand-written functionsThe script exits with a non-zero status when errors are found (causing the meson test to fail) and with zero when only warnings are present. Errors are printed to stderr so they appear in
meson-logs/testlog.txton failure.Meson test registration
The check is registered as a meson test in
libnvme/libnvme/meson.buildinside theif want_pythonblock. It does not depend on the built SWIG module (pynvme_clib) — it is pure static analysis and runs quickly.Fixes to
libnvme/libnvme/nvme.iThe new check immediately found 12 Rule 2 violations — fields with auto-generated getters that were exposed as direct struct members instead of through
%extend {}. These have been fixed:%extend {}libnvme_hosthostnqn,hostid,hostsymnamelibnvme_subsystemsubsysnqn,model,serial,firmware,subsystypelibnvme_ctrlcntrltype,dctype,discoveredlibnvme_nsnsid(new%extend {}block created)Stale comments on
cntrltype,dctype, anddiscoveredclaiming "no getter method in libnvme.map" have been removed — those getters exist and are correctly mapped innvme-swig-accessors.i.Testing
The test passes with zero errors. Remaining warnings (Check 6, Check 7) reflect known intentional patterns in the current
nvme.iand do not block the build.