Add BSD memory stats support#23
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds BSD-family platform detection and per-OS implementations of get_system_memory_stats (FreeBSD, DragonFly, OpenBSD, NetBSD) and updates the cfarm SSH CI workflow to support per-host tool paths, tar extraction, a FreeBSD debug SSH step, and configurable SSH ports. ChangesBSD Platform Support
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 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. Comment |
kluster.ai PR Review SummaryThis PR adds support for memory statistics retrieval across several BSD-based operating systems, specifically FreeBSD, OpenBSD, NetBSD, and DragonFly BSD. I have implemented To ensure consistency and reliability, I added a Additionally, I updated the GitHub Actions CI workflow to include these new environments. This involved expanding the test matrix to include compile farm hosts for the various BSDs, parameterizing the
Powered by kluster.ai - Real-Time AI code review in your IDE |
![kluster-ai[bot]](https://avatars.githubusercontent.com/in/2988314?s=60&v=4){kind=small}
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The data_size == sizeof(int) branch is unreachable because sizeof(unsigned int) evaluates to the exact same size, bypassing the intended negative-value check.
Explanation: In C, sizeof(unsigned int) is guaranteed to equal sizeof(int). Because the data_size == sizeof(unsigned int) check occurs first and returns early, any 4-byte sysctl result will be handled as an unsigned integer. This makes the subsequent sizeof(int) block dead code, meaning that if a sysctl ever returns a negative error code, it will be incorrectly cast to a massive positive uint64_t instead of safely returning false.
Recommended Action: This logic should consolidate the 4-byte checks into a single block, as size alone cannot distinguish between signed and unsigned types here.
Issue Actions:
| return false; | ||
| } | ||
|
|
||
| void get_system_memory_stats(struct system_memory_stats* stats) { |
There was a problem hiding this comment.
kluster.ai · 🟡 quality · medium
Description: The memory statistics implementations for DragonFly and FreeBSD are exactly identical.
Explanation: src/eatmemory.dragonfly.c and src/eatmemory.freebsd.c contain the exact same 70-line implementation, including identical helper functions (read_sysctl_uint64, pages_to_bytes_clamped) and sysctl queries. Maintaining identical copies of this logic creates structural debt, as any future bug fixes or improvements to the FreeBSD parsing logic will need to be manually mirrored to the DragonFly file.
Recommended Action: These two implementations should be consolidated into a single shared file (such as eatmemory.freebsd.c conditionally compiled for both, or a generic eatmemory.bsd.c) instead of copying the file.
Issue Actions:
Greptile SummaryThis PR adds
Confidence Score: 4/5The new BSD backends are well-structured and handle overflow correctly; the only concerns are a dead code branch (never reachable) and an identical duplicate file that may diverge over time. The four new C backends are logically sound — overflow guards, sign checks, and sysctl parsing are all correct. The FreeBSD and DragonFly files are byte-for-byte identical, which works today but creates a maintenance surface if the two platforms diverge. The dead sizeof(int) branch in both files is unreachable noise. The shared SSH host key between cfarm429 and cfarm430 in the CI workflow is unusual and worth confirming, but the PR author reports successful smoke tests on both hosts. src/eatmemory.dragonfly.c and src/eatmemory.freebsd.c (identical implementation + dead branch); .github/workflows/cfarm-ssh-tests.yml (shared host key for two different BSD hosts) Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[eatmemory.h platform detection] -->|__APPLE__| B[eatmemory.apple.c]
A -->|_WIN32 / _WIN64| C[eatmemory.windows.c]
A -->|_AIX| D[eatmemory.aix.c]
A -->|__sun| E[eatmemory.solaris.c]
A -->|__DragonFly__| F[eatmemory.dragonfly.c NEW]
A -->|__FreeBSD__| G[eatmemory.freebsd.c NEW]
A -->|__OpenBSD__| H[eatmemory.openbsd.c NEW]
A -->|__NetBSD__| I[eatmemory.netbsd.c NEW]
A -->|__linux__| J[eatmemory.linux.c]
A -->|else| K[eatmemory.unknown.c]
F & G --> L[hw.physmem / hw.pagesize / vm.stats.vm.v_free_count]
H --> M[struct uvmexp npages / pagesize / free]
I --> N[struct uvmexp_sysctl npages / pagesize / free]
Reviews (1): Last reviewed commit: "Add BSD memory stats support" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if (data_size == sizeof(int)) { | ||
| int result; | ||
| memcpy(&result, data, sizeof(result)); | ||
| if (result < 0) { | ||
| return false; | ||
| } | ||
|
|
||
| *value = (uint64_t)result; | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| void get_system_memory_stats(struct system_memory_stats* stats) { | ||
| uint64_t total_bytes; | ||
| uint64_t page_size; | ||
| uint64_t free_pages; | ||
|
|
||
| stats->supported = false; | ||
| stats->total = 0; | ||
| stats->free = 0; | ||
|
|
||
| if (read_sysctl_uint64("hw.physmem", &total_bytes) && | ||
| read_sysctl_uint64("hw.pagesize", &page_size) && | ||
| read_sysctl_uint64("vm.stats.vm.v_free_count", &free_pages) && | ||
| total_bytes > 0 && page_size > 0) { |
There was a problem hiding this comment.
sizeof(int) branch in read_sysctl_uint64 is unreachable dead code. On all supported platforms sizeof(int) == sizeof(unsigned int), so the earlier sizeof(unsigned int) check will always match first when the kernel returns a 4-byte value, and this branch can never execute. The same dead branch exists identically in src/eatmemory.freebsd.c.
| if (data_size == sizeof(int)) { | |
| int result; | |
| memcpy(&result, data, sizeof(result)); | |
| if (result < 0) { | |
| return false; | |
| } | |
| *value = (uint64_t)result; | |
| return true; | |
| } | |
| return false; | |
| } | |
| void get_system_memory_stats(struct system_memory_stats* stats) { | |
| uint64_t total_bytes; | |
| uint64_t page_size; | |
| uint64_t free_pages; | |
| stats->supported = false; | |
| stats->total = 0; | |
| stats->free = 0; | |
| if (read_sysctl_uint64("hw.physmem", &total_bytes) && | |
| read_sysctl_uint64("hw.pagesize", &page_size) && | |
| read_sysctl_uint64("vm.stats.vm.v_free_count", &free_pages) && | |
| total_bytes > 0 && page_size > 0) { | |
| return false; | |
| } | |
| void get_system_memory_stats(struct system_memory_stats* stats) { | |
| uint64_t total_bytes; | |
| uint64_t page_size; | |
| uint64_t free_pages; | |
| stats->supported = false; | |
| stats->total = 0; | |
| stats->free = 0; | |
| if (read_sysctl_uint64("hw.physmem", &total_bytes) && | |
| read_sysctl_uint64("hw.pagesize", &page_size) && | |
| read_sysctl_uint64("vm.stats.vm.v_free_count", &free_pages) && | |
| total_bytes > 0 && page_size > 0) { |
| - name: openbsd | ||
| host: cfarm429.cfarm.net | ||
| host_key: cfarm429.cfarm.net ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIKUiISOLz6YG0eeL1JX4IyLGh5WvFBPiRUK6gSM2OzQe | ||
| cc: /usr/bin/cc | ||
| make: /usr/local/bin/gmake | ||
| tar: /bin/tar | ||
| extra_cflags: "" | ||
| shell: /usr/local/bin/bash | ||
| path: /usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin | ||
| test_script: test/simple.test.sh |
There was a problem hiding this comment.
cfarm429.cfarm.net (OpenBSD) and cfarm430.cfarm.net (FreeBSD) are configured with the same SSH ED25519 host key (AAAAC3...OzQe). If these are two separate machines, one of them has an incorrect key and SSH verification would either silently accept the wrong host or fail at connection time. Was the OpenBSD host key intentionally copied from the FreeBSD entry, or do both hosts genuinely share this key (e.g., different jails/VMs on the same physical machine)?
| #include "eatmemory.h" | ||
| #ifdef SYSMEM_MODE_DRAGONFLY | ||
| #include <sys/types.h> | ||
| #include <sys/sysctl.h> | ||
| #include <string.h> | ||
|
|
||
| static size_t bytes_to_size_clamped(uint64_t bytes) { | ||
| return bytes > SIZE_MAX ? SIZE_MAX : (size_t)bytes; | ||
| } | ||
|
|
||
| static size_t pages_to_bytes_clamped(uint64_t pages, uint64_t page_size) { | ||
| if (pages != 0 && page_size > UINT64_MAX / pages) { | ||
| return SIZE_MAX; | ||
| } | ||
|
|
||
| return bytes_to_size_clamped(pages * page_size); | ||
| } | ||
|
|
||
| static bool read_sysctl_uint64(const char* name, uint64_t* value) { | ||
| unsigned char data[sizeof(uint64_t)] = {0}; | ||
| size_t data_size = sizeof(data); | ||
|
|
||
| if (sysctlbyname(name, data, &data_size, NULL, 0) != 0) { | ||
| return false; | ||
| } | ||
|
|
||
| if (data_size == sizeof(uint64_t)) { | ||
| uint64_t result; | ||
| memcpy(&result, data, sizeof(result)); | ||
| *value = result; | ||
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(unsigned int)) { | ||
| unsigned int result; | ||
| memcpy(&result, data, sizeof(result)); | ||
| *value = result; | ||
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { | ||
| int result; | ||
| memcpy(&result, data, sizeof(result)); | ||
| if (result < 0) { | ||
| return false; | ||
| } | ||
|
|
||
| *value = (uint64_t)result; | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| void get_system_memory_stats(struct system_memory_stats* stats) { | ||
| uint64_t total_bytes; | ||
| uint64_t page_size; | ||
| uint64_t free_pages; | ||
|
|
||
| stats->supported = false; | ||
| stats->total = 0; | ||
| stats->free = 0; | ||
|
|
||
| if (read_sysctl_uint64("hw.physmem", &total_bytes) && | ||
| read_sysctl_uint64("hw.pagesize", &page_size) && | ||
| read_sysctl_uint64("vm.stats.vm.v_free_count", &free_pages) && | ||
| total_bytes > 0 && page_size > 0) { | ||
| stats->total = bytes_to_size_clamped(total_bytes); | ||
| stats->free = pages_to_bytes_clamped(free_pages, page_size); | ||
| stats->supported = true; | ||
| } | ||
| } | ||
|
|
||
| #endif |
There was a problem hiding this comment.
eatmemory.dragonfly.c and eatmemory.freebsd.c are byte-for-byte identical, including the vm.stats.vm.v_free_count sysctl key. While DragonFly BSD descends from FreeBSD 4.x and shares this sysctl interface today, any future divergence would require updating both files independently. Was sharing the FreeBSD sysctl key for v_free_count verified on a live DragonFly host, or is it assumed based on lineage?
julman99
force-pushed
the
bsd-cfarm-support
branch
from
8c4bdaf to
d04d4f3
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The validation check for negative int payloads is unreachable dead code because sizeof(int) is identical to sizeof(unsigned int).
Explanation: The read_sysctl_uint64 function attempts to distinguish between unsigned int and int sysctl responses by comparing the returned byte size against sizeof(unsigned int) and sizeof(int). Because these sizes are guaranteed to be identical in C, any 4-byte payload will always match the first sizeof(unsigned int) branch. If the system ever returns a negative 4-byte integer, it will be blindly converted to a massive unsigned value rather than being safely rejected by the intended if (result < 0) guard in the second branch.
Recommended Action: This branching should be consolidated to parse 4-byte values into a signed int (or an int64_t) first, so the negative-value guard can reliably catch invalid responses before casting to uint64_t.
Issue Actions:
| return false; | ||
| } | ||
|
|
||
| void get_system_memory_stats(struct system_memory_stats* stats) { |
There was a problem hiding this comment.
kluster.ai · 🟡 quality · medium
Description: The DragonFly and FreeBSD memory statistics implementations are completely identical.
Explanation: The entire 74-line implementation in eatmemory.dragonfly.c is a direct copy of eatmemory.freebsd.c, including the sysctlbyname logic and clamping utilities. Maintaining identical copies of system-level logic increases the risk that future bug fixes or improvements will be applied to one platform but missed on the other.
Recommended Action: These two files should be consolidated into a single shared implementation (for example, eatmemory.freebsd.c) that uses a joint macro guard like #if defined(SYSMEM_MODE_FREEBSD) || defined(SYSMEM_MODE_DRAGONFLY).
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
d04d4f3 to
5cae1d9
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/eatmemory.netbsd.c`:
- Around line 29-34: The validation for NetBSD memory stats needs an upper-bound
check so stats->free cannot exceed stats->total: in the sysctl result handling
(the block that checks sysctl(...), uvm_size == sizeof(uvm), uvm.npages > 0,
uvm.pagesize > 0, and uvm.free >= 0) add a condition that uvm.free <= uvm.npages
before computing stats->total/stats->free and setting stats->supported = true;
ensure the check is applied alongside the existing conditions that guard
pages_to_bytes_clamped(uvm.npages, uvm.pagesize) and
pages_to_bytes_clamped(uvm.free, uvm.pagesize).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5129fc46-b3f6-4e68-95b7-ff29910428a6
📒 Files selected for processing (6)
.github/workflows/cfarm-ssh-tests.ymlsrc/eatmemory.dragonfly.csrc/eatmemory.freebsd.csrc/eatmemory.hsrc/eatmemory.netbsd.csrc/eatmemory.openbsd.c
🚧 Files skipped from review as they are similar to previous changes (5)
- src/eatmemory.h
- src/eatmemory.openbsd.c
- src/eatmemory.freebsd.c
- .github/workflows/cfarm-ssh-tests.yml
- src/eatmemory.dragonfly.c
| if (sysctl(mib, 2, &uvm, &uvm_size, NULL, 0) == 0 && | ||
| uvm_size == sizeof(uvm) && | ||
| uvm.npages > 0 && uvm.pagesize > 0 && uvm.free >= 0) { | ||
| stats->total = pages_to_bytes_clamped(uvm.npages, uvm.pagesize); | ||
| stats->free = pages_to_bytes_clamped(uvm.free, uvm.pagesize); | ||
| stats->supported = true; |
There was a problem hiding this comment.
Add an upper-bound validation for free pages.
On Line 31, please also validate uvm.free <= uvm.npages before marking stats as supported; otherwise stats->free can end up larger than stats->total.
Suggested patch
- if (sysctl(mib, 2, &uvm, &uvm_size, NULL, 0) == 0 &&
- uvm_size == sizeof(uvm) &&
- uvm.npages > 0 && uvm.pagesize > 0 && uvm.free >= 0) {
+ if (sysctl(mib, 2, &uvm, &uvm_size, NULL, 0) == 0 &&
+ uvm_size == sizeof(uvm) &&
+ uvm.npages > 0 && uvm.pagesize > 0 &&
+ uvm.free >= 0 && uvm.free <= uvm.npages) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/eatmemory.netbsd.c` around lines 29 - 34, The validation for NetBSD
memory stats needs an upper-bound check so stats->free cannot exceed
stats->total: in the sysctl result handling (the block that checks sysctl(...),
uvm_size == sizeof(uvm), uvm.npages > 0, uvm.pagesize > 0, and uvm.free >= 0)
add a condition that uvm.free <= uvm.npages before computing
stats->total/stats->free and setting stats->supported = true; ensure the check
is applied alongside the existing conditions that guard
pages_to_bytes_clamped(uvm.npages, uvm.pagesize) and
pages_to_bytes_clamped(uvm.free, uvm.pagesize).
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟠 semantic · high
Description: The data_size == sizeof(int) validation block here is completely unreachable because it is shadowed by the identical sizeof(unsigned int) size check. As a result, the intended validation for negative sysctl metrics is bypassed, and they are silently misparsed as huge positive numbers.
Explanation: Because sizeof(unsigned int) and sizeof(int) evaluate to the same size (4 bytes), the unsigned int branch eagerly intercepts all 4-byte sysctl responses. The subsequent int branch promises to safely reject negative values (result < 0), but since it is dead code, any negative 4-byte metric will bypass this check and wrap into an enormous positive value instead.
Recommended Action: This size-based dispatch should either be replaced with explicit type-aware sysctl reads for each specific metric, or the generic 4-byte handler should default to a signed int block to preserve the negative-value validation.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
2 times, most recently
from
71d21ff to
dfef1a3
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(unsigned int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The negative-value check for 4-byte sysctl results is unreachable because sizeof(unsigned int) and sizeof(int) are identical.
Explanation: read_sysctl_uint64 uses data_size to infer the type returned by sysctlbyname. However, because sizeof(unsigned int) evaluates to the same byte length as sizeof(int) (typically 4 bytes), any 4-byte sysctl result will always match the first branch. The subsequent sizeof(int) branch that safely validates for negative values is dead code, meaning a negative sysctl value would be blindly misinterpreted as a huge unsigned integer instead of failing safely.
Recommended Action: This validation flow should either require the caller to pass the expected type or explicitly cast all 4-byte values to a specific type without unreachable branching, since sysctl data size alone cannot distinguish between signed and unsigned types.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
dfef1a3 to
9cd879d
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟠 semantic · high
Description: read_sysctl_uint64 includes a fallback branch to reject negative int values, but it is unreachable because sizeof(unsigned int) equals sizeof(int).
Explanation: Since unsigned int and int are guaranteed to have the exact same size in C, any 4-byte sysctl response will be caught by the first unsigned int block, which unconditionally returns true. The subsequent int block—explicitly added to prevent negative 32-bit values from becoming huge unsigned 64-bit numbers—is dead code and will never execute.
Recommended Action: This sysctl parsing should rely on the known signedness of the specific sysctl rather than data_size alone, or the negative check should be merged into the 4-byte handler if all 4-byte sysctls should be rejected when negative.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
9cd879d to
799a616
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The explicit int type handler is unreachable because sizeof(unsigned int) matches the exact same byte size.
Explanation: In C, sizeof(unsigned int) is strictly equal to sizeof(int). Because the unsigned int size check appears first and unconditionally returns, any 4-byte sysctl response will be processed there. This makes the subsequent int block dead code, meaning a negative 4-byte response would be incorrectly cast to a huge positive value instead of failing the intended < 0 check.
Recommended Action: Consolidate the 4-byte size checks into a single block that correctly reflects whether the expected sysctl values can be safely treated as signed or unsigned.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
799a616 to
0fa7d82
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The validation check for negative sysctl values is permanently unreachable because it shares the same size condition as unsigned int.
Explanation: The code attempts to distinguish between unsigned int and signed int values returned by sysctlbyname by checking data_size == sizeof(unsigned int) and then data_size == sizeof(int). However, since sizeof(unsigned int) is strictly identical to sizeof(int) in C, any 4-byte sysctl value will be intercepted by the first block. The second block—and its if (result < 0) validation check—will never execute, breaking the explicit promise to reject negative values.
Recommended Action: Consolidate the 4-byte sysctl handling into a single block and validate potential negative values there before casting to uint64_t.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
0fa7d82 to
be00637
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: This int validation block is unreachable because sizeof(int) is identical to sizeof(unsigned int), meaning the preceding unsigned int block intercepts all 4-byte sysctl returns.
Explanation: The code explicitly attempts to handle 4-byte signed integers and reject negative values by returning false. However, because sizeof(int) == sizeof(unsigned int) in C, any 4-byte sysctl result will always be caught by the earlier unsigned int block. As a result, this safety branch is dead code, and any negative 4-byte sysctl values would be incorrectly cast to huge positive uint64_t numbers instead of being rejected.
Recommended Action: This size-based type inference should be replaced by explicitly passing the expected data type for each sysctl key, or the negative check should be applied to the 4-byte result before assuming it is unsigned.
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
2 times, most recently
from
6302962 to
4fa0d15
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟠 semantic · high
Description: The sizeof(int) condition is unreachable because sizeof(unsigned int) evaluates to the exact same value.
Explanation: The read_sysctl_uint64 function attempts to distinguish between unsigned and signed 32-bit sysctls by comparing data_size to sizeof(unsigned int) and sizeof(int). Because unsigned int and int are identical in size in standard C, the first block will unconditionally consume all 4-byte sysctls. This makes the signed sizeof(int) block entirely dead code, meaning any negative 32-bit signed integer returned by the kernel would bypass the < 0 check and incorrectly be parsed as a large positive unsigned value.
Recommended Action: Since sysctl size alone cannot determine signedness, explicitly request the sysctl as a specific type or remove the generic type parsing. If these specific sysctls are guaranteed to be positive, the dead signed-check block can be safely removed.
Issue Actions:
| echo "$(timestamp) probe 5: remote tar list" | ||
| timeout 45s ssh -vvv -T cfarm-ci "cd \"$remote_work\" && \"$CFARM_TAR\" -tf source.tar | head" | ||
|
|
||
| echo "$(timestamp) probe 6: remote tar extract" |
There was a problem hiding this comment.
kluster.ai · 🟡 semantic · medium
Description: The workflow matrix configures build and test parameters, but the actual compilation and test execution steps have been completely removed.
Explanation: The workflow matrix continues to define cc, make, extra_cflags, and test_script, which are exported to the environment as CFARM_CC, CFARM_MAKE, etc. However, the step that actually invoked these variables to build and test the project has been replaced by a purely diagnostic SSH tar-extract probe. The tests are no longer running on the host.
Recommended Action: Here, the diagnostic probe has replaced the build step, but the test configuration remains active in the matrix. Should the actual compilation and test execution steps be restored in this PR?
Issue Actions:
julman99
force-pushed
the
bsd-cfarm-support
branch
from
4fa0d15 to
bdec49b
Compare
| return true; | ||
| } | ||
|
|
||
| if (data_size == sizeof(int)) { |
There was a problem hiding this comment.
kluster.ai · 🟠 semantic · high
Description: The validation block meant to reject negative 32-bit sysctl values is completely unreachable because sizeof(int) is identical to sizeof(unsigned int).
Explanation: The function checks data_size == sizeof(unsigned int) to parse a 32-bit sysctl result, returning true unconditionally. Because sizeof(unsigned int) and sizeof(int) are identical on standard architectures, the subsequent data_size == sizeof(int) block that checks for result < 0 is never executed. Consequently, if a sysctl returns a negative integer, it will be caught by the first block and incorrectly parsed as a massive unsigned integer instead of being rejected.
Recommended Action: This validation logic should be merged into a single 32-bit parsing block that safely handles the expected sysctl data types, or the unreachable int block should be removed entirely if these specific sysctls are guaranteed to be positive.
Issue Actions:
1 participant
Summary
Compile farm hosts
cfarm429 is used for OpenBSD instead of cfarm220 because cfarm220 has hundreds of GB of RAM but a low default per-process data limit, making the 1% smoke test try to allocate several GB.
Validation
Summary by CodeRabbit
New Features
Chores