Skip to content

[Tftp] Add ntohs() to the pack->info.block then get the right block number - #112

Open
lvtaolong wants to merge 2 commits into
RT-Thread-packages:masterfrom
lvtaolong:master
Open

[Tftp] Add ntohs() to the pack->info.block then get the right block number#112
lvtaolong wants to merge 2 commits into
RT-Thread-packages:masterfrom
lvtaolong:master

Conversation

@lvtaolong

@lvtaolong lvtaolong commented Sep 11, 2026

Copy link
Copy Markdown

In tftp_read_data(struct tftp_xfer *xfer, struct tftp_packet *pack, int len) , to check the block number whether is expected here:
else if ((_private->block + 1) != pack->info.block)
but because the pack->info.block is from network package, so to get the right block number , there need to use ntohs(pack->info.block)

Summary by CodeRabbit

  • Bug Fixes
    • Improved TFTP data transfer reliability by correctly interpreting received block numbers.
    • Error messages now display the received block number in a readable format.

to get the correct block number in received network package, there need to use ntohs(pack->info.block)
Add ntohs() to the pack->info.block then get the right block number
@coderabbitai

coderabbitai Bot commented Sep 11, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 29563264-964e-433b-9ab0-713eb65d4ce8

📥 Commits

Reviewing files that changed from the base of the PR and between b77ba3f and 193a20b.

📒 Files selected for processing (1)
  • tftp/tftp_xfer.c

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


Walkthrough

The TFTP data reader now converts the received block number from network byte order before comparing it with the expected block and printing the mismatch error. No other control flow or error handling changes.

Changes

TFTP block validation

Layer / File(s) Summary
Decode received block numbers
tftp/tftp_xfer.c
tftp_read_data applies ntohs() to the received block number for mismatch validation and error reporting.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Merge Risk: ⚪ Minimal · up to 193a2

This change correctly normalizes received TFTP block numbers before validation, with no remaining merge-blocking risk.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the main change: applying ntohs() to pack->info.block to obtain the correct TFTP block number. The wording is slightly awkward but remains specific and understandable.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@lvtaolong lvtaolong changed the title Add ntohs() to the pack->info.block then get the right block number [Tftp] Add ntohs() to the pack->info.block then get the right block number Sep 11, 2026
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.

1 participant