Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ringbuf [nosplit] bugfixes (IDFGH-5649) #7371

Closed
wants to merge 1 commit into from

Conversation

bugadani
Copy link
Contributor

@bugadani bugadani commented Aug 5, 2021

This PR only affects NOSPLIT ring buffers.

Fix tracking free space. Fix a number of potential underflow issues, as well as a few incorrect comments. Don't round up data size for the last chunk.

Fixes #7344

Fix tracking free space. Fix a number of potential underflow issues, as well as a few incorrect comments. Don't round up data size for the last chunk.

Fixes espressif#7344
@espressif-bot espressif-bot added the Status: Opened Issue is new label Aug 5, 2021
@github-actions github-actions bot changed the title Ringbuf [nosplit] bugfixes Ringbuf [nosplit] bugfixes (IDFGH-5649) Aug 5, 2021
@Alvin1Zhang
Copy link
Collaborator

Thanks for your contribution.

@Dazza0 Dazza0 self-requested a review September 28, 2021 03:23
@Dazza0 Dazza0 self-assigned this Sep 28, 2021
@Dazza0
Copy link
Collaborator

Dazza0 commented Sep 28, 2021

Hi @bugadani , thank you for your contribution. But I don't think these changes tackle the source of the bug (see explanation in #7344 (comment)). I'll push a fix for this bug shortly.

@espressif-bot espressif-bot added Status: In Progress Work is in progress and removed Status: Opened Issue is new labels Sep 28, 2021
@bugadani bugadani closed this Oct 4, 2021
@espressif-bot espressif-bot added Resolution: Won't Do This will not be worked on Status: Done Issue is done internally and removed Status: In Progress Work is in progress labels Oct 22, 2021
espressif-bot pushed a commit that referenced this pull request Nov 3, 2021
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
espressif-bot pushed a commit that referenced this pull request Dec 9, 2021
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
espressif-bot pushed a commit that referenced this pull request Dec 10, 2021
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
espressif-bot pushed a commit that referenced this pull request Dec 31, 2021
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
espressif-bot pushed a commit that referenced this pull request Jan 1, 2022
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
espressif-bot pushed a commit that referenced this pull request Jan 6, 2022
…and resulted in incorrect free size for no-split/allow-split buffers

This commit fixes a bug in no-split and allow-split ring buffers free buffer size calculation.
When the free size available in the buffers less than the size of one item header,
the function prvGetCurMaxSizeNoSplit/AllowSplit() incorrectly returned the maxItemSize instead of 0.
This is due to the comparision between a negative and a positive value
where both operands are treated as unsigned during the comparision operation,
thereby treating the negative operand as a large integer.

Also added new unit tests to test buffer-full and almost-full conditions
where this scenario is likely to be hit.

Closes #7344
Closes #7371
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Resolution: Won't Do This will not be worked on Status: Done Issue is done internally
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ringbuffer timeouts in certain cases (IDFGH-5624)
4 participants