Conversation
Using a template taken from FreeBSD Add a github action to enforce this
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds C/C++ formatting and linting configs (.clang-format, .clang-tidy, .clangd), a GitHub Actions format-check workflow, Makefile targets for format/lint, and applies widespread formatting-only updates across the src tree and some workflow files. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Repo as Repository
participant ClangFmt as clang-format Action
participant ClangTidy as clang-tidy (Make target)
Dev->>Repo: push / open PR
GH->>Repo: workflow triggered (format.yml)
GH->>ClangFmt: run clang-format v19 on `src/**/*.c` `src/**/*.h`
ClangFmt-->>GH: report pass/fail
alt format fails
GH-->>Dev: report formatting failures in PR
else format passes
GH-->>Dev: report success
end
Note over Dev,ClangTidy: Local/CI make targets can run clang-tidy via `make check-format` / `_lint`
Dev->>ClangTidy: invoke `make _lint` (adds -isystem vendor includes)
ClangTidy-->>Dev: lint results
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 31 minutes and 46 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ipv4ll.c (1)
223-239:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winLeak in test path due to overwritten allocation
At Line 225 an
iaallocation is performed forDHCPCD_TEST, but at Line 237iais reassigned fromipv4_addaddr(...)unconditionally, leaking the first allocation.Proposed fix
if (ia == NULL) { if (ifp->ctx->options & DHCPCD_TEST) { - ia = malloc(sizeof(*ia)); + ia = calloc(1, sizeof(*ia)); if (ia == NULL) { logerr(__func__); return; } ia->iface = ifp; ia->addr = state->pickedaddr; - } else if (!(ifp->options->options & DHCPCD_CONFIGURE)) { + } else if (!(ifp->options->options & DHCPCD_CONFIGURE)) { logwarnx("%s: refusing to add IPv4LL address %s", ifp->name, inet_ntoa(state->pickedaddr)); return; + } else { + ia = ipv4_addaddr(ifp, &state->pickedaddr, &inaddr_llmask, + &inaddr_llbcast, DHCP_INFINITE_LIFETIME, + DHCP_INFINITE_LIFETIME); } - ia = ipv4_addaddr(ifp, &state->pickedaddr, &inaddr_llmask, - &inaddr_llbcast, DHCP_INFINITE_LIFETIME, - DHCP_INFINITE_LIFETIME); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ipv4ll.c` around lines 223 - 239, The test-path allocates ia when ifp->ctx->options & DHCPCD_TEST but then unconditionally overwrites ia with the return of ipv4_addaddr(...), leaking the first allocation; fix by changing the control flow so ipv4_addaddr(...) is only called when ia is still NULL (i.e., move or guard the ipv4_addaddr call behind an "if (ia == NULL)" branch) or explicitly free the temporary ia before reassigning; reference symbols: ia, ifp->ctx->options, DHCPCD_TEST, ipv4_addaddr().
🧹 Nitpick comments (2)
.github/workflows/format.yml (1)
7-9: ⚡ Quick winPin GitHub Actions to immutable commit SHAs.
Lines 7 and 9 use mutable tags (
@v5). For CI supply-chain hardening, pin to full commit SHAs (at least for third-party actions).Suggested update
- - uses: actions/checkout@v5 + - uses: actions/checkout@<full_commit_sha> ... - - uses: RafikFarhad/clang-format-github-action@v5 + - uses: RafikFarhad/clang-format-github-action@<full_commit_sha>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/format.yml around lines 7 - 9, Replace mutable action tags with immutable commit SHAs: update the uses entries referencing actions/checkout@v5 and RafikFarhad/clang-format-github-action@v5 to use their corresponding full commit SHA pins (e.g., actions/checkout@<full-sha> and RafikFarhad/clang-format-github-action@<full-sha>), ensuring you fetch the exact commit SHAs from each action's repository and update the two uses lines accordingly.src/Makefile.inc (1)
2-7: ⚡ Quick winHarden formatter file discovery and argument passing.
Line 2 and Line 6 use
find | xargsin a form that can break on paths with spaces/newlines and may pass unintended arguments. Use null-delimited paths and restrict to regular files.Proposed patch
check-format: - find . -iname "*.c" -o -iname "*.h" | \ - xargs clang-format --dry-run --Werror + find . -type f \( -iname "*.c" -o -iname "*.h" \) -print0 | \ + xargs -0 -- clang-format --dry-run --Werror format: - find . -iname "*.c" -o -iname "*.h" | \ - xargs clang-format -i + find . -type f \( -iname "*.c" -o -iname "*.h" \) -print0 | \ + xargs -0 -- clang-format -i🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Makefile.inc` around lines 2 - 7, The find|xargs pipeline in the format and check rules can break on filenames with spaces/newlines and may pass non-regular files; update both occurrences (the rule invoking clang-format --dry-run --Werror and the format: rule invoking clang-format -i) to restrict to regular files and use null-delimited path passing: e.g. use find with -type f and grouped name tests like find . -type f \( -iname "*.c" -o -iname "*.h" \) -print0 piped to xargs -0 clang-format ... (or use find ... -exec clang-format {} +) so filenames with whitespace are handled safely and no unintended args are passed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.clangd:
- Around line 7-9: The PathMatch regex in .clangd is incorrect because it uses a
comma, so replace the current pattern "PathMatch: src/.*\.(c,h)" with a proper
alternation or character class (e.g., use "src/.*\.(c|h)" or "src/.*\.[ch]") so
that the rule matches files ending in .c or .h; update the PathMatch entry
accordingly.
In @.github/workflows/format.yml:
- Line 2: The workflow currently triggers only on the push event (the top-level
on key with value [push]); update the workflow trigger so it also runs on pull
requests by adding the pull_request event to the on configuration (e.g., change
the on array to include pull_request) so formatting checks run for PRs as well.
---
Outside diff comments:
In `@src/ipv4ll.c`:
- Around line 223-239: The test-path allocates ia when ifp->ctx->options &
DHCPCD_TEST but then unconditionally overwrites ia with the return of
ipv4_addaddr(...), leaking the first allocation; fix by changing the control
flow so ipv4_addaddr(...) is only called when ia is still NULL (i.e., move or
guard the ipv4_addaddr call behind an "if (ia == NULL)" branch) or explicitly
free the temporary ia before reassigning; reference symbols: ia,
ifp->ctx->options, DHCPCD_TEST, ipv4_addaddr().
---
Nitpick comments:
In @.github/workflows/format.yml:
- Around line 7-9: Replace mutable action tags with immutable commit SHAs:
update the uses entries referencing actions/checkout@v5 and
RafikFarhad/clang-format-github-action@v5 to use their corresponding full commit
SHA pins (e.g., actions/checkout@<full-sha> and
RafikFarhad/clang-format-github-action@<full-sha>), ensuring you fetch the exact
commit SHAs from each action's repository and update the two uses lines
accordingly.
In `@src/Makefile.inc`:
- Around line 2-7: The find|xargs pipeline in the format and check rules can
break on filenames with spaces/newlines and may pass non-regular files; update
both occurrences (the rule invoking clang-format --dry-run --Werror and the
format: rule invoking clang-format -i) to restrict to regular files and use
null-delimited path passing: e.g. use find with -type f and grouped name tests
like find . -type f \( -iname "*.c" -o -iname "*.h" \) -print0 piped to xargs -0
clang-format ... (or use find ... -exec clang-format {} +) so filenames with
whitespace are handled safely and no unintended args are passed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3794afca-47d4-4c8d-87ae-ee18749b3928
📒 Files selected for processing (66)
.clang-format.clang-tidy.clangd.github/workflows/format.ymlsrc/Makefilesrc/Makefile.incsrc/arp.csrc/arp.hsrc/auth.csrc/auth.hsrc/bpf.csrc/bpf.hsrc/common.csrc/common.hsrc/control.csrc/control.hsrc/defs.hsrc/dev.csrc/dev/udev.csrc/dhcp-common.csrc/dhcp-common.hsrc/dhcp.csrc/dhcp.hsrc/dhcp6.csrc/dhcp6.hsrc/dhcpcd.csrc/dhcpcd.hsrc/duid.csrc/duid.hsrc/if-bsd.csrc/if-linux-wext.csrc/if-linux.csrc/if-options.csrc/if-options.hsrc/if-sun.csrc/if.csrc/if.hsrc/ipv4.csrc/ipv4.hsrc/ipv4ll.csrc/ipv4ll.hsrc/ipv6.csrc/ipv6.hsrc/ipv6nd.csrc/ipv6nd.hsrc/logerr.csrc/logerr.hsrc/privsep-bpf.csrc/privsep-bpf.hsrc/privsep-bsd.csrc/privsep-control.csrc/privsep-control.hsrc/privsep-inet.csrc/privsep-linux.csrc/privsep-root.csrc/privsep-root.hsrc/privsep-sun.csrc/privsep.csrc/privsep.hsrc/queue.hsrc/route.csrc/route.hsrc/sa.csrc/sa.hsrc/script.csrc/script.h
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/build.yml:
- Around line 31-113: Uncomment and restore the openbsd, freebsd, and netbsd
jobs in the GitHub Actions workflow so BSD CI runs again, but make them
non-blocking by adding continue-on-error: true to each job block (openbsd,
freebsd, netbsd) or alternatively move them into a separate scheduled/manual
workflow; after doing this remove the large commented blocks and ensure the
existing job steps (Bootstrap, Install Dependencies, Build, Tests) remain
unchanged so the matrix/steps still execute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c20f046-ec4e-4bce-a544-3f4fbc00be99
📒 Files selected for processing (5)
.clangd.github/workflows/build.yml.github/workflows/format.ymlsrc/Makefile.incsrc/dev/Makefile
✅ Files skipped from review due to trivial changes (3)
- src/Makefile.inc
- .github/workflows/format.yml
- .clangd
Comment on lines
+31
to
+113
| # Sadly the BSDs have stopped building .... | ||
|
|
||
| # openbsd: | ||
| # runs-on: ubuntu-latest | ||
| # steps: | ||
| # - name: Bootstrap OpenBSD-latest | ||
| # uses: mario-campos/emulate@v1 | ||
| # with: | ||
| # operating-system: openbsd-latest | ||
|
|
||
| # - name: Install Dependencies | ||
| # run: pkg_add git | ||
|
|
||
| # - name: Build | ||
| # run: | | ||
| # git clone --depth=1 "${{ github.server_url }}/${{ github.repository }}" build | ||
| # cd build | ||
| # [ "${{ github.event.pull_request.number }}" = "" ] || (echo "fetching PR ${{ github.event.pull_request.number }}"; git fetch origin pull/${{ github.event.pull_request.number }}/head:pr-${{ github.event.pull_request.number }} && git checkout "pr-${{ github.event.pull_request.number }}") | ||
| # ./configure | ||
| # make | ||
|
|
||
| # - name: Tests | ||
| # run: | | ||
| # ulimit -n 1024 | ||
| # cd build | ||
| # make tests | ||
|
|
||
| # freebsd: | ||
| # runs-on: ubuntu-latest | ||
| # steps: | ||
| # - name: Bootstrap FreeBSD-latest | ||
| # uses: mario-campos/emulate@v1 | ||
| # with: | ||
| # operating-system: freebsd-latest | ||
|
|
||
| # - name: Install Dependencies | ||
| # run: pkg install -y git | ||
|
|
||
| # - name: Build | ||
| # run: | | ||
| # git clone --depth=1 "${{ github.server_url }}/${{ github.repository }}" build | ||
| # cd build | ||
| # [ "${{ github.event.pull_request.number }}" = "" ] || (echo "fetching PR ${{ github.event.pull_request.number }}"; git fetch origin pull/${{ github.event.pull_request.number }}/head:pr-${{ github.event.pull_request.number }} && git checkout "pr-${{ github.event.pull_request.number }}") | ||
| # ./configure | ||
| # make | ||
|
|
||
| # - name: Tests | ||
| # run: | | ||
| # cd build | ||
| # make tests | ||
|
|
||
| # netbsd: | ||
| # strategy: | ||
| # matrix: | ||
| # args: | ||
| # - | ||
| # - --disable-ipv4 | ||
| # - --disable-arp | ||
| # - --disable-ipv4ll | ||
| # - --disable-ipv6 | ||
| # - --disable-dhcp6 | ||
| # cppflags: | ||
| # - | ||
| # - -DSMALL | ||
| # runs-on: ubuntu-latest | ||
| # steps: | ||
| # - name: Bootstrap NetBSD-latest | ||
| # uses: mario-campos/emulate@v1 | ||
| # with: | ||
| # operating-system: netbsd-latest | ||
|
|
||
| # - name: Build | ||
| # run: | | ||
| # git clone --depth=1 "${{ github.server_url }}/${{ github.repository }}" build | ||
| # cd build | ||
| # [ "${{ github.event.pull_request.number }}" = "" ] || (echo "fetching PR ${{ github.event.pull_request.number }}"; git fetch origin pull/${{ github.event.pull_request.number }}/head:pr-${{ github.event.pull_request.number }} && git checkout "pr-${{ github.event.pull_request.number }}") | ||
| # CFLAGS=-Werror CPPFLAGS="${{ matrix.cppflags }}" ./configure ${{ matrix.args }} | ||
| # make | ||
|
|
||
| # - name: Tests | ||
| # run: | | ||
| # cd build | ||
| # make tests |
There was a problem hiding this comment.
Re-enable BSD CI as non-blocking instead of fully commenting it out.
Commenting out openbsd, freebsd, and netbsd removes all BSD regression signal; from this point, BSD breakage can merge unnoticed. Keep these jobs runnable but non-blocking (continue-on-error: true) and/or move them to a scheduled/manual workflow if they’re currently flaky.
Suggested direction
+ openbsd:
+ continue-on-error: true
+ if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
+ runs-on: ubuntu-latest
+ steps:
+ # existing steps...Apply the same pattern to freebsd and netbsd, and remove the large commented blocks once moved.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/build.yml around lines 31 - 113, Uncomment and restore the
openbsd, freebsd, and netbsd jobs in the GitHub Actions workflow so BSD CI runs
again, but make them non-blocking by adding continue-on-error: true to each job
block (openbsd, freebsd, netbsd) or alternatively move them into a separate
scheduled/manual workflow; after doing this remove the large commented blocks
and ensure the existing job steps (Bootstrap, Install Dependencies, Build,
Tests) remain unchanged so the matrix/steps still execute.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Using a template taken from FreeBSD
Add a github action to enforce this