Skip to content

lint + static analysis: clang-tidy and cppcheck fixes#5594

Merged
renecannao merged 43 commits intov3.0from
v3.0-lint
Apr 10, 2026
Merged

lint + static analysis: clang-tidy and cppcheck fixes#5594
renecannao merged 43 commits intov3.0from
v3.0-lint

Conversation

@renecannao
Copy link
Copy Markdown
Contributor

@renecannao renecannao commented Apr 10, 2026

Summary

This PR applies clang-tidy and cppcheck static analysis fixes to improve code quality and fix real bugs in ProxySQL.

Changes Overview

Category Files Changes
Include guards 37 __CLASS_*PROXYSQL_*_H
Reserved identifiers 15+ __varvar_, __sizeSIZE_
NULL → nullptr 32+ C-style NULL to modern nullptr
Uninitialized members 8 classes Added proper initialization
Memory safety 5 fixes Leak, dangling pointers, bounds
Printf bugs 6 fixes Wrong format specifiers/args
Null pointer bugs 4 fixes Missing null checks
Missing copy ops 15 classes Added deleted copy operators

Detailed Fixes

1. Clang-Tidy Fixes (Reserved Identifiers)

Include guards (37 files):

  • Changed #ifndef __CLASS_*#ifndef PROXYSQL_*_H
  • Updated corresponding #endif comments

Member variables:

  • __activeactive_
  • __useruser_
  • __sesssess_

Enum sentinels:

  • __sizeSIZE_ (for Prometheus metric enums)
  • ROS__SIZEROS_SIZE_

Constant names:

  • __HGM_TABLES_SIZEHGM_TABLES_SIZE_
  • __TYPE_SIZETYPE_SIZE_

NULL → nullptr (32 fixes in headers)

2. Cppcheck Fixes (Real Bugs Fixed)

Memory Issues

  • memleakOnRealloc (gen_utils.h): Fixed realloc that could lose original pointer on failure
  • invalidLifetime (MySQL_HostGroups_Manager.cpp): Fixed dangling pointer to moved unique_ptr
  • noCopyConstructor (15 classes): Added deleted copy ops to prevent double-free

Uninitialized Variables (~40 fixes)

  • Query_Processor_Output: 22 member variables
  • MySQL_Protocol: 4 member variables
  • Query_Info: 4 member variables
  • MySQL_Query_Processor_Output: 2 member variables
  • stmt_execute_metadata_t: 2 member variables

Buffer/Format Bugs

  • wrongPrintfScanfArgNum: 4 fixes - removed extra arguments from sprintf
  • invalidPrintfArgType: 2 fixes - corrected %d/%u mismatches
  • invalidFunctionArg: Fixed memcpy with negative size

Null Pointer Bugs

  • nullPointerRedundantCheck: 2 fixes - improved null handling
  • nullPointer: 2 fixes - added null checks before use
  • arrayIndexOutOfBounds: Fixed out-of-bounds array access

3. Performance Improvements

  • useInitializationList: 15+ constructors converted to use initializer lists

Files Changed

77 files changed, 1821 insertions(+), 428 deletions(-)

Key Files Modified

  • include/*.h (37 files) - Include guards, nullptr, enum fixes
  • lib/MySQL_HostGroups_Manager.cpp - Memory safety, printf bugs
  • lib/MySQL_Session.cpp - Null pointers, array bounds, copy ops
  • lib/MySQL_Authentication.cpp - Member initialization
  • include/query_processor.h - Uninitialized members
  • include/gen_utils.h - Memory leak fix
  • scripts/lint/ - New lint scripts added

New Lint Infrastructure

Added comprehensive lint scripts (from earlier PR):

  • scripts/lint/run-local.sh - Local lint runner
  • scripts/lint/normalize-clang-tidy.py - Normalize clang-tidy output
  • scripts/lint/normalize-cppcheck.py - Normalize cppcheck output
  • scripts/lint/generate-compile-commands.sh - Generate compile commands

Testing

  • All changes compile successfully with make debug
  • cppcheck reports now show only 5 warnings (all architectural - inherited method overrides)
  • No functional changes - all fixes are static analysis improvements

Before/After Cppcheck

Metric Before After
uninitMemberVar ~50 0
noCopyConstructor 15 0
wrongPrintfScanfArgNum 4 0
nullPointer issues 6 0
memleakOnRealloc 1 0
Total warnings ~100+ 5

Notes

  • Excluded third-party code (deps/json, deps/openssl, etc.)
  • Some style-only warnings skipped (noExplicitConstructor, cstyleCast) - too many to fix meaningfully
  • All changes are backward compatible - no API changes

Summary by CodeRabbit

  • New Features

    • Local static-analysis workflow: generate compile DB, run clang-tidy/cppcheck, normalize reports; Makefile targets to run it.
  • Documentation

    • Added rollout plans, cppcheck fixes roadmap, and linting design/spec covering baseline, suppression policy, and CI annotation workflow.
  • Refactor

    • Modernized headers and internals (guard names, nullptr defaults, enum sizing, safer copy semantics) for code quality and consistency.
  • Chores

    • Added .clang-tidy and ignore lint artifacts (lint/).

- MySQL_Thread.h: wrap negative hostgroup constants in parens
- PgSQL_Thread.h: wrap negative hostgroup constants in parens
- proxysql_admin.h: wrap expression multipliers in parens
- proxysql_debug.h: wrap macro args in ASSERT_SQLITE* macros
- proxysql_macros.h: wrap macro args in strdup_null, free_null, itostr, Copy4B
- proxysql_structs.h: wrap macro args in IS_PGTRACKED_VAR_OPTION_* macros
Use static_cast<int> instead of C-style cast for clarity and to avoid
unsigned->signed narrowing warnings while preserving the -1 sentinel.
Replace __CLASS_* and ____CLASS_* patterns with PROXYSQL_*_H to comply
with the C++ standard where identifiers starting with __ are reserved
in the global namespace.

Also fixes:
- ____CLASS_STANDARD_MYSQL_THREAD_H -> PROXYSQL_STANDARD_MYSQL_THREAD_H
- ____CLASS_STANDARD_PGSQL_THREAD_H -> PROXYSQL_STANDARD_PGSQL_THREAD_H
- __CLASS_PROXYSQL_POLL -> PROXYSQL_POLL_H (added _H for consistency)
Rename ROS__SIZE -> ROS_SIZE_, RLS__SIZE -> RLS_SIZE_, etc. to avoid
the reserved identifier pattern (double underscore at start or in name).
Rename __active -> active_, __frontend -> frontend_, __backend -> backend_
in authentication and hostgroups classes to comply with C++ reserved
identifier rules.

Includes updates to:
- MySQL_Authentication.hpp
- ClickHouse_Authentication.hpp
- PgSQL_Authentication.h
- MySQL_HostGroups_Manager.h
- Base_HostGroups_Manager.h
- Corresponding .cpp files
performance-enum-size fixes:
- p_mysql_error_type: add uint8_t underlying type (2 values)
- p_hg_dyn_gauge: add uint8_t underlying type (5 values)
- p_hg_dyn_counter: add uint8_t underlying type (9 values)

bugprone-reserved-identifier fixes:
- Rename __size -> SIZE_ in metric enums to avoid reserved identifier
  (names starting with __ are reserved in C++)
- Update all std::array size references using these enum values
…hostgroups

performance-enum-size fixes:
- p_mysql_error_type: add uint8_t underlying type (2 values)
- p_pgsql_error_type: add uint8_t underlying type (2 values)
- p_hg_dyn_gauge: add uint8_t underlying type (5 values)
- p_hg_dyn_counter: add uint8_t underlying type (9 values)
- PgSQL_p_hg_dyn_gauge: add uint8_t underlying type (5 values)
- PgSQL_p_hg_dyn_counter: add uint8_t underlying type (9 values)

bugprone-reserved-identifier fixes:
- Rename __size -> SIZE_ in all metric enums to avoid reserved identifier
- Update std::array size references to use new SIZE_ enum values

Applies same fixes as MySQL_HostGroups_Manager.h to:
- Base_HostGroups_Manager.h (base class)
- PgSQL_HostGroups_Manager.h (PostgreSQL implementation)
…, admin, thread, debug

performance-enum-size fixes:
- p_qc_counter, p_qc_gauge: add uint8_t (7 and 2 values)
- p_admin_counter, p_admin_gauge, p_admin_dyn_counter, p_admin_dyn_gauge: add uint8_t
- p_debug_dyn_counter: add uint8_t (2 values)

bugprone-reserved-identifier fixes:
- Rename __size -> SIZE_ in all metric enums
- Update std::array size references across multiple files
- Update prometheus_helpers.h template parameter names and comments

Files changed:
- Query_Cache.h
- proxysql_admin.h
- MySQL_Thread.h
- proxysql_debug.h
- prometheus_helpers.h (template changes)
- debug.cpp (usage update)
bugprone-reserved-identifier fixes:
- gen_utils.h: rename __size parameter to 'sz' in PtrArray and PtrSizeArray
  constructors (parameter names starting with __ are reserved)

- prometheus_helpers.h: update documentation comments to reference
  '::SIZE_' instead of '::__size' to match the actual enum values
modernize-use-nullptr: replace NULL with nullptr for type-safe null pointer
constants. This is a C++11 modernization that improves type safety.

Files changed:
- Base_HostGroups_Manager.h
- ClickHouse_Authentication.hpp
- MySQL_HostGroups_Manager.h
- MySQL_LDAP_Authentication.hpp
- MySQL_Monitor.hpp
- MySQL_Session.h
- PgSQL_Error_Helper.h
- PgSQL_HostGroups_Manager.h
- PgSQL_Session.h
Remove lint output files from git tracking - these are generated files
that should not be committed. Keep only SUPPRESSIONS.md and .gitkeep.
- Rename __HGM_TABLES_SIZE -> HGM_TABLES_SIZE_ (Base/MySQL/PgSQL HGM)
- Rename __TYPE_SIZE -> TYPE_SIZE_ (Base/MySQL/PgSQL HGM)
- Fix include guard defines: __PROXYSQL_* -> PROXYSQL_*
- Fix include guard comments: __CLASS_* -> PROXYSQL_*_H (29 files)
- Add uint8_t underlying type to small enums
- Rename __HGM_TABLES_SIZE -> HGM_TABLES_SIZE_ (Base/MySQL/PgSQL HGM)
- Rename __TYPE_SIZE -> TYPE_SIZE_ (Base/MySQL/PgSQL HGM)
- Fix include guard defines: __PROXYSQL_* -> PROXYSQL_*
- Fix include guard comments: __CLASS_* -> PROXYSQL_*_H (29 files)
- Add uint8_t underlying type to small enums
…era_Info, AWS_Aurora_Info, MySQL_Errors_stats)
- Fix invalidPrintfArgType: %d->%u for hid, %u->%d for use_ssl
- Fix nullPointerRedundantCheck: replace assert with continue
- Add deleted copy ctor/operator= to KillArgs and MySQL_Session
- Fix invalidFunctionArg: add query_len > 3 check before memcpy
- Fix wrongPrintfScanfArgNum: remove extra args in 4 sprintf calls
- Fix nullPointer: add GloMTH check, add null checks for strdup args
- All high-risk cppcheck warnings now resolved
…unds

- Fix invalidLifetime: look up pointer from map after insertion, not before
- Fix arrayIndexOutOfBounds: add bounds check before array access
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0755ba66-a38d-463f-83ea-e581f08aaff8

📥 Commits

Reviewing files that changed from the base of the PR and between 24e56d4 and 76251a7.

📒 Files selected for processing (1)
  • lib/MySQL_Session.cpp
✅ Files skipped from review due to trivial changes (1)
  • lib/MySQL_Session.cpp
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: CI-builds / builds (debian12,-dbg)
  • GitHub Check: CI-builds / builds (ubuntu22,-tap)
  • GitHub Check: CI-builds / builds (ubuntu24,-tap-genai-gcov)
  • GitHub Check: run / trigger
  • GitHub Check: claude-review

📝 Walkthrough

Walkthrough

Adds repository-wide static-analysis tooling and CI workflow: top-level .clang-tidy, lint scripts and Make targets, normalization scripts, docs/specs/plans, and widespread C++ source/header modernizations (include-guard renames, nullptr adoption, enum sizing/sentinels, deleted copies, macro safety, initializer fixes) with matching implementation updates.

Changes

Cohort / File(s) Summary
Lint config & infra
\.clang-tidy, \.gitignore, Makefile
Add repo .clang-tidy, ignore lint/, and new Make targets: lint-generate-cdb, lint-run, lint.
Lint scripts
scripts/lint/generate-compile-commands.sh, scripts/lint/run-local.sh, scripts/lint/normalize-clang-tidy.py, scripts/lint/normalize-cppcheck.py
New scripts to produce compile_commands.json, run per-file clang-tidy/cppcheck, merge exports, and normalize outputs into stable reports under lint/.
Docs / plans / specs
docs/superpowers/plans/*, docs/superpowers/specs/*
Add rollout plan, cppcheck-fix checklist, and linting design/spec including CI workflow, baseline/suppression strategy, and developer guidance.
Header guards & trivial header edits
include/* (many, e.g., include/MySQL_Thread.h, include/proxysql_admin.h, include/proxysql_utils.h, ...)
Rename include-guard macros from __* to PROXYSQL_* across many headers; adjust mismatched #endif comments.
Null modernizations
include/*, lib/* (e.g., MySQL_*, PgSQL_*, ClickHouse_*)
Replace NULL with nullptr in defaults/returns/initializers and update default pointer arguments to nullptr.
Enum sizing & sentinels
include/*, lib/* (e.g., ProxySQL_Cluster.hpp, Query_Cache.h, Base_HostGroups_Manager.h, prometheus_helpers.h)
Convert many metric/index enums to fixed underlying types (: uint8_t) and rename terminal sentinels from __size/__...SIZE to SIZE_/..._SIZE; update std::array sizes and comparisons.
Member renames (active/frontend/backend)
include/*, lib/* (auth/hostgroup files like MySQL_Authentication.*, PgSQL_Authentication.*, ClickHouse_Authentication.*)
Rename struct fields __active, __frontend, __backendactive_, frontend_, backend_ and update callers/implementations.
Non-copyable safety
include/*, lib/* (e.g., MySQL_Session.h, ProxySQL_Poll.h, hostgroup structs)
Delete copy constructors/assignment operators for resource-owning types to prevent accidental copies.
Constructors & initialization
include/*, lib/* (e.g., MySQL_Protocol.h, Servers_SslParams.h, query_processor.h, MySQL_PreparedStatement.h)
Refactor constructors to member-initializer lists, tighten defaults, and fix initialization/order semantics.
Macro & expression safety
include/proxysql_macros.h, include/proxysql_structs.h, others
Parenthesize macro parameters/values and numeric macro expressions for safer expansions.
Array/template signature updates
include/prometheus_helpers.h, include/gen_utils.h, include/*
Update function/template signatures to reference new SIZE_ sentinels; improve PtrArray::shrink() realloc safety and mark small constructors explicit.
Implementation alignment
lib/*.cpp (e.g., MySQL_Authentication.cpp, MySQL_HostGroups_Manager.cpp, MySQL_Session.cpp, ProxySQL_Admin.cpp)
Align implementations with header changes: renamed members, bounds checks, safer memory handling, small formatting/printf fixes, and logic hardening.

Sequence Diagram(s)

sequenceDiagram
  participant Dev as Developer
  participant Make as Makefile ("make lint")
  participant ScriptGen as scripts/lint/generate-compile-commands.sh
  participant Bear as bear
  participant RunLocal as scripts/lint/run-local.sh
  participant ClangTidy as clang-tidy
  participant Cppcheck as cppcheck
  participant Normalize as normalize scripts
  participant LintDir as lint/ (artifacts)

  Dev->>Make: run `make lint`
  Make->>ScriptGen: run generate-compile-commands.sh
  ScriptGen->>Bear: wrap build to produce compile_commands.json
  Bear-->>ScriptGen: compile_commands.json
  ScriptGen-->>Make: done
  Make->>RunLocal: run run-local.sh
  RunLocal->>ClangTidy: run clang-tidy per file (--export-fixes)
  RunLocal->>Cppcheck: run cppcheck --project=compile_commands.json
  ClangTidy->>Normalize: export YAMLs -> normalize-clang-tidy.py
  Cppcheck->>Normalize: raw output -> normalize-cppcheck.py
  Normalize->>LintDir: write normalized `clang-tidy.txt`, `cppcheck.txt`
  LintDir-->>Dev: artifacts for review/CI upload
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐇 I hopped through headers, guards made neat,
nullptr tucked in where old NULLs would meet,
Linters whisper, scripts collect each line,
Baselines set, diagnostics align,
A rabbit's nibble — tidy code, so sweet!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.37% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title clearly and specifically describes the main changes: applying lint and static analysis fixes (clang-tidy and cppcheck) to the codebase. It accurately reflects the comprehensive refactoring detailed in the PR objectives.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch v3.0-lint

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a static analysis framework using clang-tidy and cppcheck, incorporating configuration files, implementation plans, and build system integration. The changes feature extensive codebase modernization, including the adoption of nullptr, updated header guards, and the use of initialization lists. Furthermore, several bugs were resolved, such as a use-after-move error, potential null pointer dereferences, and incorrect arguments in sprintf and fprintf calls. Feedback indicates that an unreachable logic block in the run-local.sh script should be removed.

Comment thread scripts/lint/run-local.sh
Comment on lines +80 to +89
else
# Merge all per-file stderr clang-tidy outputs if present
if ls ${TMPD:-/dev/null}/ct-*.stderr >/dev/null 2>&1; then
cat ${TMPD}/ct-*.stderr > lint/clang-tidy-raw.txt || true
python3 scripts/lint/normalize-clang-tidy.py lint/clang-tidy-raw.txt > lint/clang-tidy.txt || true
else
# Nothing found
:
fi
fi
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

This else block appears to be unreachable. The Python script at lines 53-66 always creates lint/clang-tidy-fixes.yaml, so the condition [ -f lint/clang-tidy-fixes.yaml ] on line 78 will always evaluate to true. The logic for handling cases with no diagnostics should be handled within the normalize-clang-tidy.py script when parsing the (potentially empty) YAML file. This else block can be removed.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Note

Due to the large number of review comments, Critical severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (7)
include/proxysql_restapi.h (1)

43-43: ⚠️ Potential issue | 🟡 Minor

#endif comment is outdated and inconsistent.

Line 43 references __PROXYSQL_RESTAPI_H__, but the current guard macro is PROXYSQL_RESTAPI_H. Please align the trailing comment with the actual guard used.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_restapi.h` at line 43, The trailing comment on the include
guard is inconsistent: change the outdated "__PROXYSQL_RESTAPI_H__" reference to
match the actual guard macro "PROXYSQL_RESTAPI_H" used in the file; update the
comment after the `#endif` to read "// PROXYSQL_RESTAPI_H" so the closing comment
matches the opening guard (refer to the guard macro PROXYSQL_RESTAPI_H in the
file).
include/proxysql_sslkeylog.h (1)

121-121: ⚠️ Potential issue | 🟡 Minor

Closing guard comment no longer matches the defined macro.

Line 121 still references __PROXYSQL_SSLKEYLOG_H while the active guard is PROXYSQL_SSLKEYLOG_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_sslkeylog.h` at line 121, The closing header-guard comment
is inconsistent: the active macro is PROXYSQL_SSLKEYLOG_H but the trailing
comment on the `#endif` uses __PROXYSQL_SSLKEYLOG_H; update the `#endif` comment to
match the defined macro (PROXYSQL_SSLKEYLOG_H) so the guard name is consistent
(look for the `#ifndef/`#define pair and the final `#endif` in the
proxysql_sslkeylog header).
include/LLM_Bridge.h (1)

337-337: ⚠️ Potential issue | 🟡 Minor

Fix stale #endif comment to match the active guard.

Line 337 still references __CLASS_LLM_BRIDGE_H while the active macro at Line 28 is PROXYSQL_LLM_BRIDGE_H, which makes maintenance/error-tracing harder.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/LLM_Bridge.h` at line 337, The closing preprocessor comment is stale:
replace the comment on the final `#endif` so it matches the active include guard
macro PROXYSQL_LLM_BRIDGE_H (not __CLASS_LLM_BRIDGE_H); update the comment after
the `#endif` in include/LLM_Bridge.h to read // PROXYSQL_LLM_BRIDGE_H to keep the
guard name consistent with the opening `#ifndef/`#define for
PROXYSQL_LLM_BRIDGE_H.
include/proxysql_coredump.h (1)

27-27: ⚠️ Potential issue | 🟡 Minor

Normalize trailing #endif comment to the actual macro.

Line 27 still references __PROXYSQL_COREDUMP_H, which is inconsistent with the current defined guard.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_coredump.h` at line 27, The trailing endif comment currently
reads "#endif // __PROXYSQL_COREDUMP_H" but doesn't match the header guard
actually declared at the top of the file; update the comment on the closing
`#endif` to exactly match the macro used in the `#ifndef/`#define guard (replace
__PROXYSQL_COREDUMP_H with the actual guard macro name present at the top of
this header) so the closing comment reflects the correct symbol.
include/MySQL_Protocol.h (1)

145-151: ⚠️ Potential issue | 🟠 Major

Initialize dump_pkt in the constructor under #ifdef DEBUG.

Under #ifdef DEBUG, dump_pkt is declared but not initialized by the constructor or init() method. Multiple methods read this member with if (dump_pkt) {...} checks throughout the codebase. The explicit assignment in Base_Thread.cpp::138 happens only after the object is already in use, creating undefined behavior for any reads before that point.

Suggested fix
 MySQL_Protocol()
-  : userinfo(nullptr), sess(nullptr), myds(nullptr), current_PreStmt(nullptr)
+  : userinfo(nullptr), sess(nullptr), myds(nullptr)
+#ifdef DEBUG
+  , dump_pkt(false)
+#endif
+  , current_PreStmt(nullptr)
  {
 		sent_auth_plugin_id = AUTH_MYSQL_NATIVE_PASSWORD;
 		auth_plugin_id = AUTH_UNKNOWN_PLUGIN;
 		prot_status=0;
 		more_data_needed = false;
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Protocol.h` around lines 145 - 151, The MySQL_Protocol
constructor (and/or init()) must explicitly initialize the DEBUG-only member
dump_pkt to a safe default to avoid undefined reads; update the MySQL_Protocol()
initializer or body to set dump_pkt = nullptr (or false as appropriate) under
`#ifdef` DEBUG so that any code checking "if (dump_pkt) { ... }" before
Base_Thread.cpp later assigns it will behave deterministically; ensure the
initialization is present in the same places where other members (userinfo,
sess, myds, current_PreStmt, sent_auth_plugin_id, auth_plugin_id, prot_status,
more_data_needed) are set.
include/MySQL_LDAP_Authentication.hpp (1)

4-11: ⚠️ Potential issue | 🟡 Minor

Rename enum member __SIZE to SIZE_ to comply with reserved identifier guidelines.

The enum sentinel __SIZE at line 9 uses a reserved identifier (double underscore prefix) and should be renamed to SIZE_ per the PR objectives and C++ naming conventions for constants. The member is unused in the codebase, making this refactoring safe.

Suggested fix
 struct LDAP_USER_FIELD_IDX {
 	enum index {
 		USERNAME = 0,
 		FRONTEND_CONNECTIONS = 1,
 		FRONTED_MAX_CONNECTIONS = 2,
-		__SIZE
+		SIZE_
 	};
 };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_LDAP_Authentication.hpp` around lines 4 - 11, The enum sentinel
in LDAP_USER_FIELD_IDX::index uses a reserved identifier __SIZE; rename that
member to SIZE_ (replace __SIZE with SIZE_) across the enum declaration and any
references to it so the identifier no longer begins with double underscore;
update LDAP_USER_FIELD_IDX and any code that might reference
LDAP_USER_FIELD_IDX::index::__SIZE to LDAP_USER_FIELD_IDX::index::SIZE_.
lib/MySQL_Authentication.cpp (1)

17-29: ⚠️ Potential issue | 🔴 Critical

Bug: Duplicate ad.password free block causes potential double-free.

Lines 26-29 duplicate the exact same logic as lines 18-21, freeing ad.password twice. This will cause undefined behavior (double-free) if ad.password is non-null.

🐛 Proposed fix: Remove duplicate block
 void free_account_details(account_details_t& ad) {
 	if (ad.password) {
 		free(ad.password);
 		ad.password = nullptr;
 	}
 	if (ad.sha1_pass) {
 		free(ad.sha1_pass);
 		ad.sha1_pass=NULL;
 	}
-	if (ad.password) {
-		free(ad.password);
-		ad.password = nullptr;
-	}
 	if (ad.clear_text_password[PASSWORD_TYPE::PRIMARY]) {
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Authentication.cpp` around lines 17 - 29, The function
free_account_details contains a duplicate block that frees ad.password twice;
remove the second duplicate if-check and free so ad.password is only freed once
and then set to nullptr, leaving the ad.sha1_pass free logic intact; update the
function free_account_details (operating on account_details_t and its members
ad.password and ad.sha1_pass) to ensure each pointer is freed exactly once and
nulled afterwards.
🟠 Major comments (36)
include/StatCounters.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Guard rename breaks mandated include-guard convention.

The new PROXYSQL_STAT_COUNTERS_H guard does not follow the required __CLASS_*_H pattern for include/ headers.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

Also applies to: 74-74

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/StatCounters.h` around lines 1 - 2, The include guard in
include/StatCounters.h currently uses PROXYSQL_STAT_COUNTERS_H which violates
the mandated pattern; rename the guard to follow the required __CLASS_*_H
convention (e.g., use __CLASS_STATCOUNTERS_H or __CLASS_STAT_COUNTERS_H
consistently) and update both the `#ifndef` and corresponding `#define` and the
matching `#endif` comment if present; ensure the symbol used matches elsewhere in
the file (replace PROXYSQL_STAT_COUNTERS_H wherever referenced) so the header
conforms to the include/**/*.{h,hpp} guideline.
include/AI_Vector_Storage.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Header guard format conflicts with include-directory rule.

PROXYSQL_AI_VECTOR_STORAGE_H does not match the required __CLASS_*_H include-guard convention for headers under include/.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/AI_Vector_Storage.h` around lines 1 - 2, The header guard macro
PROXYSQL_AI_VECTOR_STORAGE_H does not follow the required include-guard
convention for files under include/; update both occurrences of the guard (the
`#ifndef` and matching `#define`) in include/AI_Vector_Storage.h to the mandated
pattern using the __CLASS_*_H style (e.g., replace PROXYSQL_AI_VECTOR_STORAGE_H
with an appropriately named macro like __CLASS_AI_VECTOR_STORAGE_H) and ensure
the corresponding `#endif` comment matches the new macro.
include/MySQL_Query_Cache.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard does not match required repository pattern.

PROXYSQL_MYSQL_QUERY_CACHE_H conflicts with the enforced __CLASS_*_H format for headers in include/.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

Also applies to: 26-26

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Query_Cache.h` around lines 1 - 2, The include guard macro in
this header uses PROXYSQL_MYSQL_QUERY_CACHE_H which violates the repo pattern;
rename the guard to follow `#ifndef` __CLASS_*_H (e.g.,
__CLASS_MYSQL_QUERY_CACHE_H) and update the matching `#define` and trailing `#endif`
comment to the new macro; ensure the symbol references in this file
(MySQL_Query_Cache.h) are consistently replaced so the ifndef/define/endif trio
uses __CLASS_MYSQL_QUERY_CACHE_H.
include/PgSQL_Protocol.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard rename violates include-header convention.

PROXYSQL_PGSQL_PROTOCOL_H does not match the required __CLASS_*_H pattern for files under include/.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Protocol.h` around lines 1 - 2, The include guard
PROXYSQL_PGSQL_PROTOCOL_H in PgSQL_Protocol.h violates the project's
include-header convention; rename the guard to follow the __CLASS_*_H pattern
(for example __CLASS_PGSQL_PROTOCOL_H) by replacing the current `#ifndef` and
`#define` macros and update the matching `#endif` comment if present so the header
now uses __CLASS_PGSQL_PROTOCOL_H consistently throughout the file.
include/mysql_connection.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard naming violates repository include-header rule.

This guard was renamed to PROXYSQL_MYSQL_CONNECTION_H, but the include-path guideline requires the __CLASS_*_H pattern.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

Also applies to: 279-279

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/mysql_connection.h` around lines 1 - 2, The include guard in
include/mysql_connection.h currently uses PROXYSQL_MYSQL_CONNECTION_H which
violates the repo rule; rename the guard everywhere to the required pattern
(e.g. __CLASS_MYSQL_CONNECTION_H) so the `#ifndef`, matching `#define`, and the
trailing `#endif` comment use the new symbol; update references to
PROXYSQL_MYSQL_CONNECTION_H in this file to __CLASS_MYSQL_CONNECTION_H to
conform with the include-header guideline.
include/mysql_backend.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Header guard naming is out of policy for include/ headers.

The new PROXYSQL_MYSQL_BACKEND_H guard should follow the required __CLASS_*_H convention.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

Also applies to: 25-25

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/mysql_backend.h` around lines 1 - 2, The header guard macro
PROXYSQL_MYSQL_BACKEND_H does not follow the required include pattern; rename
the guard to the __CLASS_*_H style (e.g., __CLASS_MYSQL_BACKEND_H) by updating
the `#ifndef` and matching `#define` at the top and any matching `#endif` comment at
the bottom so the guard consistently uses __CLASS_MYSQL_BACKEND_H instead of
PROXYSQL_MYSQL_BACKEND_H (ensure you update all occurrences in the file,
including any trailing comment after `#endif`).
include/proxysql_config.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard no longer complies with required pattern.

PROXYSQL_CONFIG_H does not satisfy the __CLASS_*_H include-guard rule defined for headers in include/.

As per coding guidelines, include/**/*.{h,hpp}: "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_config.h` around lines 1 - 2, The include guard
PROXYSQL_CONFIG_H doesn't follow the required pattern; replace both occurrences
of PROXYSQL_CONFIG_H in the header's guards with the mandated pattern
__CLASS_PROXYSQL_CONFIG_H (update the `#ifndef` and matching `#define` and ensure
the trailing `#endif` comment, if any, references the new symbol) so the header
conforms to the include/ include-guard rule.
include/LLM_Bridge.h-28-29 (1)

28-29: ⚠️ Potential issue | 🟠 Major

Header guard format violates include-path policy.

Line 28 and Line 29 use PROXYSQL_LLM_BRIDGE_H; this header path requires the __CLASS_*_H include-guard format.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/LLM_Bridge.h` around lines 28 - 29, The header guard macro
PROXYSQL_LLM_BRIDGE_H in LLM_Bridge.h violates the project's include-path
policy; replace it with the required pattern using the __CLASS_*_H format (e.g.,
change both the `#ifndef` and `#define` from PROXYSQL_LLM_BRIDGE_H to a guard
following __CLASS_<UNIQUE>_H and update the corresponding `#endif` comment if
present) so the guard matches the coding guideline for include/**/*.{h,hpp}.
include/proxysql_restapi.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Use __CLASS_*_H include guards for files under include/.

Line 1 and Line 2 currently define PROXYSQL_RESTAPI_H; this does not match the repository guard policy for this path.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_restapi.h` around lines 1 - 2, The current include guard
symbol PROXYSQL_RESTAPI_H does not follow the repository policy; change the
guards to the required pattern by replacing PROXYSQL_RESTAPI_H with
__CLASS_PROXYSQL_RESTAPI_H in the header (update both the `#ifndef` and `#define` at
the top and the matching `#endif` comment if present) so the file uses `#ifndef`
__CLASS_PROXYSQL_RESTAPI_H / `#define` __CLASS_PROXYSQL_RESTAPI_H and the
corresponding `#endif`.
include/PgSQL_Backend.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Use the repository-required include guard pattern.

Lines 1-2 use PROXYSQL_PGSQL_BACKEND_H, but this path is under include/ and the required guard format is __CLASS_*_H. Please switch this guard back to the mandated pattern for consistency and policy compliance.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 25-25

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Backend.h` around lines 1 - 2, Change the non-conforming
include guard PROXYSQL_PGSQL_BACKEND_H to the mandated pattern by replacing the
`#ifndef` and `#define` with a guard using the __CLASS_*_H format (e.g.,
__CLASS_PGSQL_BACKEND_H) and update the matching `#endif` comment/identifier
accordingly so both the opening and closing guard symbols match; modify the
macros surrounding the PgSQL_Backend declarations (the existing
PROXYSQL_PGSQL_BACKEND_H symbols and the corresponding `#endif`) to the new
__CLASS_PGSQL_BACKEND_H macro.
include/PgSQL_Connection.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Please revert to the mandated include-guard format for include/ headers.

Line 1 and Line 2 use PROXYSQL_PGSQL_CONNECTION_H, but the repository rule for this path requires guards in __CLASS_*_H format.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 743-743

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Connection.h` around lines 1 - 2, Replace the current include
guard macro PROXYSQL_PGSQL_CONNECTION_H with the mandated pattern
__CLASS_PgSQL_Connection_H in this header; update both the `#ifndef` and `#define`
lines to use __CLASS_PgSQL_Connection_H and adjust the matching `#endif` comment
(if present) to reference __CLASS_PgSQL_Connection_H so the guard names are
consistent.
include/PgSQL_Data_Stream.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard naming is non-compliant for headers under include/.

Line 1 and Line 2 use PROXYSQL_PGSQL_DATA_STREAM_H; the project rule for headers in this path requires __CLASS_*_H guards.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 272-272

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Data_Stream.h` around lines 1 - 2, The include guard macro
PROXYSQL_PGSQL_DATA_STREAM_H does not follow the project's include path
convention; replace it with the required pattern by renaming the guard to
__CLASS_PGSQL_DATA_STREAM_H (update both the `#ifndef/`#define at the top and the
corresponding `#endif` comment at the bottom) so the header uses
__CLASS_PGSQL_DATA_STREAM_H consistently; locate the guards in the file
(symbols: PROXYSQL_PGSQL_DATA_STREAM_H, the opening `#ifndef/`#define and the
closing `#endif`) and update them to the new macro.
include/proxysql_sslkeylog.h-17-18 (1)

17-18: ⚠️ Potential issue | 🟠 Major

Include guard style does not follow required project convention.

Line 17 and Line 18 use PROXYSQL_SSLKEYLOG_H; headers in include/ are required to use the __CLASS_*_H pattern.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_sslkeylog.h` around lines 17 - 18, The include guard in
include/proxysql_sslkeylog.h uses PROXYSQL_SSLKEYLOG_H but must follow the
project convention using the __CLASS_*_H pattern; update the guard macro (both
the `#ifndef` and `#define` and the corresponding `#endif` comment) to use the
required pattern (e.g., replace PROXYSQL_SSLKEYLOG_H with
__CLASS_PROXYSQL_SSLKEYLOG_H) so the header's include guard matches the
__CLASS_*_H naming convention throughout (ensure you update the matching
identifier wherever referenced in this file).
include/sqlite3db.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard naming does not comply with current repository rule.

Line 1 and Line 2 define PROXYSQL_SQLITE3DB_H; this path requires include guards in __CLASS_*_H format.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 250-250

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/sqlite3db.h` around lines 1 - 2, The current include guard macro
PROXYSQL_SQLITE3DB_H does not follow the repository pattern; change the guard to
the required format by replacing PROXYSQL_SQLITE3DB_H with __CLASS_SQLITE3DB_H
in both the `#ifndef` and `#define` at the top of include/sqlite3db.h, and update
the matching `#endif` comment at the bottom (if present) to reference
__CLASS_SQLITE3DB_H so the pair remains consistent; ensure you also update any
repeated occurrences of the old macro in the same file.
include/proxysql_coredump.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Guard macro naming violates include-header policy.

Line 1 and Line 2 define PROXYSQL_COREDUMP_H; for headers in include/, guard macros are required to follow __CLASS_*_H.

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_coredump.h` around lines 1 - 2, The include guard macro
PROXYSQL_COREDUMP_H does not follow the required pattern; rename the guard to
the policy-compliant form (e.g., __CLASS_PROXYSQL_COREDUMP_H) and update the
matching `#ifndef`, `#define` and closing `#endif` comment to use the new macro;
ensure all three occurrences in include/proxysql_coredump.h reference
__CLASS_PROXYSQL_COREDUMP_H so the header aligns with the include/**/*.{h,hpp}
guard naming convention.
include/Command_Counter.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Header guard format should match the repository include policy.

Line 1, Line 2, and Line 63 should use the __CLASS_*_H pattern for headers under include/.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 63-63

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/Command_Counter.h` around lines 1 - 2, The include guard in
Command_Counter.h uses PROXYSQL_COMMAND_COUNTER_H but must follow the repo
pattern; replace the current `#ifndef/`#define/#endif with the __CLASS_*_H form
(use __CLASS_COMMAND_COUNTER_H) so the guard and the trailing `#endif` comment
match; update the symbol references in this header (e.g., the opening
`#ifndef/`#define and the closing `#endif` comment) to __CLASS_COMMAND_COUNTER_H to
comply with include/**/*.{h,hpp} guidelines.
include/Anomaly_Detector.h-35-36 (1)

35-36: 🛠️ Refactor suggestion | 🟠 Major

Please keep include guard naming consistent with the include/ rule set.

Line 35 and Line 36 should use the __CLASS_*_H guard format mandated for headers in this directory.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/Anomaly_Detector.h` around lines 35 - 36, The include guard in
Anomaly_Detector.h uses PROXYSQL_ANOMALY_DETECTOR_H but must follow the project
pattern __CLASS_*_H; update the guard around the file (the `#ifndef/`#define at
the top and the matching `#endif` at the bottom) to use __CLASS_ANOMALY_DETECTOR_H
so it conforms with the include/ rule set and matches the class/name in
Anomaly_Detector.h.
include/MySQL_Data_Stream.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Align include guard with the repository’s include-header convention.

Line 1 and Line 2 changed the guard to PROXYSQL_MYSQL_DATA_STREAM_H; Line 290 mirrors it. The current repo rule for headers under include/ expects __CLASS_*_H.

Proposed fix
-#ifndef PROXYSQL_MYSQL_DATA_STREAM_H
-#define PROXYSQL_MYSQL_DATA_STREAM_H
+#ifndef __CLASS_MYSQL_DATA_STREAM_H
+#define __CLASS_MYSQL_DATA_STREAM_H
...
-#endif /* PROXYSQL_MYSQL_DATA_STREAM_H */
+#endif /* __CLASS_MYSQL_DATA_STREAM_H */

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 290-290

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Data_Stream.h` around lines 1 - 2, The include guard macro
PROXYSQL_MYSQL_DATA_STREAM_H does not follow the repository convention; replace
both occurrences of that macro (the opening `#ifndef/`#define and the matching
`#endif` identifier at the bottom) with a guard following the __CLASS_*_H pattern
used for headers under include/ (e.g., change PROXYSQL_MYSQL_DATA_STREAM_H to
the repository's __CLASS_<SOMETHING>_H style name that matches this header's
class/filename), ensuring the same identifier is used in the `#ifndef`, `#define`,
and trailing `#endif` comment.
include/PgSQL_Query_Cache.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Use the required __CLASS_*_H include-guard pattern here too.

Line 1, Line 2, and Line 22 currently use PROXYSQL_PGSQL_QUERY_CACHE_H, which diverges from the include-header guard convention configured for this repository.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 22-22

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Query_Cache.h` around lines 1 - 2, The include guard symbol
PROXYSQL_PGSQL_QUERY_CACHE_H does not follow the repository pattern; replace it
with __CLASS_PgSQL_Query_Cache_H in the `#ifndef` and `#define` at the top (and the
matching `#endif` at the bottom), ensuring the symbol exactly matches the
file/class name casing used in PgSQL_Query_Cache.h and update any trailing
comment on the `#endif` if present to use the same symbol.
include/PgSQL_Query_Processor.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Guard naming needs to follow the __CLASS_*_H rule for include/ headers.

Line 1, Line 2, and Line 45 currently use a PROXYSQL_* guard, which is out of policy for this path pattern.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 45-45

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Query_Processor.h` around lines 1 - 2, The header guard macro
PROXYSQL_PGSQL_QUERY_PROCESSOR_H used in PgSQL_Query_Processor.h does not follow
the include/ header policy; replace it with the required pattern (e.g.
__CLASS_PGSQL_QUERY_PROCESSOR_H) by updating the `#ifndef` and matching `#define` at
the top and the corresponding `#endif` comment at the bottom (and any
duplicate/secondary occurrence around the previous line 45) so the guard name
consistently uses the __CLASS_*_H format.
include/log_utils.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Please revert this include guard to the mandated __CLASS_*_H format.

Line 1, Line 2, and Line 221 currently use PROXYSQL_LOG_UTILS_H, which does not match the configured include guard rule for headers in include/.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 221-221

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/log_utils.h` around lines 1 - 2, The include guard macro
PROXYSQL_LOG_UTILS_H does not follow the mandated pattern; update the guard in
include/log_utils.h by replacing PROXYSQL_LOG_UTILS_H with the configured format
(e.g., __CLASS_LOG_UTILS_H) in the `#ifndef` and `#define` at the top and the
matching `#endif` comment at the bottom (the occurrence around line 221); ensure
all three occurrences use the exact same new macro name so the guard remains
consistent.
include/proxysql_utils.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Guard macro naming is out of policy for include/ headers.

Line 1 and Line 2 should follow the __CLASS_*_H guard format used by the repo rules for include/**/*.{h,hpp}.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_utils.h` around lines 1 - 2, The header guard macros in
include/proxysql_utils.h use PROXYSQL_UTILS_H which violates the repo policy;
rename the guard to follow the __CLASS_*_H pattern (e.g., change `#ifndef`
PROXYSQL_UTILS_H and `#define` PROXYSQL_UTILS_H to `#ifndef`
__CLASS_PROXYSQL_UTILS_H and `#define` __CLASS_PROXYSQL_UTILS_H) and update the
corresponding `#endif` comment if present so the opening and closing macros match
exactly.
include/PgSQL_Error_Helper.h-1-2 (1)

1-2: 🛠️ Refactor suggestion | 🟠 Major

Include guard should follow the repository __CLASS_*_H pattern.

Line 1, Line 2, and Line 672 currently use PROXYSQL_PGSQL_ERROR_HELPER_H, which diverges from the include-header guard requirement configured for this repo.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 672-672

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Error_Helper.h` around lines 1 - 2, Replace the current include
guard macro PROXYSQL_PGSQL_ERROR_HELPER_H with the repo-required pattern
__CLASS_PGSQL_ERROR_HELPER_H throughout the header (start guard, define, and the
trailing `#endif` comment), i.e., update the `#ifndef`, `#define` and the closing
`#endif` comment in include/PgSQL_Error_Helper.h to use
__CLASS_PGSQL_ERROR_HELPER_H so it matches the repository include guard
convention.
include/query_digest_topk.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

This include guard format conflicts with the configured header rule.

Line 1 and Line 2 should follow the __CLASS_*_H naming scheme required for headers in include/.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/query_digest_topk.h` around lines 1 - 2, The include guard in
include/query_digest_topk.h uses PROXYSQL_QUERY_DIGEST_TOPK_H which violates the
required pattern; rename the guard to follow the __CLASS_*_H scheme (e.g.,
__CLASS_QUERY_DIGEST_TOPK_H) and update the matching `#ifndef` and `#define` macros
(and the trailing `#endif` comment if present) so the header uses the new
__CLASS_QUERY_DIGEST_TOPK_H identifier consistently.
include/PgSQL_Thread.h-1-3 (1)

1-3: ⚠️ Potential issue | 🟠 Major

Header guard naming is out of policy for include/ headers.

Line 1 and Line 2 should use the __CLASS_*_H include-guard pattern expected by the repository rules.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Thread.h` around lines 1 - 3, The header guards in
include/PgSQL_Thread.h currently use PROXYSQL_PGSQL_THREAD_H /
PROXYSQL_STANDARD_PGSQL_THREAD_H and must be changed to follow the repository
pattern; replace both `#ifndef` and `#define` with a single guard named
__CLASS_PGSQL_THREAD_H (and update the matching `#endif` comment if present) so
the file uses the required __CLASS_*_H include-guard convention for the
PgSQL_Thread header.
include/MySQL_Set_Stmt_Parser.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Revert this include guard to the expected __CLASS_*_H form.

Line 1 and Line 2 no longer match the include-guard naming convention enforced for headers under include/.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Set_Stmt_Parser.h` around lines 1 - 2, The include guard uses
PROXYSQL_MYSQL_SET_STMT_PARSER_H but must follow the project pattern `#ifndef`
__CLASS_*_H; change both the `#ifndef` and `#define` to use
__CLASS_MYSQL_SET_STMT_PARSER_H (and update the trailing `#endif` comment if
present) in the MySQL_Set_Stmt_Parser.h header so the guard matches the required
__CLASS_*_H naming convention.
include/ProxySQL_Poll.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Please keep this include guard in the expected __CLASS_*_H format.

Line 1 and Line 2 currently use a guard naming style that conflicts with the configured rule for headers under include/.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/ProxySQL_Poll.h` around lines 1 - 2, Replace the existing include
guard macros PROXYSQL_POLL_H with the required pattern by renaming both
occurrences to __CLASS_PROXYSQL_POLL_H (update the `#ifndef` and matching `#define`
and any trailing `#endif` comment if present) so the header uses the mandated
__CLASS_*_H format; ensure the macro name matches exactly in all places within
ProxySQL_Poll.h.
include/AI_Features_Manager.h-30-31 (1)

30-31: ⚠️ Potential issue | 🟠 Major

Use the repository-required include-guard pattern.

Line 30 and Line 31 use PROXYSQL_AI_FEATURES_MANAGER_H, but this header path is configured to require the __CLASS_*_H guard format.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/AI_Features_Manager.h` around lines 30 - 31, The include guard macro
PROXYSQL_AI_FEATURES_MANAGER_H in AI_Features_Manager.h doesn't follow the
required pattern; replace the guard with the repository standard (e.g.,
__CLASS_AI_FEATURES_MANAGER_H) by changing the `#ifndef` and `#define` to use
__CLASS_AI_FEATURES_MANAGER_H and update the matching `#endif` comment to
reference the same macro, ensuring the three occurrences (ifndef, define, endif)
are consistent.
include/PgSQL_Set_Stmt_Parser.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Keep include guards consistent with project convention.

Line 1 and Line 2 use PROXYSQL_* naming, but this directory’s rule requires __CLASS_*_H guard names.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/PgSQL_Set_Stmt_Parser.h` around lines 1 - 2, The include guard in
PgSQL_Set_Stmt_Parser.h uses PROXYSQL_PGSQL_SET_STMT_PARSER_H but must follow
the project pattern `#ifndef` __CLASS_*_H; update the guard to use
__CLASS_PgSQL_Set_Stmt_Parser_H (replace the existing `#ifndef` and `#define`) and
adjust the trailing `#endif` comment if present to match the new guard; ensure
references to the header’s guard correspond to the class/header name
PgSQL_Set_Stmt_Parser.
include/MCP_Thread.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Guard naming here should follow the repository include-header convention.

Line 1 and Line 2 should use the __CLASS_*_H form for this path.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MCP_Thread.h` around lines 1 - 2, The include guard currently uses
PROXYSQL_MCP_THREAD_H but must follow the repository pattern `#ifndef`
__CLASS_*_H; replace the guard macro in the top two lines with
__CLASS_MCP_THREAD_H (and update the matching `#endif` comment at the end of the
file to reference __CLASS_MCP_THREAD_H) so the macros used in MCP_Thread.h
follow the __CLASS_*_H convention.
include/GenAI_Thread.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

This include-guard rename breaks the configured naming rule for include/.

Line 1 and Line 2 should stay in the __CLASS_*_H guard pattern.

As per coding guidelines "Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/GenAI_Thread.h` around lines 1 - 2, The include guard in
include/GenAI_Thread.h uses PROXYSQL_GENAI_THREAD_H which violates the project
rule; replace the two lines using that guard with the project pattern by
renaming them to the __CLASS_*_H form (e.g., change both `#ifndef`
PROXYSQL_GENAI_THREAD_H and `#define` PROXYSQL_GENAI_THREAD_H to `#ifndef`
__CLASS_GENAI_THREAD_H and `#define` __CLASS_GENAI_THREAD_H) and update the
corresponding closing `#endif` comment (if present) to match the new
__CLASS_GENAI_THREAD_H symbol.
include/query_processor.h-187-196 (1)

187-196: ⚠️ Potential issue | 🟠 Major

Constructor initialization is dead code—remove or align with init() method.

The constructor initializer list (lines 189-193) sets fields to specific values (e.g., reconnect(true), retries(3), cache_ttl(0)), but init() immediately overwrites all these fields with -1 sentinels. Every instantiation in the codebase follows the pattern:

new Query_Processor_Output();
obj->init();

Even MCP_Query_Processor_Output explicitly calls init() in its constructor. The constructor initialization serves no purpose and should be removed or aligned with init() to avoid future confusion.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/query_processor.h` around lines 187 - 196, The constructor
Query_Processor_Output currently sets member defaults that are immediately
overwritten by init(); either remove the redundant initializer list and leave
the constructor empty or have the constructor call init() so there is a single
source of truth; update Query_Processor_Output() to either (a) drop the long
initializer list and implement an empty body, or (b) call this->init() from the
constructor body (and ensure MCP_Query_Processor_Output still calls init() only
if needed), keeping init() as the canonical initializer for all fields.
include/Query_Cache.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Include guard naming deviates from repo header-guard convention.

Line 1, Line 2, and Line 219 use PROXYSQL_QUERY_CACHE_H, but this file path matches the include-guard rule requiring __CLASS_*_H.

Suggested fix
-#ifndef PROXYSQL_QUERY_CACHE_H
-#define PROXYSQL_QUERY_CACHE_H
+#ifndef __CLASS_QUERY_CACHE_H
+#define __CLASS_QUERY_CACHE_H
...
-#endif /* PROXYSQL_QUERY_CACHE_H */
+#endif /* __CLASS_QUERY_CACHE_H */

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 219-219

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/Query_Cache.h` around lines 1 - 2, The include guard macro
PROXYSQL_QUERY_CACHE_H in Query_Cache.h deviates from the repo convention;
replace it with the required pattern (e.g., __CLASS_QUERY_CACHE_H) everywhere it
appears (the opening `#ifndef/`#define and the closing `#endif` comment) so the
guard uses the __CLASS_*_H form; update the macro name consistently in the
header (look for the symbols PROXYSQL_QUERY_CACHE_H and the matching `#endif`) to
prevent mismatched guards.
include/MySQL_Thread.h-1-3 (1)

1-3: ⚠️ Potential issue | 🟠 Major

Header guards do not match the configured include-guard pattern.

Line 1-3 and Line 765 switched to PROXYSQL_* guards, but this header is under include/ and should follow the __CLASS_*_H pattern.

Suggested fix
-#ifndef PROXYSQL_MYSQL_THREAD_H
-#define PROXYSQL_MYSQL_THREAD_H
-#define PROXYSQL_STANDARD_MYSQL_THREAD_H
+#ifndef __CLASS_MYSQL_THREAD_H
+#define __CLASS_MYSQL_THREAD_H
+#define ____CLASS_STANDARD_MYSQL_THREAD_H
...
-#endif /* PROXYSQL_MYSQL_THREAD_H */
+#endif /* __CLASS_MYSQL_THREAD_H */

As per coding guidelines, include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

Also applies to: 765-765

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Thread.h` around lines 1 - 3, The header guard macros at the
top and bottom use PROXYSQL_MYSQL_THREAD_H and PROXYSQL_STANDARD_MYSQL_THREAD_H
but must follow the include/ file pattern __CLASS_*_H; replace those PROXYSQL_*
macros with a single consistent guard matching the class/header name (e.g.,
__CLASS_MYSQL_THREAD_H) and update both the `#ifndef/`#define and the matching
`#endif` macro reference (the existing PROXYSQL_MYSQL_THREAD_H /
PROXYSQL_STANDARD_MYSQL_THREAD_H identifiers) so the macro names are identical
and conform to the __CLASS_*_H pattern used for files under include/.
include/proxysql_macros.h-7-7 (1)

7-7: 🛠️ Refactor suggestion | 🟠 Major

Remove local itostr redefinitions or update to match the safer header version.

Three files locally redefine itostr without the protective parentheses present in include/proxysql_macros.h:7:

  • lib/MySQL_HostGroups_Manager.cpp:25
  • lib/PgSQL_HostGroups_Manager.cpp:26
  • lib/Base_HostGroups_Manager.cpp:25

All three use { __s=char_malloc(32); sprintf(__s, "%lld", __i); } instead of the safer { (__s)=char_malloc(32); sprintf(__s, "%lld", (__i)); }. Remove these local redefinitions to use the header version or update all to match the safer implementation.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_macros.h` at line 7, The local macro redefinitions of itostr
in MySQL_HostGroups_Manager.cpp, PgSQL_HostGroups_Manager.cpp and
Base_HostGroups_Manager.cpp are unsafe and differ from the header's safer form;
remove the local definitions so the header's itostr is used, or update each
local macro to exactly match the header version (ensure parentheses around both
__s and __i as in ({ (__s)=char_malloc(32); sprintf(__s, "%lld", (__i)); })).
Locate the macro name itostr in those files, delete the local `#define`, or
replace it with the header-equivalent definition, and then rebuild to confirm no
warnings or redefinition conflicts.
include/proxysql_admin.h-1-2 (1)

1-2: ⚠️ Potential issue | 🟠 Major

Resolve include-guard policy mismatch.

This guard rename is consistent with the lint cleanup intent, but it conflicts with the current repository guard rule for headers under include/. Please align either the code or the guideline to avoid recurring churn.

As per coding guidelines include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef __CLASS_*_H.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/proxysql_admin.h` around lines 1 - 2, The header's include guard
macro PROXYSQL_ADMIN_H doesn't follow the project's include/ header naming
policy; change the guard to match the required pattern (e.g.
__CLASS_PROXYSQL_ADMIN_H) by updating the `#ifndef`, `#define` and matching `#endif`
macro to the new name used in include_guard checking; ensure the new macro is
unique and all three places (ifndef, define, endif comment if present) reference
the same __CLASS_*_H style macro (for the header represented by PROXYSQL_ADMIN).
🧹 Nitpick comments (9)
include/gen_utils.h (1)

159-166: Update parameter name in PtrSizeArray::PtrSizeArray implementation for consistency.

The constructor declaration at include/gen_utils.h:246 uses sz as the parameter name, but the definition at lib/gen_utils.cpp:167 still uses __size. Update the implementation to match the declaration for consistency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/gen_utils.h` around lines 159 - 166, The PtrSizeArray constructor
implementation uses a different parameter name (__size) than its declaration
(sz); update the implementation of PtrSizeArray::PtrSizeArray in
lib/gen_utils.cpp to use the same parameter name (sz) as the declaration so
signatures match and avoid confusion, and ensure any uses of the old name inside
the function are renamed to sz (references to PtrSizeArray,
PtrSizeArray::PtrSizeArray).
lib/MySQL_Session.cpp (1)

3344-3352: Either drop end or render it.

These paths still trim to 253 chars and compute end = "...", but the format string no longer has a placeholder for it. Low priority, but the truncation marker is now dead code and the message no longer shows that the query was shortened.

Also applies to: 3517-3525, 5389-5397

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Session.cpp` around lines 3344 - 3352, The message-building code
computes an ellipsis in variable end when a query is truncated but never uses
it, resulting in a dead truncation marker and an incorrect buffer size; update
the Session logging in MySQL_Session.cpp (around the sprintf that builds buf
using err_msg, nqn and end) to either remove the unused end variable and its
length from the malloc calculation or include a "%s" placeholder for end in
err_msg and pass end to sprintf, and while here replace malloc+sprintf with a
bounds-safe allocation/formatting (e.g., compute required length including the
ellipsis and use snprintf) so the truncated query and the "..." marker are
correctly reflected and no buffer overflow can occur (apply same fix at the
other similar sites referenced).
lib/MySQL_Thread.cpp (1)

6120-6131: Align PostgreSQL thread metrics to use SIZE_ sentinel naming.

The MySQL_Thread code has been updated to use p_th_counter::SIZE_ and p_th_gauge::SIZE_ (compliant with UPPER_SNAKE_CASE), but lib/PgSQL_Thread.cpp still references __size at lines 5487 and 5520. Update these comparisons to match the MySQL naming convention and coding guidelines.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Thread.cpp` around lines 6120 - 6131, The PostgreSQL thread code
still compares index/sentinel against the old symbol `__size`; update those
comparisons to use the new sentinel names `p_th_counter::SIZE_` and
`p_th_gauge::SIZE_` (matching the MySQL change) wherever `__size` is referenced
(e.g., the checks at the locations that currently read like `if (m_idx !=
__size)` and similar for gauge indices); ensure you replace both counter and
gauge comparisons with the corresponding `p_th_counter::SIZE_` and
`p_th_gauge::SIZE_` symbols so the naming and behavior match the MySQL_Thread
pattern.
scripts/lint/normalize-cppcheck.py (1)

13-16: Use a context manager for file reads.

open(path).read() leaves lifecycle management implicit. Prefer with open(...) for deterministic close behavior.

Suggested patch
-if path and os.path.exists(path):
-    data = open(path).read()
+if path and os.path.exists(path):
+    with open(path, encoding="utf-8") as f:
+        data = f.read()
 else:
     data = sys.stdin.read()
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/normalize-cppcheck.py` around lines 13 - 16, Replace the
implicit open() call with a context manager so the file is deterministically
closed: in the branch that reads from path (the code using variables path and
data) use a with-open block (opening in read mode) to read into data, leaving
the else branch that reads from sys.stdin unchanged; ensure the variable names
(path, data) and behavior remain the same.
include/MySQL_Authentication.hpp (1)

1-2: Align include guard with repository guard convention.

This header guard no longer matches the repository’s include-guard pattern for files under include/.

Suggested patch
-#ifndef PROXYSQL_MYSQL_AUTHENTICATION_H
-#define PROXYSQL_MYSQL_AUTHENTICATION_H
+#ifndef __CLASS_MYSQL_AUTHENTICATION_H
+#define __CLASS_MYSQL_AUTHENTICATION_H

As per coding guidelines "include/**/*.{h,hpp}: Include guards must follow the pattern #ifndef _CLASS*_H".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_Authentication.hpp` around lines 1 - 2, The include guard macro
in MySQL_Authentication.hpp (currently PROXYSQL_MYSQL_AUTHENTICATION_H) does not
follow the repository convention; change it to the required pattern using the
__CLASS_*_H format (e.g., align with the file/class name so the macro becomes
something like __CLASS_MYSQL_AUTHENTICATION_H), updating both the `#ifndef` and
`#define` (and corresponding `#endif` comment if present) to match the new macro;
locate the guard at the top of include/MySQL_Authentication.hpp and update any
occurrences to the new __CLASS_*_H token so it follows the repository
include-guard convention.
scripts/lint/normalize-clang-tidy.py (1)

130-131: Rename ambiguous variable l for clarity.

The variable name l is easily confused with the number 1. Consider using line or diag instead.

📝 Suggested fix
-for l in sorted(diagnostics):
-    print(l)
+for line in sorted(diagnostics):
+    print(line)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/normalize-clang-tidy.py` around lines 130 - 131, The loop
variable `l` in the for-loop iterating over `sorted(diagnostics)` is ambiguous;
rename it to a clear identifier like `diag` or `line` throughout the loop (e.g.,
change `for l in sorted(diagnostics): print(l)` to `for diag in
sorted(diagnostics): print(diag)`) and update any other references in the same
scope to use the new name so readability is improved and `l` isn’t confused with
the digit one.
scripts/lint/run-local.sh (2)

39-39: Minor: Simplify redundant echo around command substitution.

The echo "$(pwd)" is equivalent to just pwd. The git command output doesn't need echo wrapping either.

📝 Suggested fix
-  REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || echo "$(pwd)")
+  REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/run-local.sh` at line 39, The REPO_ROOT assignment uses a
redundant echo wrapper; update the fallback to use pwd directly so
REPO_ROOT=$(git rev-parse --show-toplevel 2>/dev/null || pwd) (replace the
current use of echo "$(pwd)" and keep the existing command substitution and
error redirection intact).

82-84: Quote glob patterns to prevent word splitting issues.

The ${TMPD} variable should be quoted to handle paths with spaces correctly.

📝 Suggested fix
-  if ls ${TMPD:-/dev/null}/ct-*.stderr >/dev/null 2>&1; then
-    cat ${TMPD}/ct-*.stderr > lint/clang-tidy-raw.txt || true
+  if ls "${TMPD:-/dev/null}"/ct-*.stderr >/dev/null 2>&1; then
+    cat "${TMPD}"/ct-*.stderr > lint/clang-tidy-raw.txt || true
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/lint/run-local.sh` around lines 82 - 84, The shell glob uses ${TMPD}
unquoted which breaks on paths with spaces; update the checks and commands that
reference ${TMPD} and the ct-*.stderr pattern (the ls condition, the cat into
lint/clang-tidy-raw.txt, and the call to python3
scripts/lint/normalize-clang-tidy.py) to quote the variable and glob (e.g. use
"${TMPD:-/dev/null}/ct-*.stderr" and "${TMPD}/ct-*.stderr") so word-splitting is
prevented while preserving the same redirection and fallback behavior.
docs/superpowers/plans/2026-04-10-cppcheck-fixes.md (1)

174-181: Minor: Add language specifier to fenced code block.

The suppression policy code block should specify a language (e.g., markdown or text) for consistency with other documentation.

📝 Suggested fix
-```
+```text
 Lint suppressions policy
 
 - Prefer fixing issues over suppressing.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/superpowers/plans/2026-04-10-cppcheck-fixes.md` around lines 174 - 181,
The fenced code block containing the "Lint suppressions policy" text lacks a
language specifier; update that block to use a language tag (e.g., `text` or
`markdown`) so it matches other docs and renders consistently, ensuring the
opening fence becomes ```text (or ```markdown) and the closing fence remains
```; locate the block by searching for the literal lines "Lint suppressions
policy" or the nearby snippet shown in the diff and update the fence
accordingly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d394ad47-f14a-4e0f-8f02-03a69a2aa438

📥 Commits

Reviewing files that changed from the base of the PR and between 29a95af and d5ffcba.

📒 Files selected for processing (77)
  • .clang-tidy
  • .gitignore
  • Makefile
  • compile_commands.events.json
  • docs/superpowers/plans/2026-04-09-static-analysis-plan.md
  • docs/superpowers/plans/2026-04-10-cppcheck-fixes.md
  • docs/superpowers/specs/2026-04-09-linting-design.md
  • include/AI_Features_Manager.h
  • include/AI_Vector_Storage.h
  • include/Anomaly_Detector.h
  • include/Base_HostGroups_Manager.h
  • include/ClickHouse_Authentication.hpp
  • include/Command_Counter.h
  • include/GenAI_Thread.h
  • include/LLM_Bridge.h
  • include/MCP_Thread.h
  • include/MySQL_Authentication.hpp
  • include/MySQL_Data_Stream.h
  • include/MySQL_HostGroups_Manager.h
  • include/MySQL_LDAP_Authentication.hpp
  • include/MySQL_Logger.hpp
  • include/MySQL_Monitor.hpp
  • include/MySQL_PreparedStatement.h
  • include/MySQL_Protocol.h
  • include/MySQL_Query_Cache.h
  • include/MySQL_Query_Processor.h
  • include/MySQL_Session.h
  • include/MySQL_Set_Stmt_Parser.h
  • include/MySQL_Thread.h
  • include/PgSQL_Authentication.h
  • include/PgSQL_Backend.h
  • include/PgSQL_Connection.h
  • include/PgSQL_Data_Stream.h
  • include/PgSQL_Error_Helper.h
  • include/PgSQL_HostGroups_Manager.h
  • include/PgSQL_Logger.hpp
  • include/PgSQL_Protocol.h
  • include/PgSQL_Query_Cache.h
  • include/PgSQL_Query_Processor.h
  • include/PgSQL_Session.h
  • include/PgSQL_Set_Stmt_Parser.h
  • include/PgSQL_Thread.h
  • include/ProxySQL_Cluster.hpp
  • include/ProxySQL_Poll.h
  • include/Query_Cache.h
  • include/Servers_SslParams.h
  • include/StatCounters.h
  • include/gen_utils.h
  • include/log_utils.h
  • include/mysql_backend.h
  • include/mysql_connection.h
  • include/prometheus_helpers.h
  • include/proxysql_admin.h
  • include/proxysql_config.h
  • include/proxysql_coredump.h
  • include/proxysql_debug.h
  • include/proxysql_macros.h
  • include/proxysql_restapi.h
  • include/proxysql_sslkeylog.h
  • include/proxysql_structs.h
  • include/proxysql_utils.h
  • include/query_digest_topk.h
  • include/query_processor.h
  • include/sqlite3db.h
  • lib/ClickHouse_Authentication.cpp
  • lib/MySQL_Authentication.cpp
  • lib/MySQL_HostGroups_Manager.cpp
  • lib/MySQL_Protocol.cpp
  • lib/MySQL_Session.cpp
  • lib/MySQL_Thread.cpp
  • lib/PgSQL_Authentication.cpp
  • lib/ProxySQL_Admin.cpp
  • lib/debug.cpp
  • scripts/lint/generate-compile-commands.sh
  • scripts/lint/normalize-clang-tidy.py
  • scripts/lint/normalize-cppcheck.py
  • scripts/lint/run-local.sh

Comment thread lib/MySQL_Session.cpp
Comment on lines +205 to +207
username=u ? strdup(u) : nullptr;
password=p ? strdup(p) : nullptr;
hostname=h ? strdup(h) : nullptr;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Don’t make hostname nullable without hardening the worker path.

This constructor now allows hostname == nullptr, but kill_query_thread() still treats it as a mandatory C string at Lines 266, 271, 313, 319, and 321. That turns the old constructor-time failure into a later null-deref/UB path in the kill worker. Either normalize hostname here to a safe value for all call paths, or make the worker consistently guard and use get_host_address()/safe logging before shipping this.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Session.cpp` around lines 205 - 207, The constructor currently
allows hostname to be nullptr (hostname = h ? strdup(h) : nullptr;) which later
causes null-derefs in kill_query_thread(); change the constructor to normalize
hostname to a non-null safe string (e.g., strdup("") or strdup("unknown")
instead of nullptr) so all code paths can assume a valid C string, and update
any associated cleanup accordingly; alternatively, if you prefer defensive
checks, modify kill_query_thread() to guard uses of hostname (or call
get_host_address() / safe logging helpers) before dereferencing so no UB occurs.
Ensure the fix references the hostname field initialization and the
kill_query_thread() usages.

- Fix double-free bug in free_account_details() (lib/MySQL_Authentication.cpp)
- Fix trailing #endif comments in 5 files to match guard names
- Rename __SIZE -> SIZE_ in LDAP_USER_FIELD_IDX enum
- Remove dead code: Query_Processor_Output ctor now calls init()
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
lib/MySQL_Authentication.cpp (3)

814-827: ⚠️ Potential issue | 🟡 Minor

The documented column order and these indices disagree.

Lines 798-800 say the resultset includes active before use_ssl, but this lambda hardcodes active_ = true and still reads use_ssl from row[2]. Please make the mapping and the comment match; otherwise every field from use_ssl onward is one edit away from drifting silently.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Authentication.cpp` around lines 814 - 827, The lambda
create_account_details currently hardcodes acc_details->active_ = true and reads
use_ssl from row[2], which misaligns with the documented column order (active
before use_ssl); fix by reading active_ from the correct column (e.g.,
acc_details->active_ = strcmp(row[2], "1") == 0) and shift all subsequent column
reads one index higher (use_ssl from row[3], default_hostgroup via atoi(row[4]),
default_schema from row[5], schema_locked from row[6], transaction_persistent
from row[7], fast_forward from row[8], backend_ from row[9], frontend_ from
row[10]), keep the null checks (row[n] ? row[n] : "") and boolean strcmp
patterns, and/or update the surrounding comment/doc to reflect the final column
ordering consistently.

221-254: ⚠️ Potential issue | 🟠 Major

Initialize struct fields for new accounts.

The new_ad path (line 222) allocates but does not initialize num_connections_used_addl_pass, frontend_, and backend_. These are mutated later (lines 430 and 466 for num_connections_used_addl_pass), leaving them in indeterminate state on first use.

Minimal fix
 		new_ad=true;
 		ad->sha1_pass=NULL;
 		ad->num_connections_used=0;
+		ad->num_connections_used_addl_pass=0;
 		ad->clear_text_password[0] = NULL;
 		ad->clear_text_password[1] = NULL;
+		ad->frontend_=false;
+		ad->backend_=false;
 		// FIXME: if the password is a clear text password, automatically generate sha1_pass and clear_text_password
 	}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Authentication.cpp` around lines 221 - 254, The new account
allocation path (when new_ad is set) fails to initialize account_details_t
fields that are later mutated: set ad->num_connections_used_addl_pass = 0 and
initialize the connection pointers ad->frontend_ = NULL and ad->backend_ = NULL
in the same block where new_ad is created (alongside ad->num_connections_used
and ad->sha1_pass). This ensures those fields have defined values before later
use (see account_details_t and the new_ad allocation block).

337-362: ⚠️ Potential issue | 🟠 Major

Use calloc() instead of malloc() to zero-initialize account_details_t allocations.

The struct uses C++ default member initializers (= nullptr), which only apply to stack or new-allocated objects. With malloc(), the _complete == false branch (lines 337-349) leaves sha1_pass and clear_text_password[] uninitialized. While free_account_details() checks before freeing, garbage pointers will likely trigger invalid free operations. The second loop at lines 369–387 already explicitly initializes these fields, so apply the fix to both locations for consistency.

Fix
-		account_details_t *ad=(account_details_t *)malloc(sizeof(account_details_t));
+		account_details_t *ad=(account_details_t *)calloc(1, sizeof(account_details_t));
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_Authentication.cpp` around lines 337 - 362, The allocation of
account_details_t using malloc leaves members like sha1_pass and
clear_text_password[] uninitialized (causing unsafe frees); replace
malloc(sizeof(account_details_t)) with calloc(1, sizeof(account_details_t)) for
the ad allocations (both places where account_details_t *ad is created) so the
struct is zero-initialized, and keep the existing explicit assignments for other
fields; ensure this fixes fields used by free_account_details() such as
sha1_pass and clear_text_password.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@include/MySQL_LDAP_Authentication.hpp`:
- Line 5: The header is not self-sufficient: types used by enum index (uint8_t,
uint64_t), the std::unique_ptr members, and the SQLite3_result references are
undeclared; add `#include` <cstdint> and `#include` <memory> at the top and either
include the appropriate SQLite header or add a forward declaration like "struct
SQLite3_result;" so symbols referenced in MySQL_LDAP_Authentication.hpp (enum
index, uint8_t, uint64_t, std::unique_ptr<...>, and SQLite3_result) are defined
when this header is parsed standalone.

---

Outside diff comments:
In `@lib/MySQL_Authentication.cpp`:
- Around line 814-827: The lambda create_account_details currently hardcodes
acc_details->active_ = true and reads use_ssl from row[2], which misaligns with
the documented column order (active before use_ssl); fix by reading active_ from
the correct column (e.g., acc_details->active_ = strcmp(row[2], "1") == 0) and
shift all subsequent column reads one index higher (use_ssl from row[3],
default_hostgroup via atoi(row[4]), default_schema from row[5], schema_locked
from row[6], transaction_persistent from row[7], fast_forward from row[8],
backend_ from row[9], frontend_ from row[10]), keep the null checks (row[n] ?
row[n] : "") and boolean strcmp patterns, and/or update the surrounding
comment/doc to reflect the final column ordering consistently.
- Around line 221-254: The new account allocation path (when new_ad is set)
fails to initialize account_details_t fields that are later mutated: set
ad->num_connections_used_addl_pass = 0 and initialize the connection pointers
ad->frontend_ = NULL and ad->backend_ = NULL in the same block where new_ad is
created (alongside ad->num_connections_used and ad->sha1_pass). This ensures
those fields have defined values before later use (see account_details_t and the
new_ad allocation block).
- Around line 337-362: The allocation of account_details_t using malloc leaves
members like sha1_pass and clear_text_password[] uninitialized (causing unsafe
frees); replace malloc(sizeof(account_details_t)) with calloc(1,
sizeof(account_details_t)) for the ad allocations (both places where
account_details_t *ad is created) so the struct is zero-initialized, and keep
the existing explicit assignments for other fields; ensure this fixes fields
used by free_account_details() such as sha1_pass and clear_text_password.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4545da87-b745-4258-8452-40d880cc9a19

📥 Commits

Reviewing files that changed from the base of the PR and between d5ffcba and a82224f.

📒 Files selected for processing (7)
  • include/LLM_Bridge.h
  • include/MySQL_LDAP_Authentication.hpp
  • include/proxysql_coredump.h
  • include/proxysql_restapi.h
  • include/proxysql_sslkeylog.h
  • include/query_processor.h
  • lib/MySQL_Authentication.cpp
✅ Files skipped from review due to trivial changes (4)
  • include/proxysql_restapi.h
  • include/proxysql_coredump.h
  • include/proxysql_sslkeylog.h
  • include/LLM_Bridge.h
🚧 Files skipped from review as they are similar to previous changes (1)
  • include/query_processor.h
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: CI-builds / builds (debian12,-dbg)
  • GitHub Check: CI-builds / builds (ubuntu22,-tap)
  • GitHub Check: CI-builds / builds (ubuntu24,-tap-genai-gcov)
  • GitHub Check: run / trigger
  • GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{cpp,h,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{cpp,h,hpp}: Class names must use PascalCase with protocol prefixes (MySQL_, PgSQL_, ProxySQL_)
Member variables must use snake_case
Constants and macros must use UPPER_SNAKE_CASE
C++17 is required; use conditional compilation via #ifdef PROXYSQLGENAI, #ifdef PROXYSQL31, etc. for feature flags
Use pthread mutexes for synchronization and std::atomic<> for counters

Files:

  • lib/MySQL_Authentication.cpp
  • include/MySQL_LDAP_Authentication.hpp
**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

Use RAII for resource management and jemalloc for memory allocation

Files:

  • lib/MySQL_Authentication.cpp
lib/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

One class per file is the typical convention

Files:

  • lib/MySQL_Authentication.cpp
include/**/*.{h,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

Include guards must follow the pattern #ifndef _CLASS*_H

Files:

  • include/MySQL_LDAP_Authentication.hpp
🧠 Learnings (3)
📓 Common learnings
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:27.165Z
Learning: In ProxySQL test files (test/tap/tests/), resource leaks (such as not calling `mysql_close()` on early return paths) are not typically fixed because test processes are short-lived and the OS frees resources on process exit. This is a common pattern across the test suite.
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T14:38:16.093Z
Learning: Applies to **/*.{cpp,h,hpp} : C++17 is required; use conditional compilation via `#ifdef` PROXYSQLGENAI, `#ifdef` PROXYSQL31, etc. for feature flags
📚 Learning: 2026-03-22T14:38:16.093Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T14:38:16.093Z
Learning: Applies to **/*.{cpp,h,hpp} : C++17 is required; use conditional compilation via `#ifdef` PROXYSQLGENAI, `#ifdef` PROXYSQL31, etc. for feature flags

Applied to files:

  • include/MySQL_LDAP_Authentication.hpp
📚 Learning: 2026-03-22T14:38:16.093Z
Learnt from: CR
Repo: sysown/proxysql PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-22T14:38:16.093Z
Learning: Admin interface uses SQLite3 backend for SQL-based configuration and schema versions are tracked in ProxySQL_Admin_Tables_Definitions.h

Applied to files:

  • include/MySQL_LDAP_Authentication.hpp
🪛 Clang (14.0.6)
include/MySQL_LDAP_Authentication.hpp

[error] 5-5: unknown type name 'uint8_t'

(clang-diagnostic-error)


[error] 24-24: use of undeclared identifier 'std'

(clang-diagnostic-error)


[error] 24-24: use of undeclared identifier 'SQLite3_result'

(clang-diagnostic-error)


[error] 34-34: unknown type name 'SQLite3_result'

(clang-diagnostic-error)


[error] 35-35: unknown type name 'SQLite3_result'

(clang-diagnostic-error)


[error] 36-36: unknown type name 'SQLite3_result'

(clang-diagnostic-error)


[error] 37-37: unknown type name 'uint64_t'

(clang-diagnostic-error)


[error] 38-38: unknown type name 'SQLite3_result'

(clang-diagnostic-error)

🪛 Cppcheck (2.20.0)
lib/MySQL_Authentication.cpp

[warning] 254-254: If memory allocation fails, then there is a possible null pointer dereference

(nullPointerOutOfMemory)


[warning] 361-361: If memory allocation fails, then there is a possible null pointer dereference

(nullPointerOutOfMemory)


[warning] 362-362: If memory allocation fails, then there is a possible null pointer dereference

(nullPointerOutOfMemory)


[warning] 386-386: If memory allocation fails, then there is a possible null pointer dereference

(nullPointerOutOfMemory)


[warning] 387-387: If memory allocation fails, then there is a possible null pointer dereference

(nullPointerOutOfMemory)

🔇 Additional comments (2)
include/MySQL_LDAP_Authentication.hpp (1)

20-22: nullptr migration is clean and consistent.

These replacements improve type safety and readability; no concerns on this segment.

Also applies to: 29-31, 35-36, 38-38

lib/MySQL_Authentication.cpp (1)

78-118: Consistent rename to active_.

This rename lines up with include/MySQL_Authentication.hpp and the sibling auth backends, so the inactive-mark/sweep flow stays coherent.


struct LDAP_USER_FIELD_IDX {
enum index {
enum index : uint8_t {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n include/MySQL_LDAP_Authentication.hpp

Repository: sysown/proxysql

Length of output: 2097


Add required includes and forward declarations for header self-sufficiency.

This header uses uint8_t, uint64_t, std::unique_ptr<>, and SQLite3_result without the necessary includes or declarations:

  • uint8_t and uint64_t (line 5, 37) require #include <cstdint>
  • std::unique_ptr<> (line 24) requires #include <memory>
  • SQLite3_result (lines 24, 34, 35, 36, 38) requires a forward declaration or include

Without these, the header will fail to parse standalone and break compilation.

Proposed fix
 `#ifndef` CLASS_MYSQL_LDAP_AUTHENTICATION_H
 `#define` CLASS_MYSQL_LDAP_AUTHENTICATION_H
 
+#include <cstdint>
+#include <memory>
+
+class SQLite3_result;
+
 struct LDAP_USER_FIELD_IDX {
 	enum index : uint8_t {
 		USERNAME = 0,
🧰 Tools
🪛 Clang (14.0.6)

[error] 5-5: unknown type name 'uint8_t'

(clang-diagnostic-error)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@include/MySQL_LDAP_Authentication.hpp` at line 5, The header is not
self-sufficient: types used by enum index (uint8_t, uint64_t), the
std::unique_ptr members, and the SQLite3_result references are undeclared; add
`#include` <cstdint> and `#include` <memory> at the top and either include the
appropriate SQLite header or add a forward declaration like "struct
SQLite3_result;" so symbols referenced in MySQL_LDAP_Authentication.hpp (enum
index, uint8_t, uint64_t, std::unique_ptr<...>, and SQLite3_result) are defined
when this header is parsed standalone.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/MySQL_HostGroups_Manager.cpp`:
- Line 1880: The debug fprintf prints the latency label "max_latency_ms" but
uses mysrvc->max_latency_us * 1000 (microseconds multiplied), yielding values
1000x too large; update the print expression for mysrvc->max_latency_us in the
fprintf call (the line that references mysrvc->myhgc->hid, mysrvc->address, ...
mysrvc->max_latency_us*1000, mysrvc->comment) to convert microseconds to
milliseconds by dividing by 1000 instead of multiplying so the printed
"max_latency_ms" value matches the rest of the file.
- Around line 3581-3583: The code silently skips when host_server_mapping is
null; before the continue add a warning log using the same logging facility used
elsewhere in this file (e.g., MXB_WARNING / MXS_WARNING or the local logger)
that reports the unexpected null mapping and any available context (host name,
host id, server identifier or the variables in scope) so topology drift is
visible; keep the continue but ensure the log message clearly indicates the
function/loop (mention host_server_mapping) and relevant identifiers to aid
diagnosis.
- Around line 4997-4999: Change the declaration of q from a non-const pointer to
a const char* (e.g., const char* q = "DELETE ...") so the string literal is
treated as const in this C++17 code path; update any uses (snprintf call using
q, qsz, query, _hostname, _port, info->offline_hostgroup,
info->backup_writer_hostgroup, info->writer_hostgroup) accordingly to avoid
discarding qualifiers.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 152094ce-a319-4d99-aa6e-e8978820986f

📥 Commits

Reviewing files that changed from the base of the PR and between a82224f and 24e56d4.

📒 Files selected for processing (2)
  • include/gen_utils.h
  • lib/MySQL_HostGroups_Manager.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
  • include/gen_utils.h
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: CI-builds / builds (ubuntu22,-tap)
  • GitHub Check: CI-builds / builds (debian12,-dbg)
  • GitHub Check: CI-builds / builds (ubuntu24,-tap-genai-gcov)
  • GitHub Check: run / trigger
  • GitHub Check: claude-review
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{cpp,h,hpp}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{cpp,h,hpp}: Class names must use PascalCase with protocol prefixes (MySQL_, PgSQL_, ProxySQL_)
Member variables must use snake_case
Constants and macros must use UPPER_SNAKE_CASE
C++17 is required; use conditional compilation via #ifdef PROXYSQLGENAI, #ifdef PROXYSQL31, etc. for feature flags
Use pthread mutexes for synchronization and std::atomic<> for counters

Files:

  • lib/MySQL_HostGroups_Manager.cpp
**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

Use RAII for resource management and jemalloc for memory allocation

Files:

  • lib/MySQL_HostGroups_Manager.cpp
lib/**/*.cpp

📄 CodeRabbit inference engine (CLAUDE.md)

One class per file is the typical convention

Files:

  • lib/MySQL_HostGroups_Manager.cpp
🧠 Learnings (1)
📚 Learning: 2026-01-20T09:34:27.165Z
Learnt from: yuji-hatakeyama
Repo: sysown/proxysql PR: 5307
File: test/tap/tests/reg_test_5306-show_warnings_with_comment-t.cpp:39-48
Timestamp: 2026-01-20T09:34:27.165Z
Learning: In ProxySQL test files (test/tap/tests/), resource leaks (such as not calling `mysql_close()` on early return paths) are not typically fixed because test processes are short-lived and the OS frees resources on process exit. This is a common pattern across the test suite.

Applied to files:

  • lib/MySQL_HostGroups_Manager.cpp
🔇 Additional comments (1)
lib/MySQL_HostGroups_Manager.cpp (1)

1054-1054: Good fix on the post-insert lookup.

Fetching the pointer after insertion avoids keeping a raw pointer from a moved unique_ptr, which was the real hazard in this path.

break;
}
fprintf(stderr,"HID: %d , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %u , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us*1000, mysrvc->comment);
fprintf(stderr,"HID: %u , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %d , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us*1000, mysrvc->comment);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix the latency unit in this debug dump.

The label says max_latency_ms, but this prints mysrvc->max_latency_us * 1000. Everywhere else in the file converts max_latency_us to milliseconds with / 1000, so this log will report values far too large and make troubleshooting misleading.

Suggested fix
- fprintf(stderr,"HID: %u , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %d , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us*1000, mysrvc->comment);
+ fprintf(stderr,"HID: %u , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %d , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us/1000, mysrvc->comment);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
fprintf(stderr,"HID: %u , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %d , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us*1000, mysrvc->comment);
fprintf(stderr,"HID: %u , address: %s , port: %d , gtid_port: %d , weight: %ld , status: %s , max_connections: %ld , max_replication_lag: %u , use_ssl: %d , max_latency_ms: %u , comment: %s\n", mysrvc->myhgc->hid, mysrvc->address, mysrvc->port, mysrvc->gtid_port, mysrvc->weight, st, mysrvc->max_connections, mysrvc->max_replication_lag, mysrvc->use_ssl, mysrvc->max_latency_us/1000, mysrvc->comment);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_HostGroups_Manager.cpp` at line 1880, The debug fprintf prints the
latency label "max_latency_ms" but uses mysrvc->max_latency_us * 1000
(microseconds multiplied), yielding values 1000x too large; update the print
expression for mysrvc->max_latency_us in the fprintf call (the line that
references mysrvc->myhgc->hid, mysrvc->address, ... mysrvc->max_latency_us*1000,
mysrvc->comment) to convert microseconds to milliseconds by dividing by 1000
instead of multiplying so the printed "max_latency_ms" value matches the rest of
the file.

Comment on lines +3581 to +3583
if (!host_server_mapping) {
continue;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Don’t silently swallow a null mapping here.

If host_server_mapping is unexpectedly null, skipping the server with no log leaves topology drift invisible. Please emit at least a warning before continue so this state is diagnosable.

Suggested fix
 if (!host_server_mapping) {
+	proxy_warning("Server mapping unexpectedly null for %s:%d\n", hostname.c_str(), port);
 	continue;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!host_server_mapping) {
continue;
}
if (!host_server_mapping) {
proxy_warning("Server mapping unexpectedly null for %s:%d\n", hostname.c_str(), port);
continue;
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_HostGroups_Manager.cpp` around lines 3581 - 3583, The code silently
skips when host_server_mapping is null; before the continue add a warning log
using the same logging facility used elsewhere in this file (e.g., MXB_WARNING /
MXS_WARNING or the local logger) that reports the unexpected null mapping and
any available context (host name, host id, server identifier or the variables in
scope) so topology drift is visible; keep the continue but ensure the log
message clearly indicates the function/loop (mention host_server_mapping) and
relevant identifiers to aid diagnosis.

Comment on lines +4997 to +4999
q="DELETE FROM mysql_servers_incoming WHERE hostname='%s' AND port=%d AND hostgroup_id in (%d, %d, %d)";
//query=(char *)malloc(strlen(q)+strlen(_hostname)+64);
sprintf(query,q,_hostname,_port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup, info->writer_hostgroup);
snprintf(query, qsz, q, _hostname, _port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

In C++17, can a string literal be assigned to char*, or must it be stored in const char*?

💡 Result:

In C++17, an ordinary string literal like "hello" has type const char[N] (an array of const char) and decays to const char* in most expressions, so it must be stored in const char* (or std::string, etc.). (en.cppreference.com)

Assigning it to char* (e.g., char* p = "hello";) is ill-formed in standard C++ (many compilers may still accept it as a non-standard extension with a deprecation warning). (en.cppreference.com)

Citations:


🏁 Script executed:

# First, check the actual code at lines 4997-4999
sed -n '4995,5005p' lib/MySQL_HostGroups_Manager.cpp | cat -n

Repository: sysown/proxysql

Length of output: 769


🏁 Script executed:

# Search for the declaration of q in the function context around line 4997
sed -n '4950,5010p' lib/MySQL_HostGroups_Manager.cpp | head -80

Repository: sysown/proxysql

Length of output: 2899


Keep the SQL template const in this C++17 path.

Line 4997 assigns a string literal to char* q. In C++17, string literals have type const char*, so this reintroduces a non-const-literal conversion. Change the assignment to use const char*:

- q="DELETE FROM mysql_servers_incoming WHERE hostname='%s' AND port=%d AND hostgroup_id in (%d, %d, %d)";
+ const char* q = "DELETE FROM mysql_servers_incoming WHERE hostname='%s' AND port=%d AND hostgroup_id in (%d, %d, %d)";
  snprintf(query, qsz, q, _hostname, _port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
q="DELETE FROM mysql_servers_incoming WHERE hostname='%s' AND port=%d AND hostgroup_id in (%d, %d, %d)";
//query=(char *)malloc(strlen(q)+strlen(_hostname)+64);
sprintf(query,q,_hostname,_port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup, info->writer_hostgroup);
snprintf(query, qsz, q, _hostname, _port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup);
const char* q = "DELETE FROM mysql_servers_incoming WHERE hostname='%s' AND port=%d AND hostgroup_id in (%d, %d, %d)";
//query=(char *)malloc(strlen(q)+strlen(_hostname)+64);
snprintf(query, qsz, q, _hostname, _port, info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/MySQL_HostGroups_Manager.cpp` around lines 4997 - 4999, Change the
declaration of q from a non-const pointer to a const char* (e.g., const char* q
= "DELETE ...") so the string literal is treated as const in this C++17 code
path; update any uses (snprintf call using q, qsz, query, _hostname, _port,
info->offline_hostgroup, info->backup_writer_hostgroup, info->writer_hostgroup)
accordingly to avoid discarding qualifiers.

@renecannao renecannao merged commit 669ab91 into v3.0 Apr 10, 2026
3 checks passed
@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarQube Cloud

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.

1 participant