Skip to content

Conversation

@rsmarples
Copy link
Member

ldop itself cannot be non NULL as it points to the location. but *ldop CAN be NULL.

Fixes #567.

ldop itself cannot be non NULL as it points to the location.
but *ldop CAN be NULL.

Fixes #567.
@coderabbitai
Copy link

coderabbitai bot commented Dec 14, 2025

Walkthrough

A pointer dereference condition in src/if-options.c was modified from checking if the pointer itself is non-NULL to checking if the value it points to is non-NULL, affecting the control flow for assigning embedded options after define/encap operations when no existing embedded option exists.

Changes

Cohort / File(s) Summary
NULL pointer dereference fix
src/if-options.c
Modified pointer null-check condition from ldop to *ldop in embedded options branching logic to prevent dereferencing a NULL pointer target

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

  • Single-line conditional modification in a well-defined branch
  • Fix directly addresses the NULL pointer dereference reported in the linked issue
  • No changes to function signatures, exported interfaces, or complex logic patterns

Pre-merge checks and finishing touches

❌ 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%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the primary change: preventing NULL pointer dereference of the dereferenced ldop pointer in the if-options.c file.
Description check ✅ Passed The description explains the distinction between ldop pointer validity and dereferenced value validity, and references the linked issue #567 being fixed.
Linked Issues check ✅ Passed The code change directly addresses issue #567 by changing the condition from checking if ldop is non-NULL to checking if *ldop is non-NULL, preventing NULL pointer dereference of the dereferenced value.
Out of Scope Changes check ✅ Passed The change is narrowly scoped to fixing the NULL dereference issue in the embed/define branching logic, with no additional unrelated modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch if-options-null-deref

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2de751b and f2a7340.

📒 Files selected for processing (1)
  • src/if-options.c (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
  • GitHub Check: netbsd (--disable-dhcp6)
  • GitHub Check: netbsd (--disable-ipv6)
  • GitHub Check: netbsd (--disable-dhcp6, -DSMALL)
  • GitHub Check: netbsd (-DSMALL)
  • GitHub Check: netbsd (--disable-ipv4ll)
  • GitHub Check: netbsd (--disable-ipv4ll, -DSMALL)
  • GitHub Check: netbsd (--disable-arp, -DSMALL)
  • GitHub Check: netbsd
  • GitHub Check: netbsd (--disable-ipv4)
  • GitHub Check: netbsd (--disable-ipv6, -DSMALL)
  • GitHub Check: netbsd (--disable-ipv4, -DSMALL)
  • GitHub Check: netbsd (--disable-arp)
  • GitHub Check: openbsd
  • GitHub Check: freebsd
🔇 Additional comments (1)
src/if-options.c (1)

1885-1887: LGTM! Critical NULL dereference fix.

The change correctly addresses the NULL pointer dereference reported in issue #567. Since ldop is a double-pointer (struct dhcp_opt **), the parameter itself cannot be NULL, but the value it points to (*ldop) can be (as set at line 1878). The old condition if (ldop) was always true and failed to guard the dereferences at lines 1886-1887. The corrected condition if (*ldop) properly validates that the pointed-to value is non-NULL before accessing (*ldop)->embopts.


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.

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.

dhcpcd/src/if-options.c NULL Pointer Dereference in parse_option

2 participants