Skip to content

test: ping has extra delay at the end#4862

Open
jiceathome wants to merge 4 commits into
scionproto:masterfrom
jiceathome:fix4861
Open

test: ping has extra delay at the end#4862
jiceathome wants to merge 4 commits into
scionproto:masterfrom
jiceathome:fix4861

Conversation

@jiceathome

Copy link
Copy Markdown
Contributor

Fixes: #4861

@jiceathome jiceathome changed the title Fix4861 test: ping has extra delay at the end Dec 25, 2025
@oncilla oncilla requested a review from Copilot December 29, 2025 12:28

Copilot AI left a comment

Copy link
Copy Markdown

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 an issue where the ping function had an extra delay at the end of its execution. The change modifies the loop structure to avoid waiting for a final tick after all ping attempts have been sent.

Key changes:

  • Restructured the ping sending loop to break before the final sleep/tick wait
  • Changed from a traditional for loop to a manual counter decrement approach

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scion/ping/ping.go
defer log.HandlePanic()
defer wg.Done()
for i := uint16(0); i < p.attempts; i++ {

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

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

If p.attempts is 0, the loop will run indefinitely since i will underflow when decremented. Add a check to handle the case where p.attempts is 0, or ensure p.attempts is always greater than 0.

Suggested change
if p.attempts == 0 {
// No packets to send; avoid underflow in the decrementing loop.
return
}

Copilot uses AI. Check for mistakes.
Comment thread scion/ping/ping.go
defer wg.Done()
for i := uint16(0); i < p.attempts; i++ {

i := p.attempts

Copilot AI Dec 29, 2025

Copy link

Choose a reason for hiding this comment

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

The variable i should retain the uint16 type annotation for clarity and to match the original type, e.g., i := uint16(p.attempts) or ensure type consistency with the original loop counter.

Copilot uses AI. Check for mistakes.
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.

test: ping has unnecessary delay

2 participants