From a3d7378ad0bc2a64e060168992b855fc5d43c720 Mon Sep 17 00:00:00 2001 From: Rene Cannao Date: Sat, 11 Apr 2026 13:29:22 +0000 Subject: [PATCH] fix(test/tap): misleading-indentation bug fix + function-address bug + leak cleanup test_ssl_fast_forward-1-t.cpp: - Fix a bug I introduced in the prior batch: my scripted wrapping of braceless 'if (cond) return exit_status();' bodies skipped the one 'if' line that had a trailing '// comment' (it didn't end with ')'). The result was a misleading-indentation case where only close_all() was inside the if body and return exit_status() ran unconditionally. Added explicit { } around that block. test_digest_umap_aux-t.cpp: - Fix real logic bug: 'tests_failed == 0' (missing parentheses) compared the *address* of the function against zero. The address is never NULL, so the second operand of the '&&' was always false, and the TRUNCATE TABLE stats.stats_mysql_query_digest cleanup at end of main never actually ran. Fixed to 'tests_failed() == 0'. Surfaced by -Waddress. test_default_conn_collation-t.cpp: - Fix NULL-deref UB: mysql_init was not checked for NULL before being passed to mysql_real_connect. - Fix 'admin' leak on mysql_real_connect failure. - Remove the dead 'cleanup:' label: the label's cleanup code (mysql_close + return exit_status) was unreachable because the preceding happy-path flow was a plain fall-through into it. Moved the cleanup body inline and deleted the label. Compile-verified with -fsyntax-only (-Wall for the first two files). --- test/tap/tests/test_default_conn_collation-t.cpp | 13 ++++++++----- test/tap/tests/test_digest_umap_aux-t.cpp | 5 ++++- test/tap/tests/test_ssl_fast_forward-1-t.cpp | 3 ++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/test/tap/tests/test_default_conn_collation-t.cpp b/test/tap/tests/test_default_conn_collation-t.cpp index a130ac130..91227787d 100644 --- a/test/tap/tests/test_default_conn_collation-t.cpp +++ b/test/tap/tests/test_default_conn_collation-t.cpp @@ -86,25 +86,28 @@ int check_all_collations(const CommandLine& cl, MYSQL* admin) { int main(int argc, char** argv) { CommandLine cl; + MYSQL* admin = NULL; if (cl.getEnv()) { diag("Failed to get the required environmental variables."); return EXIT_FAILURE; } - - MYSQL* admin = mysql_init(NULL); + admin = mysql_init(NULL); + if (!admin) { + fprintf(stderr, "File %s, line %d, Error: mysql_init failed for admin\n", + __FILE__, __LINE__); + return EXIT_FAILURE; + } if (!mysql_real_connect(admin, cl.host, cl.admin_username, cl.admin_password, NULL, cl.admin_port, NULL, 0)) { fprintf(stderr, "File %s, line %d, Error: %s\n", __FILE__, __LINE__, mysql_error(admin)); + mysql_close(admin); return EXIT_FAILURE; } check_all_collations(cl, admin); -cleanup: - mysql_close(admin); - return exit_status(); } diff --git a/test/tap/tests/test_digest_umap_aux-t.cpp b/test/tap/tests/test_digest_umap_aux-t.cpp index 22391e05b..cfd47d789 100644 --- a/test/tap/tests/test_digest_umap_aux-t.cpp +++ b/test/tap/tests/test_digest_umap_aux-t.cpp @@ -315,7 +315,10 @@ int main(int argc, char** argv) { ); } - if (tests_last() == nplan && tests_failed == 0) { + // Missing parentheses on tests_failed turned this into a comparison + // of the function's address against 0. The address is never NULL so + // the second operand was always false and the TRUNCATE was never run. + if (tests_last() == nplan && tests_failed() == 0) { string q = "TRUNCATE TABLE stats.stats_mysql_query_digest"; diag("Running %s", q.c_str()); MYSQL_QUERY(proxy_admin, q.c_str()); diff --git a/test/tap/tests/test_ssl_fast_forward-1-t.cpp b/test/tap/tests/test_ssl_fast_forward-1-t.cpp index 3fc501cef..3cff51e82 100644 --- a/test/tap/tests/test_ssl_fast_forward-1-t.cpp +++ b/test/tap/tests/test_ssl_fast_forward-1-t.cpp @@ -244,9 +244,10 @@ int main(int argc, char** argv) { diag("We now create a connection using SSL for both client or backend"); - if (run_queries_sets(queries_set4, mysqladmin, "Running on Admin")) // note: we use queries_set4 again + if (run_queries_sets(queries_set4, mysqladmin, "Running on Admin")) { // note: we use queries_set4 again close_all(); return exit_status(); + } mysqls[4] = mysql_init(NULL); if (!mysqls[4]) { close_all();