Introduce bounded native interfaces and grouped runtime state, then link
regression and fault contracts against the real poller implementations.
The contracts capture CLI/configuration input, availability
combinations, SNMP request/response ownership, script framing,
reindexing, output persistence, worker completion and shutdown. This
foundational change is larger because the shared interfaces and their
existing production contracts must move together; later changes have
separate review boundaries.
Step 1/14 of the complete C17/native-correctness series. Base: develop.
Earlier validation, before this review follow-up, in isolated copied
snapshots on Linux arm64:
- Clean GCC build and make check: 13 passed, 0 existing prerequisite
skips, 0 failures.
- ASan/UBSan: 12 passed, 1 existing privilege/allocator-interposition
prerequisite skips, 0 failures; no sanitizer findings.
- Production source/profile integrity verification against disposable
MariaDB and loopback SNMP fixtures passed.
- Release archive extracted into a separate source tree and built/tested
out of tree; fuzz checks passed.
Earlier local profiles retain source hashes for the copied validation
tree; GitHub checks validate the published commit. No production
database/device, host installation or live privilege change is part of
validation. Existing prerequisite skips were retained; no test or
finding was suppressed to obtain a pass.
The worker contracts publish owned heap state and release it only after
joining workers and restoring the runtime table. Coverage counters use
atomic updates; cppcheck receives an explicit model of the fatal exit
contract. Fixture images use public mirrors to avoid shared Docker Hub
pull limits.
Series navigation: review and merge in order. Every PR targets the
preceding branch so its diff contains one step.
1. [refactor(core): add bounded native interfaces and polling
contracts](https://github.com/Cacti/spine/pull/638)
2. [fix(privileges): drop root before input and retain owned ICMP
sockets](https://github.com/Cacti/spine/pull/639)
3. [fix(poller): isolate worker failures and bound database
reconnects](https://github.com/Cacti/spine/pull/640)
4. [fix(results): normalize output and unify SNMP batch
persistence](https://github.com/Cacti/spine/pull/641)
5. [fix(runtime): bound probe retries and validate CLI and
configuration](https://github.com/Cacti/spine/pull/642)
6. [build: select GNU C17 explicitly and probe hardening
support](https://github.com/Cacti/spine/pull/643)
7. [fix(threads): size worker stacks and guard size
arithmetic](https://github.com/Cacti/spine/pull/644)
8. [refactor(c17): simplify ownership guards and share test
support](https://github.com/Cacti/spine/pull/645)
9. [refactor(core): extract configuration polling and runtime
modules](https://github.com/Cacti/spine/pull/646)
10. [fix(boundaries): validate script slots and make cleanup
portable](https://github.com/Cacti/spine/pull/647)
11. [refactor(layout): organize responsibilities and separate main from
runtime](https://github.com/Cacti/spine/pull/648)
12. [docs(license): adopt SPDX notices and retain contributor
credits](https://github.com/Cacti/spine/pull/649)
13. [test(runtime): measure all suites and cover protocol
boundaries](https://github.com/Cacti/spine/pull/650)
14. [build(ci): validate connectors time width and native C17
modules](https://github.com/Cacti/spine/pull/651)
Review follow-up (2026-10-09): Publish the decoded GET/GETNEXT status
back to the host so availability checks observe exception and
missing-response failures, while preserving the legacy transport-success
behavior for agent PDU errors.
Updated step 1 snapshot: isolated Linux arm64 build and make check
passed (13 passed, no skips or failures).
The restacked tip passes all five clang-tidy gate regression tests and
real clang-tidy analysis (26 reviewed baseline findings, no new
findings). Actionlint and matching DCO trailers pass. The setuid test
also passes when run in the separate disposable root container.
GitHub checks validate the updated published commits; earlier
profile/archive/sanitizer evidence above belongs to the previous
revisions.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
## Summary
Consolidates the connector/build work from #613 and the broader
test-infrastructure work extracted from #597 into one review:
- discover MariaDB Connector/C first while retaining MySQL and
Percona-compatible support
- test MariaDB and MySQL Connector/C with strict GCC and Clang builds on
Ubuntu 24.04
- add a real macOS 15 build and unit-test lane with portable Autotools
platform detection
- run unit tests under ASan/UBSan and ThreadSanitizer
- enforce the measured 7% global coverage baseline and 70% changed-line
coverage
- add libFuzzer coverage for output parsing alongside the existing
hostname parser
- exercise the SNMPv3 integration suite against MariaDB and MySQL 8.4
- add cppcheck, shellcheck, DCO verification, CODEOWNERS, distcheck, and
Linux-capability checks while retaining the repository-managed default
CodeQL setup
- expose a stable `CI / required` aggregate check for branch protection
- pin runners and third-party actions and keep workflow permissions
read-only by default
## Correctness fixes found by the new gates
- reject truncated capability strings and poller-output SQL tuples
instead of publishing partial data
- make `bootstrap` fail closed and select `libtoolize`/`glibtoolize`
portably
- use Autoconf's canonical host for Darwin detection instead of an empty
native-build alias
- remove stale initialization/name-shadow findings and make read-only
interfaces const-correct
- make the ICMP parser test portable across Linux and BSD header names
## Verification
- strict GCC and Clang builds: 4/4 unit targets passed
- ASan/UBSan: 4/4 passed
- ThreadSanitizer: 4/4 passed
- macOS Clang/Homebrew build: 4/4 passed
- MariaDB integration: passed
- MySQL 8.4 integration: passed
- both libFuzzer targets: passed sustained local runs
- coverage: 7.2% global and 84% on changed lines
- cppcheck, shellcheck, actionlint, DCO verification, and `git diff
--check`: passed locally
## Post-merge repository settings
After these workflows exist on `develop`, an administrator should
require `CI / required`, Fuzz, Integration, Rocky Linux, and the
repository-managed CodeQL/code-scanning result in the active ruleset;
enable code-owner approval; and require the branch to be current (or use
a merge queue). Enabling those settings before this PR merges would
create required checks that the base branch cannot yet emit.
Consolidates and supersedes #613. Closes#612.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: TheWitness <thewitness@cacti.net>
Signed-off-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
Extracted from #597 into a focused SNMPv3 correctness and safety change.
This restores noAuthNoPriv and authNoPriv session construction, derives
authentication keys on the authNoPriv path, validates configured
algorithms when they are used, wipes temporary passphrase copies, and
makes the effective security-level decision agree with Cacti. It also
prevents a failed mid-loop session rebuild from passing NULL into
Net-SNMP multi-get.
Compatibility and failure policy:
- [None] is the explicit no-protocol sentinel; stale passphrases paired
with it downgrade with a warning to the Cacti effective level.
- A selected protocol missing its passphrase downgrades with an explicit
warning, preserving existing polling behavior.
- A blank protocol field paired with a passphrase is ambiguous and now
fails closed with an error.
- Complete privacy credentials without usable authentication fail closed
because USM cannot honor privacy without authentication.
- Unknown authentication algorithms fail closed instead of silently
substituting a default; unused privacy algorithms do not block
authNoPriv.
Verification:
- warning-enabled C99 build on macOS arm64
- make check: all 5 suites passed
- deterministic SNMPv3 session suite: 22/22 pass, including literal OID
mapping, effective-level combinations, refusal paths, key derivation,
caller-buffer ownership, and NULL multi-get handling
- mandatory independent pre-push review: PASS
Closes#582
Refs #597
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: TheWitness <copilot@users.noreply.github.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: TheWitness <copilot@users.noreply.github.com>
**Problem Description**
When a device does not support one or more of the system OIDs queried by
get_system_information(), not only system information collection but
also resource-related SNMP polling may fail.
This issue is especially noticeable when the device's Poller Thread
setting is 1, where resource-related SNMP polling does not occur at all.
During analysis, I found that if one of the OIDs queried in
get_system_information() returns NoSuchObject, the host is marked as
ignored through the following call path:
snmp_get()
-> snmp_get_base(current_host, snmp_oid, true)
-> NoSuchObject returned
-> current_host->ignore_host = TRUE
The OIDs queried by get_system_information() are:
- sysDescr
- sysObjectID
- sysUpTime
- sysContact
- sysName
- sysLocation
- Existing Behavior
Issue #366 already introduced special handling for the following OIDs:
- sysDescr
- sysUpTime
Reference:
Issue #366: Failing to actually poll a device that returns
"NoSuchObject"
However, the same handling was not applied to:
- sysObjectID
- sysContact
- sysName
- sysLocation
As a result, devices that do not implement one or more of these OIDs can
still be marked as ignored, causing polling to stop unexpectedly.
**Observed Behavior**
The following log shows the issue occurring when the target device
returns NoSuchObject for sysContact (.1.3.6.1.2.1.1.4.0).
After the error is detected, ignore_host becomes active and subsequent
system OID requests are skipped. The device polling thread then
completes without polling any of the scheduled data sources.
```
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] ERROR: No such Object for oid '.1.3.6.1.2.1.1.4.0' for Device[72] with Status[1]
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] WARNING: Skipped oid '.1.3.6.1.2.1.1.5.0' for Device[72] as host ignore flag is active
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] WARNING: Skipped oid '.1.3.6.1.2.1.1.6.0' for Device[72] as host ignore flag is active
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] Device[72] HT[1] NOTE: There are '17' Polling Items for this Device
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] DEBUG: Setting up writes to local database
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] Device[72] HT[1] Total Time: 0.019 Seconds
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] DEBUG: Freeing Local Pool ID 0
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502752032320] DEBUG: Device[72] HT[1] DEBUG: HOST COMPLETE: About to Exit Device Polling Thread Function
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502985176896] The final count of Threads is 0
2026-07-30 10:56:53 - SPINE: Poller[1] PID[1069197] PT[140502985176896] INFO: Device[72] Thread complete and 0 to 17 sources
```
In this example, the device contains 17 polling items, but polling
completes with:
INFO: Device[72] Thread complete and 0 to 17 sources
indicating that none of the scheduled data sources were successfully
polled.
**Fix**
This change adds the same exception handling for the remaining four
OIDs:
- sysObjectID
- sysContact
- sysName
- sysLocation
If any of these OIDs return NoSuchObject, the host will no longer be
marked as ignored, allowing normal resource-related polling to continue.
**Expected Result**
Devices that do not implement one or more optional system OIDs should
still be polled successfully for resource-related metrics.
Only the unavailable system information fields should be skipped, while
normal SNMP data collection continues without interruption.
Co-authored-by: TheWitness <thewitness@cacti.net>
## Problem
`snmp_count()` (used by the `POLLER_ACTION_SNMP_COUNT` reindex check)
already treats a `NULL` response, `STAT_TIMEOUT`, and `STAT_ERROR` as
failures, but two gaps remained:
- SNMPv1 has no `endOfMibView` variable type; it signals the normal end
of a `GETNEXT` walk with a PDU-level `noSuchName` (`response->errstat ==
SNMP_ERR_NOSUCHNAME`) instead. That was falling into the generic
"internal Net-SNMP error" branch, so a normal SNMPv1 walk completion was
logged and treated as an error.
- The function always `return count;`, so a caller had no way to tell a
real zero-item count apart from a walk that failed partway through
(missing session, timeout, internal error). `poll_host()`'s caller
stored whatever it got directly as the poller result either way.
This is a smaller, targeted counterpart to the equivalent fix already
backported to `1.2.x` in #574.
## Fix
- Distinguish `SNMP_ERR_NOSUCHNAME` (normal SNMPv1 walk completion) from
any other non-`SNMP_ERR_NOERROR` `errstat`, which is a genuine internal
error.
- Treat a missing SNMP session (`current_host->snmp_session == NULL`)
the same as any other failure.
- Return `-1` instead of `count` whenever an error occurred, so the
walk's outcome is unambiguous.
- Update `poll_host()`'s one caller in the reindex loop to mark the
result undefined (`SET_UNDEFINED`) on a negative count instead of
formatting `-1` as the literal poller result string.
## Testing
Compiled cleanly (via WSL); no new warnings in `snmp.c` or `poller.c`.
## Summary
A batch of small, independent, unaddressed bugs from the open-issue
backlog. None of these overlapped with any currently open PR's actual
diff (verified by inspecting each candidate PR's changes directly, not
just its declared `Closes` list).
## Fixes
- **`poll_host()` skips `mysql_thread_end()` when the device row is
missing** — the early `return` on `num_rows != 1` released both DB
connections and freed six allocations, but never called
`mysql_thread_end()`, unlike the other two exit paths. Added the missing
call.
- **SNMPv3 passphrase scrubbing flag never set** — `zero_sensitive` was
a function-local initialized to 0 and never assigned, so every guarded
`memset()` was dead code. Made scrubbing of this module's own heap
copies (`Apsz`/`Xpsz`) unconditional, and moved the wipe to immediately
precede each free (including both `generate_Ku()` error paths) after
review found the first pass still scrubbed before assignment. Left the
caller's `snmp_password`/`snmp_priv_passphrase` alone rather than also
scrubbing those in place — those are `host_t`-owned and get reused for
later session rebuilds against the same host, so zeroing them would have
broken subsequent polls.
- **`php_readpipe()` FD_SET without an FD_SETSIZE check** — `FD_SET()`
doesn't range-check, so a read descriptor at or above `FD_SETSIZE` (1024
on glibc) wrote past the end of the on-stack `fd_set`. Added a bounds
check; on an out-of-range descriptor it marks the result undefined and
closes the slot (a review finding caught the first version recursing
without bound through `php_init()` → `php_readpipe()` under sustained
descriptor exhaustion, so it no longer restarts synchronously from
here).
- **Signal handler never re-arms, so the second SIGPIPE kills the
poller** — every fatal signal reset itself to `SIG_DFL` and never
re-armed. That's the right idiom for a real fault (SIGSEGV and friends
re-execute and die with a core), but SIGPIPE is a routine, recurring
condition — a second one previously killed the whole poller. A review
finding pointed out that re-arming only at the end still left a window
(signal dispositions are process-wide) where another thread's SIGPIPE
could hit `SIG_DFL`; fixed by never resetting SIGPIPE's disposition in
the first place.
- **spine fails to link on Fedora: undefined reference to
`usmDESPrivProtocol`** — some distributions ship a `net-snmp-config.h`
that advertises DES as the default privacy protocol while the shipped
`libnetsnmp` doesn't export the symbol. Added an `AC_LINK_IFELSE` probe
for the actual symbol and guarded the fallback to use
`usmAESPrivProtocol` when it's absent.
- **Poller results ≥1024 bytes silently truncated to 1022** —
`db_escape()` capped its internal trim buffer at a fixed `DBL_BUFSIZE`
regardless of the caller's `max_size`, so raising
`--with-results-buffer` never helped. The trim buffer now scales with
`max_size` (a review finding caught the trim-size formula going
negative/zero for `max_size` 1-3), and `escaped_result` in `poll_host()`
is sized to `RESULTS_BUFFER * 2 + 1` instead of the fixed `DBL_BUFSIZE`.
(A very large escaped result can still hit the separate, already-tracked
row-size limit in #598/#618 — this fix only addresses the escaping-stage
truncation.)
## Also included
- **Seed `spine.conf` from `spine.conf.dist` on first install** — `make
install` only ever installed `spine.conf.dist`; nothing produced a live
`spine.conf`. Added an `install-data-hook` that copies it over only when
`spine.conf` doesn't already exist, plus a matching `uninstall-hook`
that only removes it again when it's still byte-identical to the `.dist`
(so a real uninstall never discards a customized config, while `make
distcheck`'s install/uninstall symmetry check still passes).
## Dropped from this PR
- A custom `codeql.yml` workflow (originally added for #580) was removed
— the repo already has GitHub's default CodeQL setup enabled and running
(`gh api repos/Cacti/spine/code-scanning/default-setup` → `state:
configured`), and a custom "advanced configuration" workflow cannot
coexist with it; GitHub rejected the SARIF upload and failed CI. #580's
underlying gap was already resolved out of band, so this PR no longer
claims to close it.
## Testing
Full build via WSL (`./bootstrap && ./configure --enable-warnings &&
make`) succeeds with no new warnings in `poller.c`, `snmp.c`, `php.c`,
`error.c`, or `sql.c` beyond two pre-existing, unrelated
`-Wformat-truncation` warnings in `php_init()`. Confirmed the
`usmDESPrivProtocol` configure probe correctly detects the missing
symbol in this build environment and takes the AES fallback path without
a link error. `make distcheck` passes end to end.
Closes#594Closes#600Closes#605Closes#607Closes#575Closes#583
Poller correctness and memory-safety fixes, with the ICMP hardening from
#543 folded in.
Verified in a clean `ubuntu:24.04` container: builds at **38 warnings**
against develop's 39, and the unit tests build and pass on Linux (39
assertions across three binaries).
**Fixes**
- `php_readpipe` off-by-one — the old bound let `bptr` reach
`result_string + 2048` and then wrote the terminator one byte past a
2048-byte allocation.
- `get_namebyhost` used `strtok`, which keeps one process-wide save
pointer, and runs unlocked on every poller thread. Two devices could be
polled at each other's addresses. Now `strtok_r`.
- The log date format was read from `default_dateformat`; Cacti's key is
`default_date_format`, so the lookup never matched and the format was
never loaded.
- `db_query` returns `mysql_store_result()` directly, which is NULL on
store failure. The NULL check ran *after* `mysql_fetch_row`, so the
crash happened one line before the guard.
- `snmp_count` logged a Net-SNMP `errstat` without setting either loop
flag, rebuilding the identical GETNEXT forever and pinning a thread
until the poll interval ended.
- `php_close` skipped the `close()` when the quit write failed with
EPIPE, leaking a descriptor per script-server restart.
- The reap loop's delay was compiled out on `SOLAR_THREAD` builds, so
both phases burned twenty `WNOHANG` polls in nanoseconds and the child
was never reaped.
- ICMP replies are validated for length and matched on id/sequence; on
Cygwin the peeked datagram is now drained, which previously busy-spun
until host timeout.
- The fatal signal handler no longer calls
`malloc`/`localtime_r`/`fprintf` before `_exit` — on a SIGSEGV caused by
heap corruption that deadlocked on the allocator's lock instead of
dying.
- The unit test Makefile was hardcoded to `/opt/homebrew` paths and
`-std=gnu23`, so it could not build on Linux at all. Now pkg-config
driven.
Supersedes #543.
**Added since the first revision**
- `snmp_get_multi()` now checks the varbind type before formatting.
Under v2c an agent reports a per-OID failure as an inline exception
varbind while the PDU `errstat` stays `NOERROR`, so `snprint_value()`
rendered the exception as literal text and stored it as a value.
- Two warning sources removed: a dangling line continuation ending
`sql.h`, and `strncopy()` now using `memcpy` where the byte count is
already exact. Warnings drop from **39 to 7**.
Closes#551Closes#232
- `read_config_options()` issued 23 separate `SELECT ... FROM settings
WHERE name = ?` round trips at startup; they are now one bulk read
served from memory. Call sites are unchanged.
Closes#330
**Test infrastructure**
- The SNMPv3 integration fixture was broken: the image shipped a binary
that could not start (`libmysqlclient.so.21: cannot open shared object
file`). The build context carried `*.o`, `spine`, `Makefile` and
`config.status`, so `make` treated them as current, skipped compiling,
and packaged a binary linked against libraries the runtime image does
not have. `.dockerignore` now excludes build output. The fixture runs
end to end: spine polls the SNMPv3 device, reads `uptime`, writes to
`poller_output` and exits 0.
- Two libFuzzer targets, run per PR and weekly: the host string parser
and the ICMP reply parser. The second drives
`spine_icmp_classify_reply()`, extracted from `ping_icmp` so the bounds
walk over a shared raw socket is reachable by a test. 15M executions
under ASan/UBSan, no crashes.
**Added in this revision**
- `output_regex` was applied only in the in-loop OID drain, not the pass
after the loop that handles whatever did not fill a `MAX_OIDS` batch, so
a device whose OID count missed a boundary stored raw values. A single
data source is always the leftover case.
`tests/integration/test_output_regex.sh` now asserts on the value: it
fails on the parent commit with the raw reading and passes here with the
filtered one. Addresses #210; the feature stays dormant until Cacti
populates `poller_item.output_regex`.
Merge before #555, which needs the `get_namebyhost` fix above to resolve
a device hostname at all.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Fresh develop-based port of applicable memory-safety, process-cleanup,
SNMP, poller, and PHP transport fixes from #544. Obsolete release and CI
history was excluded.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Fixes compiler warnings across spine source files: adds missing void in
function prototypes, marks internal functions static, removes unused
variables, and fixes const-correctness on function signatures.
**Note:** This PR also includes the following non-mechanical changes
that should be reviewed carefully:
- sql.c: db_column_exists now passes through the caller's type argument
instead of hardcoding LOCAL (bug fix, changes runtime behavior for
REMOTE callers)
- poller.c: Removed write-only snmp_poller_items counter and its
increment block (dead-code elimination)
- util.c: getglobalvariable now returns a strdup'd copy instead of a
pointer into opttable (fixes latent use-after-free on free())
- ping.c: Fixed strncopy(stack, hostname, strlen(stack)) to
strlen(hostname) (old code was a no-op after memset)
- util.c: Changed getsetting/getpsetting/getglobalvariable return type
from const char * to char * to reflect caller ownership semantics
### Addressed performance and DRY issues:
- Addresses #501 (Exponential Backoff)
- Addresses #503 (Centralized Logging)
- Addresses #504 (SNMP_FREE Macro)
- Addresses #505 (O(1) Hash Table)
### Test plan
- [ ] Verify spine compiles with both gcc and clang
- [ ] Run spine against a test Cacti instance and verify polling works
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Fixes#425
When an agent returns `errstat != NOERROR` with `errindex == 0` (RFC
3416
permits this for PDU-level errors), the `errindex != 0` branch in
`snmp_get_multi()` was skipped entirely. Results stayed as empty
strings,
causing blank output in `poller_output` and RRD instead of the `"U"`
sentinel.
Add an `else` branch: when `errindex == 0` set all results `UNDEFINED`
so
the `"U"` sentinel propagates correctly through the poller output path.
Also adds a Docker-based SNMPv3 regression fixture under `tests/snmpv3/`
that exercises this code path and related SNMP error-handling scenarios.
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: netniV <netniv@hotmail.com>
Twelve bugs found by running cppcheck 2.20
(--enable=warning,portability) and shellcheck v0.10.0.1 across the full
source tree.
**poller.c**
- `%llu` used for `atoll()` return (signed `long long`) at line 970 —
changed to `%lld`
- `%lld` used for `hex2dec()` return (unsigned `long long`) at five call
sites (lines 1414, 1515, 1597, 1655, 1727) — changed to `%llu`
Mismatched sign in printf format specifiers is undefined behaviour under
C99 §7.19.6.1.
**snmp.c**
- `session.localname` is `strdup()`'d at line 140 (when `SNMP_LOCALNAME
== 1`) but the early-return path for an unknown SNMP version at line 190
did not free it. Added `free(session.localname)` before that return.
`free(NULL)` is a no-op so the call is safe when `SNMP_LOCALNAME != 1`.
- `calloc(num_oids, sizeof(*name))` at line 999 has no NULL check. If
allocation fails, the immediately following `namep->name_len =
MAX_OID_LEN` dereferences a null pointer. Added a NULL guard that logs
via `SPINE_LOG` and returns early before the PDU is created.
**sql.c**
- `attempts` is `int` but printed with `%u` — changed to `%d`.
- `error` is `int` but printed with `%u` — changed to `%d`.
- `mysql = NULL` in `db_disconnect()` assigns to a by-value parameter;
the write has no effect on the caller. Removed the dead assignment.
**util.c**
- `pthread_self()` is cast to `unsigned long int` then printed with
`%ld` (signed) — changed to `%lu`.
**copyright_year.sh / scripts/verify.sh**
- `$1` unquoted in `sed` invocation in `copyright_year.sh` — word-splits
on filenames with spaces.
- `*.c *.h` glob in `scripts/verify.sh` passed without `--` separator —
filenames starting with `-` would be misinterpreted as cppcheck options.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Builds on #413 with three additional improvements to SNMPv3 error
handling.
**snmp_sess_error() retrieval before USM switch**
Without calling `snmp_sess_error()` the `sess_liberr` variable is always
`SNMPERR_SUCCESS`, so the USM switch introduced in #413 always falls
through to `default`. This call populates `sess_liberr` from the actual
session state before the switch runs.
**snmp_sess_close() mutex**
`snmp_sess_open()` acquires `LOCK_SNMP` but `snmp_host_cleanup()` calls
`snmp_sess_close()` unprotected. These operations share the net-snmp USM
engine cache. Wrap `snmp_sess_close()` with `LOCK_SNMP` to match.
**Portability guards for USM error constants**
The newer `SNMPERR_NOT_IN_TIME_WINDOW` and the older
`SNMPERR_USM_NOTINTIMEWINDOW` symbol sets map to different numeric
values. Outer `#if defined(A) || defined(B)` guards ensure both sets
compile on Net-SNMP 5.7+ and that neither case label appears when
neither symbol is defined (older versions).
Same USM decoding applied to `snmp_getnext()` which had the same
catch-all gap as `snmp_get_base()`.
Fixes: Cacti/cacti#6668
Ref: #413
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Fixes#422
`count++` at `snmp.c:899` fired before the `SNMP_ENDOFMIBVIEW` type
check.
When an agent returns ENDOFMIBVIEW with the OID still within the
requested
subtree (RFC 3416 permits this), the out-of-subtree guard does not fire
and
count reaches 1 for an empty table. The poller reindex assertion then
incorrectly triggers a reindex every poll cycle.
Move `count++` inside the `if (vars->type != SNMP_ENDOFMIBVIEW && ...)`
branch so only real entries increment the counter.
Verified by `tests/snmpv3/scripts/run_test.sh` Phase 4: inserts a
`poller_reindex` row with `action=10` for a non-existent OID subtree and
confirms spine logs `output: 0`.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Fixes#415
Four resource leaks in snmp.c and poller.c:
1. snmp.c: `session.peername` not `strdup()`'d, dangling reference after
`snmp_sess_open()`
2. snmp.c: `securityAuthProto`/`securityPrivProto` OIDs leaked on 4
error paths and happy path
3. snmp.c: PDU leaked in `snmp_get_base()` and `snmp_getnext()` when
`snmp_parse_oid()` fails
4. poller.c: `sprintf` overflow in `exec_poll()`, switched to `snprintf`
All declarations at block top per project convention. Tested with gcc
and clang.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Retrieve the actual session errno via `snmp_sess_error()` after
`STAT_ERROR` and decode the six USM report types individually.
`notInTimeWindow` is treated as recoverable since net-snmp resolves it
via internal retry on the next poll.
Also wraps `snmp_sess_close()` in `LOCK_SNMP` to protect the shared USM
engine cache.
Fixes#412
Related: Cacti/cacti#6668
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
net-snmp 5.9.5 injects `-Werror=declaration-after-statement` via
`net-snmp-config --cflags`, breaking builds on FreeBSD and other
systems where net-snmp is compiled with strict C89 flags.
Moves all variable declarations to the top of their enclosing
block in `spine.c` (main) and `util.c` (read_config_options,
spine_log, strncopy, checkAsRoot, regex_replace).
Verified: clean compile with `-Werror=declaration-after-statement`
across sql.c, spine.c, and util.c.
Supersedes #407 (which stripped the flag instead of fixing the root
cause).
Resolves#406
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
If sysDescr or sysUpTime return "NoSuchObject" then accept the return
and process normally, otherwise clear it and change status to an error.
The behavior changed due to the patch for #272, and I have not been able
to poll a few devices with spine newer than 1.2.21. I have tested this
change on 1.2.27 (my current version in production)
Co-authored-by: TheWitness <thewitness@cacti.net>
while compiling spine clang 16 generates the following warning:
```
snmp.c:590:58: warning: passing 'char[2048]' to parameter of type 'const u_char *' (aka 'const unsigned char *') converts between pointers to integer types where one is of the unique plain 'char' type and the other is not [-Wpointer-sign]
snprint_asciistring(result_string, RESULTS_BUFFER, temp_result, strlen(temp_result));
```
If the function treats this as a unsigned char *, I don't see any issue
casting it as such before passing it in to suppress the warning.
Co-authored-by: Erik Sejr <eriks@ssicanada.com>
Thomas Vincent