tests: full white-box coverage for all public interfaces + fix process_table_destroy double-destroy segfault#54
Conversation
Co-authored-by: HiGarfield <32226909+HiGarfield@users.noreply.github.com>
…li.c fix Co-authored-by: HiGarfield <32226909+HiGarfield@users.noreply.github.com>
…ouble-destroy segfault Co-authored-by: HiGarfield <32226909+HiGarfield@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Improves robustness and confidence in cpulimit by fixing a process_table_destroy double-destroy segfault and expanding the C unit test suite to cover previously untested branches (including parse_arguments by linking cli.c into the test binary).
Changes:
- Fix
process_table_destroyto clearhashsizeafter freeing the bucket array, preventing a second destroy from dereferencing a NULL table. - Update the test build to link
src/cli.c, enablingparse_argumentstests. - Add extensive white-box tests across modules to increase branch/boundary/error-path coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
src/process_table.c |
Prevents double-destroy segfault by zeroing hashsize after teardown. |
tests/Makefile |
Links cli.c into cpulimit_test so CLI parsing is testable. |
tests/cpulimit_test.c |
Adds many new white-box tests, including CLI parsing and edge/error paths. |
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * LIST MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments, which can also break builds depending on compiler/locale. Replace it with an ASCII hyphen (-).
| * LIST MODULE – ADDITIONAL COVERAGE | |
| * LIST MODULE - ADDITIONAL COVERAGE |
There was a problem hiding this comment.
Fixed in 621e2f1 — replaced all 9 en-dash – characters with ASCII hyphens - in the section header comments.
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * UTIL MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * UTIL MODULE – ADDITIONAL COVERAGE | |
| * UTIL MODULE - ADDITIONAL COVERAGE |
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * PROCESS_TABLE MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * PROCESS_TABLE MODULE – ADDITIONAL COVERAGE | |
| * PROCESS_TABLE MODULE - ADDITIONAL COVERAGE |
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * SIGNAL_HANDLER MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * SIGNAL_HANDLER MODULE – ADDITIONAL COVERAGE | |
| * SIGNAL_HANDLER MODULE - ADDITIONAL COVERAGE |
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * PROCESS_GROUP MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * PROCESS_GROUP MODULE – ADDITIONAL COVERAGE | |
| * PROCESS_GROUP MODULE - ADDITIONAL COVERAGE |
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * LIMIT_PROCESS MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * LIMIT_PROCESS MODULE – ADDITIONAL COVERAGE | |
| * LIMIT_PROCESS MODULE - ADDITIONAL COVERAGE |
| assert(init_process_iterator(&it, &filter) == 0); | ||
| /* dip is NULL because single-PID optimisation skips opendir() */ | ||
| assert(it.dip == NULL); |
There was a problem hiding this comment.
test_process_iterator_close_null_dip reaches into struct process_iterator internals (it.dip). That field only exists on Linux (process_iterator.h uses #if defined(__linux__)), so this test will not compile on FreeBSD/macOS. Prefer asserting only on public behavior (e.g., close_process_iterator(&it) == 0) or wrap the it.dip assertion in #if defined(__linux__).
| assert(init_process_iterator(&it, &filter) == 0); | |
| /* dip is NULL because single-PID optimisation skips opendir() */ | |
| assert(it.dip == NULL); | |
| assert(init_process_iterator(&it, &filter) == 0); | |
| #if defined(__linux__) | |
| /* dip is NULL because single-PID optimisation skips opendir() */ | |
| assert(it.dip == NULL); | |
| #endif |
There was a problem hiding this comment.
Fixed in 621e2f1 — the it.dip assertion is now guarded with #if defined(__linux__) so the test compiles cleanly on FreeBSD and macOS.
tests/cpulimit_test.c
Outdated
| } | ||
|
|
||
| /*************************************************************************** | ||
| * PROCESS_ITERATOR MODULE – ADDITIONAL COVERAGE |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| * PROCESS_ITERATOR MODULE – ADDITIONAL COVERAGE | |
| * PROCESS_ITERATOR MODULE - ADDITIONAL COVERAGE |
| /*************************************************************************** | ||
| * LIMITER MODULE – ADDITIONAL COVERAGE | ||
| ***************************************************************************/ |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
| /*************************************************************************** | ||
| * CLI MODULE – ADDITIONAL COVERAGE | ||
| ***************************************************************************/ |
There was a problem hiding this comment.
This comment header uses a non-ASCII en dash ("–"). AGENTS.md requires ASCII-only source/comments. Replace it with an ASCII hyphen (-).
|
@copilot apply changes based on the comments in this thread |
…x ifdef Co-authored-by: HiGarfield <32226909+HiGarfield@users.noreply.github.com>
There was a problem hiding this comment.
💡 Codex Review
cpulimit/tests/cpulimit_test.c
Line 1931 in 621e2f1
/workspace/cpulimit/AGENTS.md requires that only ASCII characters appear in code/comments, but this change introduces Unicode arrow characters (→) in tests/cpulimit_test.c comments (including this line and other newly added comments). This violates the project’s documented standards and can break tooling or checks that enforce source encoding/character-set constraints, so these comment markers should be replaced with ASCII equivalents (for example, ->).
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
process_table.c:process_table_destroynow zerospt->hashsizecli.cin test build–with ASCII hyphen-in all 9 section header commentsit.dip == NULLassertion in#if defined(__linux__)for portability💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.