Skip to content

Preserve circular buffer item semantics - #93

Open
mjc wants to merge 2 commits into
lukash:mainfrom
mjc:refloat-fix-circular-buffer
Open

Preserve circular buffer item semantics#93
mjc wants to merge 2 commits into
lukash:mainfrom
mjc:refloat-fix-circular-buffer

Conversation

@mjc

@mjc mjc commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Fix two independent circular-buffer behaviors.

circular_buffer_iterate() previously used an item index as a byte offset. For buffers containing multi-byte records, callbacks after the first could therefore receive pointers inside another record. The index is now multiplied by item_size so each callback receives a complete item. This fixes the experimental recorder plotting path, which iterates over Sample records.

Following review feedback, circular_buffer_pop() now implements a conventional FIFO pop: its unused index argument was removed, it copies the item at the tail, and then advances the tail. There are currently no callers requiring migration.

The defensive argument guards and indexed-item shifting from earlier revisions were removed following review feedback.

Copilot AI review requested due to automatic review settings July 15, 2026 17:17

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the circular buffer implementation so that operations consistently treat the underlying storage as item-sized records (not bytes), improving correctness for non-item_size == 1 buffers while keeping FIFO behavior unchanged.

Changes:

  • Added input validation in circular_buffer_push / circular_buffer_get to ignore invalid buffer/item pointers and zero-sized configurations.
  • Fixed indexed removal (circular_buffer_pop) to shift whole items to preserve item semantics.
  • Fixed iteration to pass item-aligned pointers to callbacks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/lib/circular_buffer.c
Comment thread src/lib/circular_buffer.c Outdated
Comment thread src/lib/circular_buffer.c Outdated
@mjc
mjc force-pushed the refloat-fix-circular-buffer branch 3 times, most recently from 59bfc5f to ab1fb35 Compare July 25, 2026 21:26
@lukash

lukash commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Minor, the commit title "Circular buffer: Remove indexed items" doesn't make much sense, could you improve it? Besides that looks good now.

@mjc
mjc force-pushed the refloat-fix-circular-buffer branch from ab1fb35 to 2cd1ed0 Compare July 28, 2026 17:51
@mjc

mjc commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

should be all set now. sorry about the amount of churn, doing my best

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.

3 participants