From 2f057d3530535d0e21ed6217f257ebb83a8af456 Mon Sep 17 00:00:00 2001 From: Dimitri Fontaine Date: Fri, 10 Jul 2026 22:43:14 +0200 Subject: [PATCH] fix: resolve compiler warnings on macOS and Linux Clang on macOS (Apple Silicon, Xcode 15+) emits -Wdangling-assignment for patterns like: paramValues[N] = intToString(expr).strValue; intToString() returns an IntString struct by value; its embedded strValue[] char array is in a stack temporary that is destroyed at end of expression, leaving paramValues[N] dangling before the next pgsql_execute_with_params() call. Fix: declare a named IntString variable so its lifetime covers the subsequent call. The compiler warning fires in C99/C11 mode on clang 15+ and in practice the pointer is stable until the stack frame is reused, but the pattern is formally undefined and must be corrected. Additional fixes: - primary_standby.c: remove unused loop counter in crash-recovery wait loop (variable 'attempts' set but not used). - cli_formation.c: add missing errors > 0 check in keeper_cli_formation_create_getopts() matching the pattern in keeper_cli_formation_drop_getopts(); the variable was correctly incremented in the default: case but the exit guard was missing (variable 'errors' set but not used, and a latent logic gap). --- src/bin/pg_autoctl/cli_formation.c | 6 ++++ src/bin/pg_autoctl/coordinator.c | 43 +++++++++++++++++++--------- src/bin/pg_autoctl/demoapp.c | 33 ++++++++++++++------- src/bin/pg_autoctl/primary_standby.c | 2 +- 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/src/bin/pg_autoctl/cli_formation.c b/src/bin/pg_autoctl/cli_formation.c index 09d546dd1..b06af3bff 100644 --- a/src/bin/pg_autoctl/cli_formation.c +++ b/src/bin/pg_autoctl/cli_formation.c @@ -413,6 +413,12 @@ keeper_cli_formation_create_getopts(int argc, char **argv) } } + if (errors > 0) + { + commandline_help(stderr); + exit(EXIT_CODE_BAD_ARGS); + } + /* when we have a monitor URI we don't need PGDATA */ if (cli_formation_use_monitor_option(&options)) { diff --git a/src/bin/pg_autoctl/coordinator.c b/src/bin/pg_autoctl/coordinator.c index 4845aa96c..1916d6536 100644 --- a/src/bin/pg_autoctl/coordinator.c +++ b/src/bin/pg_autoctl/coordinator.c @@ -160,9 +160,12 @@ coordinator_add_node(Coordinator *coordinator, Keeper *keeper, SingleValueResultContext parseContext = { { 0 }, PGSQL_RESULT_INT, false }; + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); + IntString groupIdStr = intToString(keeper->config.groupId); + paramValues[0] = keeper->config.hostname; - paramValues[1] = intToString(keeper->config.pgSetup.pgport).strValue; - paramValues[2] = intToString(keeper->config.groupId).strValue; + paramValues[1] = pgPortStr.strValue; + paramValues[2] = groupIdStr.strValue; paramValues[3] = citusRoleStr; paramValues[4] = clusterName; @@ -230,6 +233,8 @@ coordinator_add_inactive_node(Coordinator *coordinator, Keeper *keeper, : keeper->config.pgSetup.citusClusterName; SingleValueResultContext parseContext = { { 0 }, PGSQL_RESULT_INT, false }; + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); + IntString groupIdStr = intToString(keeper->config.groupId); if (!coordinator_master_activate_node_returns_record(pgsql, &returnsRecord)) { @@ -248,8 +253,8 @@ coordinator_add_inactive_node(Coordinator *coordinator, Keeper *keeper, } paramValues[0] = keeper->config.hostname; - paramValues[1] = intToString(keeper->config.pgSetup.pgport).strValue; - paramValues[2] = intToString(keeper->config.groupId).strValue; + paramValues[1] = pgPortStr.strValue; + paramValues[2] = groupIdStr.strValue; paramValues[3] = citusRoleStr; paramValues[4] = clusterName; @@ -323,8 +328,10 @@ coordinator_activate_node(Coordinator *coordinator, Keeper *keeper, sql = sqlInteger; } + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); + paramValues[0] = keeper->config.hostname; - paramValues[1] = intToString(keeper->config.pgSetup.pgport).strValue; + paramValues[1] = pgPortStr.strValue; if (!pgsql_execute_with_params(pgsql, sql, paramCount, paramTypes, paramValues, @@ -372,8 +379,10 @@ coordinator_remove_node(Coordinator *coordinator, Keeper *keeper) Oid paramTypes[2] = { TEXTOID, INT4OID }; const char *paramValues[2]; + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); + paramValues[0] = keeper->config.hostname; - paramValues[1] = intToString(keeper->config.pgSetup.pgport).strValue; + paramValues[1] = pgPortStr.strValue; if (!pgsql_execute_with_params(pgsql, sql, paramCount, paramTypes, paramValues, @@ -621,12 +630,16 @@ coordinator_update_node_prepare(Coordinator *coordinator, Keeper *keeper) " and not exists" " (select 1 from pg_prepared_xacts where gid = $4)"); - paramValues[0] = intToString(groupId).strValue; + IntString groupIdStr = intToString(groupId); + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); + IntString lockCooldownStr = intToString( + keeper->config.citus_master_update_node_lock_cooldown); + + paramValues[0] = groupIdStr.strValue; paramValues[1] = keeper->config.hostname; - paramValues[2] = intToString(keeper->config.pgSetup.pgport).strValue; + paramValues[2] = pgPortStr.strValue; paramValues[3] = transactionName; - paramValues[4] = intToString( - keeper->config.citus_master_update_node_lock_cooldown).strValue; + paramValues[4] = lockCooldownStr.strValue; if (!pgsql_execute_with_params(pgsql, sql, paramCount, paramTypes, paramValues, @@ -642,6 +655,8 @@ coordinator_update_node_prepare(Coordinator *coordinator, Keeper *keeper) const int paramCount = 4; Oid paramTypes[4] = { INT4OID, TEXTOID, INT4OID, TEXTOID }; const char *paramValues[4]; + IntString groupIdStr = intToString(groupId); + IntString pgPortStr = intToString(keeper->config.pgSetup.pgport); sformat(sql, sizeof(sql), @@ -652,9 +667,9 @@ coordinator_update_node_prepare(Coordinator *coordinator, Keeper *keeper) " and not exists" " (select 1 from pg_prepared_xacts where gid = $4)"); - paramValues[0] = intToString(groupId).strValue; + paramValues[0] = groupIdStr.strValue; paramValues[1] = keeper->config.hostname; - paramValues[2] = intToString(keeper->config.pgSetup.pgport).strValue; + paramValues[2] = pgPortStr.strValue; paramValues[3] = transactionName; if (!pgsql_execute_with_params(pgsql, sql, @@ -798,6 +813,7 @@ coordinator_upsert_poolinfo_port(Coordinator *coordinator, Keeper *keeper) Oid paramTypes[2] = { INT4OID, TEXTOID }; const char *paramValues[2]; char proxyInfo[MAXCONNINFO]; + IntString groupIdStr; /* * Prepare a argument for pg_dist_poolinfo table @@ -805,7 +821,8 @@ coordinator_upsert_poolinfo_port(Coordinator *coordinator, Keeper *keeper) sformat(proxyInfo, sizeof(proxyInfo), "host=%s port=%d", keeper->config.hostname, keeper->config.pgSetup.proxyport); - paramValues[0] = intToString(keeper->config.groupId).strValue; + groupIdStr = intToString(keeper->config.groupId); + paramValues[0] = groupIdStr.strValue; paramValues[1] = proxyInfo; if (!pgsql_execute_with_params(pgsql, sql, diff --git a/src/bin/pg_autoctl/demoapp.c b/src/bin/pg_autoctl/demoapp.c index 609c75c97..b94c1b728 100644 --- a/src/bin/pg_autoctl/demoapp.c +++ b/src/bin/pg_autoctl/demoapp.c @@ -482,10 +482,15 @@ demoapp_register_client(const char *pguri, const Oid paramTypes[4] = { INT4OID, INT4OID, INT4OID, INT4OID }; const char *paramValues[4] = { 0 }; - paramValues[0] = intToString(clientId).strValue; - paramValues[1] = intToString(getpid()).strValue; - paramValues[2] = intToString(retrySleep).strValue; - paramValues[3] = intToString(retryCap).strValue; + IntString clientIdStr = intToString(clientId); + IntString pidStr = intToString(getpid()); + IntString retrySleepStr = intToString(retrySleep); + IntString retryCapStr = intToString(retryCap); + + paramValues[0] = clientIdStr.strValue; + paramValues[1] = pidStr.strValue; + paramValues[2] = retrySleepStr.strValue; + paramValues[3] = retryCapStr.strValue; pgsql_init(&pgsql, (char *) pguri, PGSQL_CONN_APP); @@ -519,8 +524,11 @@ demoapp_update_client_failovers(const char *pguri, int clientId, int failovers) const Oid paramTypes[2] = { INT4OID, INT4OID }; const char *paramValues[2] = { 0 }; - paramValues[0] = intToString(clientId).strValue; - paramValues[1] = intToString(failovers).strValue; + IntString clientIdStr = intToString(clientId); + IntString failoversStr = intToString(failovers); + + paramValues[0] = clientIdStr.strValue; + paramValues[1] = failoversStr.strValue; pgsql_init(&pgsql, (char *) pguri, PGSQL_CONN_APP); @@ -699,10 +707,15 @@ demoapp_start_client(const char *pguri, int clientId, const Oid paramTypes[5] = { INT4OID, INT4OID, INT8OID, INT8OID, BOOLOID }; const char *paramValues[5] = { 0 }; - paramValues[0] = intToString(clientId).strValue; - paramValues[1] = intToString(index).strValue; - paramValues[2] = intToString(pgsql.retryPolicy.attempts).strValue; - paramValues[3] = intToString(INSTR_TIME_GET_MICROSEC(duration)).strValue; + IntString clientIdStr = intToString(clientId); + IntString indexStr = intToString(index); + IntString attemptsStr = intToString(pgsql.retryPolicy.attempts); + IntString durationStr = intToString(INSTR_TIME_GET_MICROSEC(duration)); + + paramValues[0] = clientIdStr.strValue; + paramValues[1] = indexStr.strValue; + paramValues[2] = attemptsStr.strValue; + paramValues[3] = durationStr.strValue; paramValues[4] = is_in_recovery ? "true" : "false"; if (!pgsql_execute_with_params(&pgsql, sql, 5, paramTypes, paramValues, diff --git a/src/bin/pg_autoctl/primary_standby.c b/src/bin/pg_autoctl/primary_standby.c index 96a171978..6bc3b01a3 100644 --- a/src/bin/pg_autoctl/primary_standby.c +++ b/src/bin/pg_autoctl/primary_standby.c @@ -1207,7 +1207,7 @@ postgres_maybe_do_crash_recovery(LocalPostgresServer *postgres) default: { /* wait until postgres crash recovery is done */ - for (int attempts = 0;; attempts++) + for (;;) { int timeout = 30;