Skip to content

Handshake fragments assembly refactoring#762

Merged
JoTurk merged 4 commits intopion:masterfrom
trs00:fragments-assembly-refactoring
Dec 18, 2025
Merged

Handshake fragments assembly refactoring#762
JoTurk merged 4 commits intopion:masterfrom
trs00:fragments-assembly-refactoring

Conversation

@trs00
Copy link
Copy Markdown
Contributor

@trs00 trs00 commented Dec 15, 2025

  • Determining the end of the fragment by looking at the fragment length, rather than handshake header's length
  • Using totalBufferSize to avoid walks the linked list to determine the total buffer size
  • Using totalFragmentCount to avoid list grow to an unbounded size with 0-length fragments
  • Removing the need to try to reassemble(by recursively calling a function for each fragment) on every new packet

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 15, 2025

Codecov Report

❌ Patch coverage is 97.61905% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.26%. Comparing base (ab5f89b) to head (963b7c8).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
fragment_buffer.go 97.61% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #762      +/-   ##
==========================================
+ Coverage   81.16%   81.26%   +0.09%     
==========================================
  Files         101      101              
  Lines        5597     5603       +6     
==========================================
+ Hits         4543     4553      +10     
+ Misses        678      676       -2     
+ Partials      376      374       -2     
Flag Coverage Δ
go 81.26% <97.61%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Sean-Der
Copy link
Copy Markdown
Member

Thank you so much @trs00! Changes look great to me.

Would you be able/interested in fixing the lints? If you don't have time I am happy to push to your branch directly :)

@trs00
Copy link
Copy Markdown
Contributor Author

trs00 commented Dec 16, 2025

@Sean-Der thanks a lot for looking into PR, I pushed lint fixes

Copy link
Copy Markdown
Member

@JoTurk JoTurk left a comment

Choose a reason for hiding this comment

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

thank you.

@trs00
Copy link
Copy Markdown
Contributor Author

trs00 commented Dec 18, 2025

@joeturki may I ask if merge option is not available due to access restrictions or it is necessary to have 100% Patch coverage?

@JoTurk JoTurk merged commit 7b9612e into pion:master Dec 18, 2025
19 checks passed
@trs00 trs00 deleted the fragments-assembly-refactoring branch December 18, 2025 19:39
@JoTurk
Copy link
Copy Markdown
Member

JoTurk commented Dec 18, 2025

@trs00 Hello, I squash merged, If you plan to make more changes, I can send you an invite to the org so you can make branches directly, and merge PRs after approval :)

Thank you a lot.

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.

4 participants