Skip to content

chore(warnings): mechanical low-risk cleanup pass #499#513

Merged
TheWitness merged 1 commit intoCacti:developfrom
somethingwithproof:issue-499-warnings-baseline-cleanup
Mar 25, 2026
Merged

chore(warnings): mechanical low-risk cleanup pass #499#513
TheWitness merged 1 commit intoCacti:developfrom
somethingwithproof:issue-499-warnings-baseline-cleanup

Conversation

@somethingwithproof
Copy link
Copy Markdown
Contributor

@somethingwithproof somethingwithproof commented Mar 14, 2026

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:

Test plan

  • Verify spine compiles with both gcc and clang
  • Run spine against a test Cacti instance and verify polling works

Copilot AI review requested due to automatic review settings March 14, 2026 23:10
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR primarily tightens C API contracts across the codebase (const-correctness and proper (void) prototypes), fixes a handful of small correctness issues, and adds an initial unit test for string utilities.

Changes:

  • Update multiple function prototypes to use const char * where appropriate and (...)(void) for no-arg functions.
  • Fix/cleanup logic in several areas (e.g., db_column_exists() query routing, strncopy() usage in ping.c, removal of unused variables, unused-parameter suppressions).
  • Add a cmocka unit test covering strpos() behavior.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
util.h Updates prototypes (const-correctness, (void) args) and exports poller_push_data_to_main().
util.c Aligns implementations with updated signatures; refactors return types for DB-backed getters; const cleanup in string helpers; adds poller_push_data_to_main(void) signature.
tests/unit/test_util_strings.c Adds unit tests for strpos() using cmocka with local stubs/mocks.
sql.h Makes db_reconnect() location parameter const char *; minor cleanup.
sql.c Updates db_reconnect() signature and fixes db_column_exists() to use the passed connection type.
spine.c Updates forward declaration and renames debug-device parsing index for clarity.
snmp.h Makes SNMP OID parameters const char * in public APIs.
snmp.c Aligns SNMP functions with updated const signatures; suppresses unused-parameter warnings in snmp_snprint_value().
poller.h Makes exec_poll() type parameter const char *.
poller.c Suppresses unused parameters in cleanup handlers; removes unused snmp_poller_items; aligns exec_poll() signature; removes unnecessary cast for nft_popen().
ping.c Renames local timing vars for clarity; makes cacti_msg const; fixes strncopy() length argument.
php.c Reworks argv constant strings for script-server spawn to avoid transient formatted buffers.
nft_popen.c Avoids const-cast by duplicating the command string; frees duplication in parent paths.
locks.c Uses (void) prototype for init_mutexes; makes internal get_name() static.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@somethingwithproof somethingwithproof marked this pull request as draft March 20, 2026 10:00
@somethingwithproof
Copy link
Copy Markdown
Contributor Author

This PR overlaps with #511 on 10 C source files. Recommend merging #513 before #511 to minimize rebase work.

Mechanical cleanup of compiler warnings plus targeted bug fixes:
- Add void to empty parameter lists (init_mutexes, poller_push_data_to_main)
- Mark file-local functions static (get_name in locks.c)
- Add const to read-only string parameters (snmp_oid, nft_popen command)
- Add UNUSED_PARAMETER for callback args
- Fix strncopy length in get_namebyhost (was no-op after memset)
- Fix db_column_exists to use passed type instead of hardcoded LOCAL
- Fix getglobalvariable to strdup opttable value (fixes latent use-after-free)
- Change getsetting/getpsetting/getglobalvariable return to char* (ownership)
- Remove write-only snmp_poller_items counter
- Add strpos unit tests

Signed-off-by: Thomas Vincent <thomasvincent@gmail.com>
@somethingwithproof somethingwithproof force-pushed the issue-499-warnings-baseline-cleanup branch from 1e8cc9f to 2834e19 Compare March 25, 2026 02:25
@TheWitness TheWitness merged commit 0dd91c4 into Cacti:develop Mar 25, 2026
3 checks passed
@somethingwithproof somethingwithproof deleted the issue-499-warnings-baseline-cleanup branch March 26, 2026 00:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants