Skip to content

lib: buffer - Annotate w_buffer with sized_by(alloc) - #318

Open
LaptopsPlural wants to merge 2 commits into
dovecot:mainfrom
LaptopsPlural:buffer-w-buffer-sized-by
Open

LaptopsPlural wants to merge 2 commits into
dovecot:mainfrom
LaptopsPlural:buffer-w-buffer-sized-by

Conversation

@LaptopsPlural

Copy link
Copy Markdown

Summary

real_buffer.w_buffer is the private writable allocation whose capacity is tracked by alloc. Annotate that relationship with sized_by(alloc) when the compiler provides the attribute (no-op otherwise), and in buffer_alloc() assign the new alloc before publishing the new pointer so the bound remains consistent while the buffer grows.

No change to public buffer_t layout, writable_size/NUL reservation, or unsafe space helpers.

AI-assisted disclosure

This change was prepared with AI assistance. I reviewed the diff for correctness and scope before opening this PR.

Tie private real_buffer.w_buffer to alloc via sized_by when the
compiler supports it, and publish alloc before updating the pointer
in buffer_alloc() so the bound stays sound across growth.
Comment thread src/lib/buffer.c Outdated
#if defined(__has_attribute) && __has_attribute(sized_by)
# define ATTR_SIZED_BY(member) __attribute__((sized_by(member)))
#elif defined(__has_attribute) && __has_attribute(__sized_by__)
# define ATTR_SIZED_BY(member) __attribute__((__sized_by__(member)))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably not useful to check for __sized_by__?

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.

Agreed — dropped the __sized_by__ spelling check. We now only gate on __has_attribute(sized_by) (empty macro otherwise).

@sirainen

Copy link
Copy Markdown
Contributor

Should we add this to our malloc() variants?

@LaptopsPlural

Copy link
Copy Markdown
Author

Thanks for the review.

On malloc variants: I think yes as a follow-up, not in this PR. This change is intentionally one concern — private w_bufferalloc plus the capacity-first publish order in buffer_alloc() so the attribute stays sound. Spreading sized_by onto the pool/p_malloc helpers here would widen the surface (and the review) without being required for this buffer.

Happy to open a follow-up that annotates the malloc variants once this lands, if that direction still looks useful to you.

@LaptopsPlural

Copy link
Copy Markdown
Author

Thanks again for the malloc-variants question.

I looked at folding that into this tip. The allocator APIs (i_malloc / p_malloc / t_*) are not a sized_by fit — that attribute is for ptr+capacity struct members, and those entry points would be a different (alloc_size) surface across several headers. The nearest internal companions (e.g. allocfree pool_block.block) are interior pointers into the same allocation, which I would rather not annotate here.

Prefer keeping this PR buffer-only (w_bufferalloc, capacity-first publish). Please confirm whether you want a specific malloc-side site folded into this PR, or are fine with a follow-up once this lands. Re-requesting review on the current tip either way.

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