generate-accessors: add lifecycle, defaults, and annotation redesign#3283
generate-accessors: add lifecycle, defaults, and annotation redesign#3283martin-belanger wants to merge 1 commit intolinux-nvme:masterfrom
Conversation
martin-belanger
force-pushed
the
generate-lifecycle
branch
from
1fc0236 to
97aeff5
Compare
martin-belanger
changed the title
martin-belanger
force-pushed
the
generate-lifecycle
branch
from
97aeff5 to
8c4a0bf
Compare
Extend the accessor generator with three new capabilities.
1. Lifecycle (constructor + destructor): annotate a struct's opening brace
with //!generate-lifecycle to generate foo_new() and foo_free().
foo_new() allocates a zeroed instance with calloc() and returns
-EINVAL / -ENOMEM on error. foo_free() frees all owned char* and char**
members then frees the struct. Passing NULL to foo_free() is safe:
destructors that dereference members guard with if (!p) return; those
with no members to dereference rely on free(NULL) being a no-op.
//!lifecycle:none on a member excludes it from the destructor. const
char* members are never freed (assumed externally owned).
2. Defaults (init function): annotate individual members with
//!default:VALUE to generate foo_init_defaults(), which assigns each
annotated field its compile-time default. Any valid C expression is
accepted as the value. When combined with //!generate-lifecycle,
foo_new() calls foo_init_defaults() after allocation. foo_init_defaults()
is also useful standalone to re-initialise a struct without reallocating.
3. Annotation style: drop support for the /*!annotation*/ block-comment
style. Annotations now use only the // line-comment style. The parser
was redesigned accordingly: after //, each !keyword token is treated as
a command, so multiple annotations may share one comment:
struct foo { //!generate-accessors !generate-lifecycle
private.h and private-fabrics.h are updated throughout to use the new
style.
The generated .c files now include <errno.h> for EINVAL/ENOMEM.
Apply these features to struct libnvmf_discovery_args, replacing the
manually-written libnvmf_discovery_args_create() and
libnvmf_discovery_args_free() with generated equivalents. The
constructor is renamed _new (consistent with the generated naming
convention).
Signed-off-by: Martin Belanger <[email protected]>
Assisted-by: Claude Sonnet 4.6 <[email protected]>
martin-belanger
force-pushed
the
generate-lifecycle
branch
from
8c4a0bf to
3fdc002
Compare
| }; | ||
|
|
||
| struct libnvme_path { /*!generate-accessors*/ | ||
| struct libnvme_path { //!generate-accessors |
There was a problem hiding this comment.
Could you move these changes into a separate patch? Thanks!
And while at it an extra space wouldn't hurt // !generate-accessors
| libnvmf_discovery; | ||
| libnvmf_discovery_args_create; | ||
| libnvmf_discovery_args_free; | ||
|
|
There was a problem hiding this comment.
Looks like an unnecessary empty line
| } | ||
|
|
||
| ret = libnvmf_discovery_args_create(&args); | ||
| ret = libnvmf_discovery_args_new(&args); |
There was a problem hiding this comment.
Should we look into making this consistent through out the code base? Having two types of wording is not good IMO.
There was a problem hiding this comment.
As per your text message. I will have a separate patch to change all _create to _new.
|
The general feature looks fine as far as I can tell. The Python code is slowly turning into a bit of a monster, but I don’t care too much about that. There may come a point where we need to start from scratch. Apparently, that’s just how things go. So the important part is getting the requirements right. We’ve already added quite a few features beyond the initial simple design, and I think it really helps avoid writing annoying boilerplate code. In short, I’m fine with this, just please try to split the changes into smaller patches. |
|
Closing this PR. Will submit a fresh one with incremental commits instead of everything in a single commit. |
2 participants
Extend the accessor generator with three new capabilities.
Lifecycle (constructor + destructor): annotate a struct's opening brace with
//!generate-lifecycleto generatefoo_new()andfoo_free().foo_new()allocates a zeroed instance withcalloc()and returns-EINVAL/-ENOMEMon error.foo_free()frees all ownedchar*andchar**members then frees the struct. PassingNULLtofoo_free()is safe: destructors that dereference members guard withif (!p) return;those with no members to dereference rely onfree(NULL)being a no-op.//!lifecycle:noneon a member excludes it from the destructor.const char*members are never freed (assumed externally owned).Defaults (init function): annotate individual members with
//!default:VALUEto generatefoo_init_defaults(), which assigns each annotated field its compile-time default. Any valid C expression is accepted as the value. When combined with//!generate-lifecycle,foo_new()callsfoo_init_defaults()after allocation.foo_init_defaults()is also useful standalone to re-initialise a struct without reallocating.Annotation style: drop support for the
/*!annotation*/block-comment style. Annotations now use only the//line-comment style. The parser was redesigned accordingly: after//, each!keywordtoken is treated as a command, so multiple annotations may share one comment:private.handprivate-fabrics.hare updated throughout to use the new style.The generated
.cfiles now include<errno.h>forEINVAL/ENOMEM.Apply these features to
struct libnvmf_discovery_args, replacing the manually-writtenlibnvmf_discovery_args_create()andlibnvmf_discovery_args_free()with generated equivalents. The constructor is renamed_new(consistent with the generated naming convention).