Skip to content

Fix obsolete cnt assignment in _tu_fifo_peek() overflow check#3230

Merged
hathach merged 2 commits into
masterfrom
copilot/fix-3218
Sep 4, 2025
Merged

Fix obsolete cnt assignment in _tu_fifo_peek() overflow check#3230
hathach merged 2 commits into
masterfrom
copilot/fix-3218

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 3, 2025

This PR fixes a logic error identified by static analysis in the _tu_fifo_peek() function where the cnt variable was being assigned but never used.

Issue Analysis:
In the overflow check at lines 427-432 of src/common/tusb_fifo.c, the code was setting cnt = f->depth but this value was never used afterward:

// Check overflow and correct if required
if ( cnt > f->depth )
{
  rd_idx = _ff_correct_read_index(f, wr_idx);
  cnt = f->depth;  // This assignment was never used
}

Comparison with similar functions:

  • _tu_fifo_peek_n() uses the corrected cnt value to limit the number of items read: if ( cnt < n ) n = cnt;
  • tu_fifo_get_read_info() uses the corrected cnt value to set buffer info: info->len_lin = cnt;
  • _tu_fifo_peek() only peeks at a single item and doesn't use cnt after the overflow check

Solution:
Removed the obsolete cnt = f->depth; assignment while preserving the necessary read index correction (rd_idx = _ff_correct_read_index(f, wr_idx);).

Testing:
All existing FIFO tests continue to pass (14/14) and the full unit test suite passes (21/21), confirming the change doesn't affect functionality.

Fixes #3218.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 3, 2025

@hathach 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Co-authored-by: hathach <249515+hathach@users.noreply.github.com>
Copilot AI changed the title [WIP] Possible logic error with overflow check in _tu_fifo_peek() Fix obsolete cnt assignment in _tu_fifo_peek() overflow check Sep 3, 2025
Copilot AI requested a review from hathach September 3, 2025 10:43
@hathach hathach marked this pull request as ready for review September 4, 2025 05:33
Copilot AI review requested due to automatic review settings September 4, 2025 05:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 fixes a logic error in the _tu_fifo_peek() function where a variable assignment in the overflow check was obsolete and never used, as identified by static analysis.

  • Removes the unused cnt = f->depth; assignment from the overflow correction block
  • Preserves the necessary read index correction functionality
  • Aligns the function behavior with its single-item peek purpose

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Copy Markdown
Owner

@hathach hathach left a comment

Choose a reason for hiding this comment

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

good

@hathach hathach merged commit 93487de into master Sep 4, 2025
192 of 197 checks passed
@hathach hathach deleted the copilot/fix-3218 branch September 4, 2025 06:11
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.

Possible logic error with overflow check in _tu_fifo_peek()

3 participants