ci: add missing check target to CI build loops#218
Conversation
kill() is a non-blocking system call that cannot be interrupted by signals, so it never returns EINTR per POSIX. Remove the do/while loop and call kill() directly to simplify the code.
Both clear_list() and destroy_list() manually reset first, last, and count after freeing nodes. Replace the manual reset with a call to init_list() which performs the equivalent operation via memset, reducing code duplication and ensuring consistency if the struct layout changes in the future.
In C, any object pointer type implicitly converts to void * when passed to free(). The explicit cast adds visual noise without any semantic benefit.
access() is a non-blocking filesystem metadata operation that is not specified to return EINTR per POSIX. Remove the do/while loop and call access() directly, also removing the now-unused access_result variable.
check target to CI build loops
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2875d051fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -B build | ||
| cmake --build build | ||
| cmake --build build --target check |
There was a problem hiding this comment.
In the Linux/macOS matrix this new target now requires the check toolchain, but the job only installs gcc-multilib/libc6-dev-i386 for Ubuntu and has no macOS install step for cppcheck or clang-tidy; cmake/RunCheck.cmake exits fatally when either executable is missing. The current hosted macOS images do not list these tools, and Ubuntu does not list cppcheck, so these matrix entries can fail before reaching the tests unless the workflow installs/pins the analyzers before this line.
Useful? React with 👍 / 👎.
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -B build | ||
| cmake --build build | ||
| cmake --build build --target check |
There was a problem hiding this comment.
The FreeBSD job now invokes the check target, but its prepare step installs only gcc llvm sudo cmake; cmake/RunCheck.cmake treats a missing cppcheck executable as fatal. On these VM runs this newly added line will fail every FreeBSD compiler iteration before the tests unless cppcheck (and any needed unversioned clang-tidy) is installed or the target is gated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds the CMake check static-analysis target (cppcheck + clang-tidy) to the per-compiler build loops in CI for both the Linux/macOS and FreeBSD jobs, bringing the workflow in line with the AGENTS.md policy that mandates running check after each gcc/clang build. The PR also bundles four unrelated C source cleanups not mentioned in the description.
Changes:
- Run
cmake --build build --target checkafter each gcc/clang build in the Linux/macOS and FreeBSD compiler loops. - Source cleanups: drop unneeded cast in
destroy_process_table; reuseinit_list()inclear_list/destroy_list; remove EINTR retry loops aroundaccess()inis_script_inaccessible_interpreter()and aroundkill()insend_signal_to_processes()(with updated comment).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/CI.yml | Adds check target invocation after each compiler build in both CI jobs. |
| src/process_table.c | Removes unnecessary (void *) cast on free(proc_table->buckets). |
| src/list.c | Replaces manual field reset with init_list(lst) in clear_list/destroy_list. |
| src/limiter.c | Drops EINTR retry loop around access(F_OK); access() does not return EINTR. |
| src/limit_process.c | Drops EINTR retry loop around kill() and rewrites the explanatory comment. |
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -B build | ||
| cmake --build build | ||
| cmake --build build --target check |
| -DCMAKE_BUILD_TYPE=Release \ | ||
| -B build | ||
| cmake --build build | ||
| cmake --build build --target check |
The
checktarget (static analysis viacppcheck+clang-tidy) was not being run in the gcc/clang build loops for either the Linux/macOS or FreeBSD CI jobs.cmake --build build --target checkafter each build in both the Linux/macOS and FreeBSD compiler iteration loops