Skip to content

Commit b454be1

Browse files
author
Martin Belanger
committed
python: add nvme.i consistency check and fix violations
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 <Martin.Belanger@dell.com> Assisted-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent d391152 commit b454be1

4 files changed

Lines changed: 594 additions & 14 deletions

File tree

.gitignore

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ subprojects/*
1313
cscope.*
1414
compile_commands.json
1515

16-
tests/__pycache__
16+
__pycache__/
1717
tests/nvmetests
1818
tests/*.pyc
1919

libnvme/libnvme/meson.build

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,32 @@ if want_python
7777
test_env.set('PYTHONMALLOC', 'malloc')
7878

7979
# Test section
80+
_priv_hdr_args = [
81+
'--private-header',
82+
meson.current_source_dir() / '..' / 'src' / 'nvme' / 'private.h',
83+
'--private-header',
84+
meson.current_source_dir() / '..' / 'src' / 'nvme' / 'private-fabrics.h',
85+
]
86+
_swig_pub_hdr_args = []
87+
foreach hdr : headers
88+
_swig_pub_hdr_args += [
89+
'--public-header',
90+
meson.current_source_dir() / '..' / 'src' / hdr,
91+
]
92+
endforeach
93+
test(
94+
'libnvme - check-nvme-i-consistency',
95+
python3,
96+
args: [
97+
files('../tools/check-nvme-i-consistency.py'),
98+
] + _priv_hdr_args + _swig_pub_hdr_args + [
99+
'--swig-interface',
100+
meson.current_source_dir() / 'nvme.i',
101+
'--swig-accessors',
102+
meson.current_source_dir() / 'nvme-swig-accessors.i',
103+
],
104+
)
105+
80106
test('libnvme - python-import-libnvme', python3, args: ['-c', 'from libnvme import nvme'], env: test_env, depends: pynvme_clib)
81107

82108
py_tests = [

libnvme/libnvme/nvme.i

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -582,10 +582,10 @@ struct libnvme_host {
582582
%immutable hostnqn;
583583
%immutable hostid;
584584
%immutable hostsymname;
585-
char *hostnqn;
586-
char *hostid;
587-
char *hostsymname;
588585
%extend {
586+
char *hostnqn;
587+
char *hostid;
588+
char *hostsymname;
589589
char *dhchap_host_key;
590590
}
591591
};
@@ -598,13 +598,13 @@ struct libnvme_subsystem {
598598
%immutable firmware;
599599
%immutable subsystype;
600600
%immutable iopolicy;
601-
char *subsysnqn;
602-
char *model;
603-
char *serial;
604-
char *firmware;
605-
char *subsystype;
606601

607602
%extend {
603+
char *subsysnqn;
604+
char *model;
605+
char *serial;
606+
char *firmware;
607+
char *subsystype;
608608
const char *sysfs_dir;
609609
const char *application;
610610
const char *iopolicy;
@@ -633,10 +633,6 @@ struct libnvme_ctrl {
633633
%immutable phy_slot;
634634
%immutable discovered;
635635

636-
const char *cntrltype; // Do not put in %extend because there's no getter method in libnvme.map
637-
const char *dctype; // Do not put in %extend because there's no getter method in libnvme.map
638-
const bool discovered; // Do not put in %extend because there's no getter method in libnvme.map
639-
640636
%extend {
641637
/**
642638
* By putting these attributes in an %extend block, we're
@@ -665,10 +661,13 @@ struct libnvme_ctrl {
665661
const char *trsvcid;
666662
const char *cntlid;
667663
const char *phy_slot;
664+
const char *cntrltype;
665+
const char *dctype;
668666

669667
bool unique_discovery_ctrl;
670668
bool discovery_ctrl;
671669
bool persistent;
670+
bool discovered;
672671

673672
char *keyring;
674673
char *tls_key_identity;
@@ -695,7 +694,9 @@ struct libnvme_ns {
695694
%immutable eui64;
696695
%immutable nguid;
697696
%immutable uuid;
698-
unsigned int nsid;
697+
%extend {
698+
unsigned int nsid;
699+
}
699700
uint8_t eui64[8];
700701
uint8_t nguid[16];
701702
uint8_t uuid[16];

0 commit comments

Comments
 (0)