Skip to content

Conversation

@GabeMillikan
Copy link
Contributor

Right now, this psuedocode outputs false:

buffer = GR_CircularBuffer_Create(5);
GR_CircularBuffer_Pop(buffer_ptr);
empty = GR_CircularBuffer_IsEmpty(buffer_ptr)
print(empty) // outputs false

i.e., it thinks that an empty buffer is not empty. This is because Pop() always increments head++ even if there is nothing to pop.

I added a simple check to fix this, as well as a test to prevent regression. I verified that the test fails without my addition, and all tests succeed with it.

@GabeMillikan GabeMillikan added the Bug Something is doing a thing but doing it wrong or otherwise incorrectly label Nov 13, 2025
@GabeMillikan GabeMillikan marked this pull request as ready for review November 13, 2025 05:33
@dchansen06 dchansen06 requested a review from XuChe777 November 13, 2025 09:04
@dchansen06 dchansen06 added Enhancement New feature or request 2 PRIORITY Important and a priority, but less than URGENT Small Fry Something that is small, could include bug fixes or smaller changes Peripheral Related to or involving a peripheral including abstractions labels Nov 13, 2025
@dchansen06 dchansen06 added this to the GR26 milestone Nov 13, 2025
Copy link
Contributor

@XuChe777 XuChe777 left a comment

Choose a reason for hiding this comment

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

Good Catch! Looks good to me.

Copy link
Contributor

@dchansen06 dchansen06 left a comment

Choose a reason for hiding this comment

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

Thanks for catching!

@XuChe777 XuChe777 merged commit 96c3c37 into main Nov 14, 2025
1 check passed
@XuChe777 XuChe777 deleted the gabe.fix-circularBuffer-pop-nothing branch November 14, 2025 03:07
dchansen06 pushed a commit that referenced this pull request Dec 4, 2025
Right now, this psuedocode outputs `false`:
```c
buffer = GR_CircularBuffer_Create(5);
GR_CircularBuffer_Pop(buffer_ptr);
empty = GR_CircularBuffer_IsEmpty(buffer_ptr)
print(empty) // outputs false
```
i.e., it thinks that an empty buffer is not empty. This is because
`Pop()` always increments `head++` even if there is nothing to pop.

I added a simple check to fix this, as well as a test to prevent
regression. I verified that the test fails without my addition, and all
tests succeed with it.

---------

Co-authored-by: Thomas Xu <thomas.xu.7@outlook.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2 PRIORITY Important and a priority, but less than URGENT Bug Something is doing a thing but doing it wrong or otherwise incorrectly Enhancement New feature or request Peripheral Related to or involving a peripheral including abstractions Small Fry Something that is small, could include bug fixes or smaller changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants