fix: replace deprecated POSIX semaphores with pthread wrapper#517
Merged
TheWitness merged 1 commit intoCacti:developfrom Mar 26, 2026
Merged
Conversation
TheWitness
pushed a commit
that referenced
this pull request
Mar 26, 2026
## Summary Fixes #515. Spine on `develop` fails to compile after recent feature branch merges (everything after `4837ddd`). This PR fixes 6 build/runtime bugs, replaces deprecated `vfork`, and adds 68 automated tests. ## Fixes ### Build-breaking (prevented compilation) 1. **Missing uthash.h** - vendored v2.3.0 (header-only, BSD license). The O(1) hash table feature added `#include "uthash.h"` but never committed the file. 2. **Extra closing braces in poller.c** - duplicate `}` in POLLER_ACTION_SCRIPT and POLLER_ACTION_PHP_SCRIPT_SERVER cases broke the switch statement. ### Runtime bugs 3. **SNMP_FREE macro collision** - spine's `SNMP_FREE` (calls `snmp_host_cleanup`) collided with net-snmp's `SNMP_FREE` (calls `free`). Renamed to `SNMP_SESSION_FREE`. The `errstr` from `snmp_sess_error()` now correctly uses net-snmp's `free()` instead of `snmp_host_cleanup()`. 4. **output_regex column detection** - the output_regex feature (#210) assumed the column exists, but baseline Cacti does not have it. Added `db_column_exists()` check at startup to conditionally include it in queries. ### Deprecation warnings 5. **vfork in nft_popen.c** - replaced with `posix_spawn` using file actions, matching the approach already used in `php.c`. Eliminates macOS deprecation warning. ### Test fixture 6. **snmp_auth_protocol column width** - `char(6)` too narrow for `SHA-256` (7 chars). Fixed to `char(7)` in both host and poller_item tables. Added `rocommunity public` to snmpd.conf for SNMPv2c test coverage. ## Test Results (68/68 pass) | Suite | Count | Coverage | |-------|-------|----------| | Unit (cmocka) | 16 | uthash macros, config init, regex_replace, strncopy | | Smoke | 21 | SNMPv3 + SNMPv2c polling, DB writes, multi-device, runtime validation | | Output regex | 15 | Column absent, present, regex filtering, removal fallback | | DB column detect | 16 | 3 scenarios: absent, added via ALTER TABLE, dropped | ## Files changed (15) | File | Change | |------|--------| | `uthash.h` | New: vendored header | | `Makefile.am` | Add uthash.h to EXTRA_DIST | | `common.h` | Add uthash.h include | | `poller.c` | Fix braces, conditional output_regex in queries | | `spine.c` | Add has_output_regex detection at startup | | `spine.h` | Add has_output_regex to config struct | | `snmp.h` | Add SNMP_SESSION_FREE macro | | `sql.c` | Fix indentation in db_query(), restore missing continue | | `nft_popen.c` | Replace vfork+execve with posix_spawn | | `tests/snmpv3/db/init.sql` | Fix snmp_auth_protocol char(7) | | `tests/snmpv3/snmpd/snmpd.conf` | Add rocommunity public | | `tests/integration/smoke_test.sh` | New: e2e smoke test | | `tests/integration/test_output_regex.sh` | New: output_regex scenarios | | `tests/integration/test_db_column_detect.sh` | New: db_column_exists scenarios | | `tests/unit/test_build_fixes.c` + `Makefile` | New: cmocka unit tests | ## Follow-up PRs - #517: Replace deprecated POSIX semaphores with pthread wrapper (9 remaining warnings) - SNMP error handling, spike kill, script server path testing Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
somethingwithproof
force-pushed
the
fix/replace-deprecated-semaphores
branch
from
daa171d to
6eac792
Compare
Contributor
Author
somethingwithproof
force-pushed
the
fix/replace-deprecated-semaphores
branch
9 times, most recently
from
3c4dba2 to
5b87916
Compare
somethingwithproof
force-pushed
the
fix/replace-deprecated-semaphores
branch
from
5b87916 to
70f0f58
Compare
TheWitness
approved these changes
Mar 26, 2026
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
2 participants
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.
Summary
Fixes #516. Replaces deprecated unnamed POSIX semaphores with a portable pthread-based wrapper. Eliminates all 9 remaining build warnings.
Depends on #514 (squashed single commit, rebased on latest develop).
Approach
Spine uses semaphores as atomic counters (
sem_post+sem_getvalue+sem_trywait, no blockingsem_wait). A pthread mutex + counter wrapper provides identical semantics on all platforms with zero#ifdefs.Alternatives considered:
dispatch_semaphore_t(macOS GCD): nogetvalueequivalent, requires platform ifdefs_Atomic: no wait/signal capabilityChanges (1 commit)
spine_sem.h: inlinespine_sem_init,spine_sem_post,spine_sem_getvalue,spine_sem_wait,spine_sem_trywait,spine_sem_destroy#include <semaphore.h>with#include "spine_sem.h"incommon.hsem_t/sem_*references inspine.c,poller.c,spine.hspine_sem.htoEXTRA_DISTWarnings eliminated (9)
spine.csem_initspine.csem_getvaluepoller.csem_getvalueBuild result
Zero errors, zero warnings.