Skip to content

fix(peer): GetNumberOfAddresses() always returned MAXIMUM_NUMBER_OF_INTERNAL_IDS - #43

Merged
Segfaultd merged 1 commit into
masterfrom
fix/get-number-of-addresses
Aug 19, 2026
Merged

fix(peer): GetNumberOfAddresses() always returned MAXIMUM_NUMBER_OF_INTERNAL_IDS#43
Segfaultd merged 1 commit into
masterfrom
fix/get-number-of-addresses

Conversation

@Segfaultd

@Segfaultd Segfaultd commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

RakPeer::GetNumberOfAddresses() always returned MAXIMUM_NUMBER_OF_INTERNAL_IDS (10) instead of the actual number of local addresses. The loop's unassigned-entry check was dead code — the loop condition already excluded unassigned entries, so hitting the end of the real address list exited the loop and fell through to the max-value return. Callers iterating GetLocalIP(0..count-1) then printed UNASSIGNED_SYSTEM_ADDRESS garbage entries, and IsLocalIP() compared against those garbage slots too.

Inherited from SLikeNet, where it was introduced when the original RakNet counting loop was rewritten: SLikeSoft#55.

Fix

Count leading assigned entries and return that count (Source/src/RakPeer.cpp).

Tests

New hermetic unit tests in Tests/Unit/LocalAddressTests.cpp assert that every index below the returned count is a real address and that the entry at the count boundary is unassigned, both before Startup() and on a started-but-unconnected peer.

  • Written TDD-style: failed against the old code (returned 10 on a machine with 6 addresses), passes with the fix, 10/10 --gtest_repeat runs.
  • Full unit suite: 119/119 passed.
  • Full integration suite: 32/32 passed.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected local address counting so only contiguous assigned internal addresses are included.
    • Ensured counting stops at the first unassigned address.
  • Tests

    • Added coverage for address counts before startup and after starting an unconnected peer.
    • Added validation for assigned entries, count limits, and the unassigned boundary.

…NTERNAL_IDS

The loop's unassigned-entry check was dead code: the loop condition
already excluded unassigned entries, so reaching the end of the real
address list exited the loop and fell through to returning
MAXIMUM_NUMBER_OF_INTERNAL_IDS (10) regardless of how many local
addresses exist. Callers iterating GetLocalIP(0..count-1) then printed
UNASSIGNED_SYSTEM_ADDRESS garbage entries.

Inherited from SLikeNet (SLikeSoft#55).
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bea8db29-7738-4a52-9fc4-80fee4dddfd0

📥 Commits

Reviewing files that changed from the base of the PR and between deee616 and ff28827.

📒 Files selected for processing (2)
  • Source/src/RakPeer.cpp
  • Tests/Unit/LocalAddressTests.cpp

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


Walkthrough

GetNumberOfAddresses() now returns the number of contiguous assigned internal IP entries. Unit tests verify this behavior before startup and after starting an unconnected peer.

Changes

Local address count

Layer / File(s) Summary
Address count implementation and validation
Source/src/RakPeer.cpp, Tests/Unit/LocalAddressTests.cpp
GetNumberOfAddresses() counts assigned ipList entries and stops at the first unassigned entry. Tests verify address counts, assigned entries, the unassigned boundary, startup, shutdown, and cleanup.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to ff288

The PR makes GetNumberOfAddresses() return the actual number of assigned local addresses, preventing callers from iterating into unassigned entries; the reported test suites pass, and no actionable merge-blocking risk remains.

Poem

A rabbit counts addresses in a row,
Stops where empty markers show.
Before startup, then after too,
Tests check each entry true.
Hop, hop—the count is clear!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the fix for GetNumberOfAddresses() returning the maximum count instead of the actual address count.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/get-number-of-addresses

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.

@Segfaultd
Segfaultd merged commit 0f6179b into master Aug 19, 2026
4 of 6 checks passed
@Segfaultd
Segfaultd deleted the fix/get-number-of-addresses branch August 19, 2026 19:59
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