Skip to content

swig: replace hand-written accessor wrappers with auto-generated #defines#3304

Merged
igaw merged 1 commit intolinux-nvme:masterfrom
martin-belanger:swig-accessor-defines
Apr 22, 2026
Merged

swig: replace hand-written accessor wrappers with auto-generated #defines#3304
igaw merged 1 commit intolinux-nvme:masterfrom
martin-belanger:swig-accessor-defines

Conversation

@martin-belanger
Copy link
Copy Markdown

@martin-belanger martin-belanger commented Apr 21, 2026

swig: auto-generate #define bridges for accessors

SWIG uses a naming convention (libnvme_STRUCT_MEMBER_get/set) that differs from the libnvme C API convention (libnvme_STRUCT_get/set_MEMBER). Until now this mismatch was papered over by ~35 hand-written C wrapper functions inside nvme.i — one thin shim per accessor that did nothing but call the real function.

This commit replaces all those shims with automatically generated C preprocessor #define object-like macros. Because the macros have no parameter list, the arguments flow through naturally, so no wrapper function body is needed at all.

What changed

New script: libnvme/tools/generator/generate-swig-accessors.py

Post-processing step that reads accessors.h and accessors-fabrics.h (already kept up to date by generate-accessors.py) and emits libnvme/libnvme/nvme-swig-accessors.i. It uses a non-greedy regex to split each accessor name at the first _get_ / _set_ occurrence so that multi-word member names (e.g. dhchap_host_key) and long struct prefixes (e.g. libnvmf_discovery_args) are handled correctly. Output is only rewritten when the content changes, avoiding spurious mtime updates.

Updated: libnvme/tools/generator/meson.build

  • Added update-swig-accessors run-target that invokes the new script.
  • Folded it into the existing update-accessors alias so that meson compile -C <build-dir> update-accessors regenerates all three artefact sets in one shot.

New generated file: libnvme/libnvme/nvme-swig-accessors.i

Contains one #define per accessor for every struct annotated in private.h and private-fabrics.h, grouped by struct and with the third column vertically aligned within each group for readability.

Updated: libnvme/libnvme/nvme.i

  • Added %include "nvme-swig-accessors.i" (alongside the existing %include "exception.i") so SWIG picks up all the bridges automatically.

  • Removed all four hand-written wrapper %{ %} blocks (host, subsystem, ctrl, ns) — 168 lines deleted.

  • Kept four #define bridges that cannot be auto-generated (virtual Python property names or !accessors:none annotations), consolidated into the main %{ %} preamble with a comment explaining why each one exists:

    /* Cannot be auto-generated — virtual names or !accessors:none fields */
    #define libnvme_subsystem_host_get libnvme_subsystem_get_host
    #define libnvme_ctrl_state_get     libnvme_ctrl_get_state
    #define libnvme_ctrl_subsystem_get libnvme_ctrl_get_subsystem
    #define libnvme_ctrl_address_get   libnvme_ctrl_get_traddr

Result

New accessors added to libnvme (via private.h annotations) now automatically appear in the Python bindings after running meson compile -C <build-dir> update-accessors — zero manual maintenance of nvme.i required.

…ines

SWIG generates accessor names as libnvme_STRUCT_MEMBER_get/set() but
libnvme exports them as libnvme_STRUCT_get/set_MEMBER().  Previously
nvme.i bridged this gap with ~35 hand-written C wrapper functions
(~130 lines of boilerplate) that had to be updated manually whenever
new accessors were added to the library.

Add generate-swig-accessors.py, a post-processing script that reads the
accessor header files produced by generate-accessors.py (accessors.h
and accessors-fabrics.h, covering both common and fabrics structs) and
emits nvme-swig-accessors.i containing one #define per accessor:

  #define libnvme_ctrl_name_get libnvme_ctrl_get_name

Parsing the .h files (not the .ld version scripts) ensures the output
is always in sync even before the .ld section labels are manually
bumped.  Object-like macros are used rather than function-like macros
since the call arguments follow naturally after macro expansion.

Add a new update-swig-accessors run_target in meson.build and extend
the update-accessors alias to include it.  Replace the hand-written
wrapper blocks in nvme.i with %include "nvme-swig-accessors.i".  The
four bridges that cannot be auto-generated (virtual member names or
!accessors:none annotations) are kept as #defines in a single block
next to the %include.

Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Sonnet 4.6 <[email protected]>
Comment thread libnvme/libnvme/nvme.i
#define libnvme_subsystem_host_get libnvme_subsystem_get_host
#define libnvme_ctrl_state_get libnvme_ctrl_get_state
#define libnvme_ctrl_subsystem_get libnvme_ctrl_get_subsystem
#define libnvme_ctrl_address_get libnvme_ctrl_get_traddr
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am trying to figure out if the traddr vs address name is going to bite us.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I can tell just looking at struct libnvme_ctrl (private.h, line 264), address and traddr are not the same thing. Or are they?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

address got renamed to traddr to match with the kernel argument names. I suppose we should remove address from the Python binding.

@igaw igaw merged commit 002c6d6 into linux-nvme:master Apr 22, 2026
28 of 29 checks passed
@igaw
Copy link
Copy Markdown
Collaborator

igaw commented Apr 22, 2026

Thanks!

@martin-belanger martin-belanger deleted the swig-accessor-defines branch April 22, 2026 15:00
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.

2 participants