Skip to content

ARP: Iterate over states safely as the cb could remove ours - #656

Merged
rsmarples merged 1 commit into
masterfrom
arp
Jun 21, 2026
Merged

ARP: Iterate over states safely as the cb could remove ours#656
rsmarples merged 1 commit into
masterfrom
arp

Conversation

@rsmarples

Copy link
Copy Markdown
Member

Reported by NVIDIA Project Vanessa

@coderabbitai

coderabbitai Bot commented Jun 21, 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

Run ID: a4d444d2-0395-46a1-ba15-c025f3ca5f9f

📥 Commits

Reviewing files that changed from the base of the PR and between e636ba2 and 32604bd.

📒 Files selected for processing (1)
  • src/arp.c

Walkthrough

In src/arp.c, the arp_announce() function's loop over ARP states is changed from TAILQ_FOREACH to TAILQ_FOREACH_SAFE with an added temporary pointer an, preventing iterator invalidation if arp_announced() frees the current state during iteration.

Changes

Safe ARP state iteration in arp_announce()

Layer / File(s) Summary
TAILQ_FOREACH_SAFE fix
src/arp.c
Introduces the an pointer and switches to TAILQ_FOREACH_SAFE so the loop remains valid even when arp_announced(a2) triggers removal or freeing of the current ARP state entry.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

🚥 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: switching from unsafe TAILQ_FOREACH to TAILQ_FOREACH_SAFE iteration to prevent crashes when callbacks remove states during iteration.
Description check ✅ Passed The description references the reporting source (NVIDIA Project Vanessa) which contextualizes the fix, relating it to the actual safety issue being addressed in the changeset.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 arp

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 and usage tips.

@rsmarples
rsmarples merged commit 1199f7f into master Jun 21, 2026
6 checks passed
@rsmarples
rsmarples deleted the arp branch June 21, 2026 11:15
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