## 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>
## What
- move `keywords.c`/`.h` and `locks.c`/`.h` into `src/core/`
- enable Automake `subdir-objects` for collision-safe nested object
placement
- update the Autotools binary, linked-test, standalone unit-test, and
fuzz paths
- make both cppcheck entry points share one complete nested-source
discovery path
- ignore nested Automake and Docker build artifacts
- retain existing header include names and the Autotools build
The four production source/header files are 100% renames with no content
changes. The remaining changes are build and verification plumbing
required by the new paths.
## Why
Spine production sources currently live in the repository root. This
starts the domain layout in a small, reviewable batch instead of
repeating #523, where the source move was coupled to an 11k-line
CMake/platform rewrite. `keywords` and `locks` were selected because
active PRs #597 and #614 do not modify their implementation files; only
shared build manifests overlap #597.
`subdir-objects` is enabled in this first production move so future
modules with the same basename cannot write the same top-level object
under parallel builds.
Closes#616.
## Verification
- `./bootstrap` with no subdirectory-source warnings
- `./configure --with-mysql=$(brew --prefix mariadb-connector-c)
--with-snmp=$(brew --prefix net-snmp) CPPFLAGS=-I$(brew --prefix
openssl@3)/include LDFLAGS=-L$(brew --prefix openssl@3)/lib`
- warning-enabled `make -j$(sysctl -n hw.ncpu)`; project sources
compile, with Homebrew Net-SNMP header warnings only
- `make check` (cmocka unavailable to Autoconf locally; 0 local tests)
- `make distcheck`; archive verified to contain all four `src/core`
files
- cppcheck source discovery verified under macOS Bash 3 and includes all
four relocated files
- standalone `tests/unit/Makefile` compiles through header resolution
with `-I../../src/core` (the local macOS link retains unrelated existing
client-library/toolchain issues)
- GitHub checks pass, including fuzz, sanitizers, coverage, `distcheck`,
gcc/clang, SNMPv3, and Rocky Linux
The local macOS fuzz build reaches both relocated files, then fails
identically on untouched `util.c` (`INT_MAX`) and Net-SNMP API
declarations on both this branch and `develop`. Linux fuzz CI is the
authoritative fuzz-build result.
---------
Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
Co-authored-by: TheWitness <thewitness@cacti.net>
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>
Adds a README.md to the packaging directory providing an overview of
supported platforms and links to OS-specific instructions.
## Linked issue
Fixes#461
---------
Co-authored-by: netniV <netniv@hotmail.com>
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>
