Skip to content

fix: hoist intToString() temporaries to avoid dangling strValue pointers #1133

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dimitri merged 1 commit into from
Jul 9, 2026
Merged

fix: hoist intToString() temporaries to avoid dangling strValue pointers #1133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions src/bin/pg_autoctl/coordinator.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,12 @@ coordinator_add_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);

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;

Expand Down Expand Up @@ -230,6 +232,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))
{
Expand All @@ -248,8 +252,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;

Expand Down Expand Up @@ -307,6 +311,8 @@ coordinator_activate_node(Coordinator *coordinator, Keeper *keeper,
parseContext.resultType = PGSQL_RESULT_INT;
parseContext.parsedOk = false;

IntString pgportStr = intToString(keeper->config.pgSetup.pgport);

if (!coordinator_master_activate_node_returns_record(pgsql, &returnsRecord))
{
log_error("Failed to activate node %s:%d, see above for details",
Expand All @@ -324,7 +330,7 @@ coordinator_activate_node(Coordinator *coordinator, Keeper *keeper,
}

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,
Expand Down Expand Up @@ -371,9 +377,10 @@ coordinator_remove_node(Coordinator *coordinator, Keeper *keeper)
int paramCount = 2;
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,
Expand Down Expand Up @@ -605,11 +612,16 @@ coordinator_update_node_prepare(Coordinator *coordinator, Keeper *keeper)
* private data handled by the coordinator, and the coordinator is going to
* provide for that information itself with the following SQL query.
*/
IntString groupIdStr = intToString(groupId);
IntString pgportStr = intToString(keeper->config.pgSetup.pgport);

if (supportForForce)
{
const int paramCount = 5;
Oid paramTypes[5] = { INT4OID, TEXTOID, INT4OID, TEXTOID, INT4OID };
const char *paramValues[5];
IntString lockCooldownStr = intToString(
keeper->config.citus_master_update_node_lock_cooldown);

sformat(sql,
sizeof(sql),
Expand All @@ -621,12 +633,11 @@ 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;
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,
Expand All @@ -652,9 +663,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,
Expand Down Expand Up @@ -805,7 +816,9 @@ 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;
IntString groupIdStr = intToString(keeper->config.groupId);

paramValues[0] = groupIdStr.strValue;
paramValues[1] = proxyInfo;

if (!pgsql_execute_with_params(pgsql, sql,
Expand Down
32 changes: 21 additions & 11 deletions src/bin/pg_autoctl/demoapp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,11 +481,15 @@ demoapp_register_client(const char *pguri,

const Oid paramTypes[4] = { INT4OID, INT4OID, INT4OID, INT4OID };
const char *paramValues[4] = { 0 };
IntString clientIdStr = intToString(clientId);
IntString pidStr = intToString(getpid());
IntString retrySleepStr = intToString(retrySleep);
IntString retryCapStr = intToString(retryCap);

paramValues[0] = intToString(clientId).strValue;
paramValues[1] = intToString(getpid()).strValue;
paramValues[2] = intToString(retrySleep).strValue;
paramValues[3] = intToString(retryCap).strValue;
paramValues[0] = clientIdStr.strValue;
paramValues[1] = pidStr.strValue;
paramValues[2] = retrySleepStr.strValue;
paramValues[3] = retryCapStr.strValue;

pgsql_init(&pgsql, (char *) pguri, PGSQL_CONN_APP);

Expand Down Expand Up @@ -518,9 +522,11 @@ demoapp_update_client_failovers(const char *pguri, int clientId, int failovers)

const Oid paramTypes[2] = { INT4OID, INT4OID };
const char *paramValues[2] = { 0 };
IntString clientIdStr = intToString(clientId);
IntString failoversStr = intToString(failovers);

paramValues[0] = intToString(clientId).strValue;
paramValues[1] = intToString(failovers).strValue;
paramValues[0] = clientIdStr.strValue;
paramValues[1] = failoversStr.strValue;

pgsql_init(&pgsql, (char *) pguri, PGSQL_CONN_APP);

Expand Down Expand Up @@ -698,11 +704,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 durationUsStr = intToString(INSTR_TIME_GET_MICROSEC(duration));

paramValues[0] = clientIdStr.strValue;
paramValues[1] = indexStr.strValue;
paramValues[2] = attemptsStr.strValue;
paramValues[3] = durationUsStr.strValue;
paramValues[4] = is_in_recovery ? "true" : "false";

if (!pgsql_execute_with_params(&pgsql, sql, 5, paramTypes, paramValues,
Expand Down
4 changes: 3 additions & 1 deletion src/bin/pg_autoctl/ipaddr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ fetchLocalIPAddress(char *localIpAddress, int size,
hints.ai_socktype = SOCK_STREAM; /* we only want TCP sockets */
hints.ai_protocol = IPPROTO_TCP; /* we only want TCP sockets */

IntString servicePortStr = intToString(servicePort);

if (!GetAddrInfo(serviceName,
intToString(servicePort).strValue,
servicePortStr.strValue,
&hints,
&lookup))
{
Expand Down
70 changes: 47 additions & 23 deletions src/bin/pg_autoctl/monitor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,18 +846,23 @@ monitor_register_node(Monitor *monitor, char *formation,
MonitorAssignedStateParseContext parseContext =
{ { 0 }, assignedState, false };
const char *nodeStateString = NodeStateToString(initialState);
IntString portStr = intToString(port);
IntString systemIdentifierStr = intToString(system_identifier);
IntString desiredNodeIdStr = intToString(desiredNodeId);
IntString desiredGroupIdStr = intToString(desiredGroupId);
IntString candidatePriorityStr = intToString(candidatePriority);

paramValues[0] = formation;
paramValues[1] = host;
paramValues[2] = intToString(port).strValue;
paramValues[2] = portStr.strValue;
paramValues[3] = dbname;
paramValues[4] = name == NULL ? "" : name;
paramValues[5] = intToString(system_identifier).strValue;
paramValues[6] = intToString(desiredNodeId).strValue;
paramValues[7] = intToString(desiredGroupId).strValue;
paramValues[5] = systemIdentifierStr.strValue;
paramValues[6] = desiredNodeIdStr.strValue;
paramValues[7] = desiredGroupIdStr.strValue;
paramValues[8] = nodeStateString;
paramValues[9] = nodeKindToString(kind);
paramValues[10] = intToString(candidatePriority).strValue;
paramValues[10] = candidatePriorityStr.strValue;
paramValues[11] = quorum ? "true" : "false";
paramValues[12] =
IS_EMPTY_STRING_BUFFER(citusClusterName)
Expand Down Expand Up @@ -945,13 +950,16 @@ monitor_node_active(Monitor *monitor,
MonitorAssignedStateParseContext parseContext =
{ { 0 }, assignedState, false };
const char *nodeStateString = NodeStateToString(currentState);
IntString nodeIdStr = intToString(nodeId);
IntString groupIdStr = intToString(groupId);
IntString currentTLIStr = intToString(currentTLI);

paramValues[0] = formation;
paramValues[1] = intToString(nodeId).strValue;
paramValues[2] = intToString(groupId).strValue;
paramValues[1] = nodeIdStr.strValue;
paramValues[2] = groupIdStr.strValue;
paramValues[3] = nodeStateString;
paramValues[4] = pgIsRunning ? "true" : "false";
paramValues[5] = intToString(currentTLI).strValue;
paramValues[5] = currentTLIStr.strValue;
paramValues[6] = currentLSN;
paramValues[7] = pgsrSyncState;

Expand Down Expand Up @@ -1001,7 +1009,8 @@ monitor_set_node_candidate_priority(Monitor *monitor,
int paramCount = 3;
Oid paramTypes[3] = { TEXTOID, TEXTOID, INT4OID };
const char *paramValues[3];
char *candidatePriorityText = intToString(candidate_priority).strValue;
IntString candidatePriorityStr = intToString(candidate_priority);
char *candidatePriorityText = candidatePriorityStr.strValue;
bool success = true;

paramValues[0] = formation;
Expand Down Expand Up @@ -1218,8 +1227,10 @@ monitor_set_formation_number_sync_standbys(Monitor *monitor, char *formation,
Oid paramTypes[2] = { TEXTOID, INT4OID };
const char *paramValues[2];
SingleValueResultContext parseContext = { { 0 }, PGSQL_RESULT_BOOL, false };
IntString numberSyncStandbysStr = intToString(numberSyncStandbys);

paramValues[0] = formation;
paramValues[1] = intToString(numberSyncStandbys).strValue;
paramValues[1] = numberSyncStandbysStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -1256,9 +1267,10 @@ monitor_remove_by_hostname(Monitor *monitor, char *host, int port, bool force,
int paramCount = 3;
Oid paramTypes[3] = { TEXTOID, INT4OID, BOOLOID };
const char *paramValues[3];
IntString portStr = intToString(port);

paramValues[0] = host;
paramValues[1] = intToString(port).strValue;
paramValues[1] = portStr.strValue;
paramValues[2] = force ? "true" : "false";

if (!pgsql_execute_with_params(pgsql, sql,
Expand Down Expand Up @@ -1495,9 +1507,10 @@ monitor_perform_failover(Monitor *monitor, char *formation, int group)
int paramCount = 2;
Oid paramTypes[2] = { TEXTOID, INT4OID };
const char *paramValues[2];
IntString groupStr = intToString(group);

paramValues[0] = formation;
paramValues[1] = intToString(group).strValue;
paramValues[1] = groupStr.strValue;

/*
* pgautofailover.perform_failover() returns VOID.
Expand Down Expand Up @@ -2738,12 +2751,13 @@ monitor_create_formation(Monitor *monitor,
int paramCount = 5;
Oid paramTypes[5] = { TEXTOID, TEXTOID, TEXTOID, BOOLOID, INT4OID };
const char *paramValues[5];
IntString numberSyncStandbysStr = intToString(numberSyncStandbys);

paramValues[0] = formation;
paramValues[1] = kind;
paramValues[2] = dbname;
paramValues[3] = hasSecondary ? "true" : "false";
paramValues[4] = intToString(numberSyncStandbys).strValue;
paramValues[4] = numberSyncStandbysStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -3429,11 +3443,13 @@ monitor_update_node_metadata(Monitor *monitor,
const char *paramValues[4];

SingleValueResultContext context = { { 0 }, PGSQL_RESULT_BOOL, false };
IntString nodeIdStr = intToString(nodeId);
IntString portStr = intToString(port);

paramValues[0] = intToString(nodeId).strValue;
paramValues[0] = nodeIdStr.strValue;
paramValues[1] = name;
paramValues[2] = hostname;
paramValues[3] = intToString(port).strValue;
paramValues[3] = portStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -3477,9 +3493,11 @@ monitor_set_node_system_identifier(Monitor *monitor,

NodeAddress node = { 0 };
NodeAddressParseContext parseContext = { { 0 }, &node, false };
IntString nodeIdStr = intToString(nodeId);
IntString systemIdentifierStr = intToString(system_identifier);

paramValues[0] = intToString(nodeId).strValue;
paramValues[1] = intToString(system_identifier).strValue;
paramValues[0] = nodeIdStr.strValue;
paramValues[1] = systemIdentifierStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -3525,9 +3543,11 @@ monitor_set_group_system_identifier(Monitor *monitor,
const char *paramValues[2];

SingleValueResultContext context = { 0 };
IntString groupIdStr = intToString(groupId);
IntString systemIdentifierStr = intToString(system_identifier);

paramValues[0] = intToString(groupId).strValue;
paramValues[1] = intToString(system_identifier).strValue;
paramValues[0] = groupIdStr.strValue;
paramValues[1] = systemIdentifierStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -3640,8 +3660,9 @@ monitor_start_maintenance(Monitor *monitor, int64_t nodeId, bool *mayRetry)
int paramCount = 1;
Oid paramTypes[1] = { INT8OID };
const char *paramValues[1];
IntString nodeIdStr = intToString(nodeId);

paramValues[0] = intToString(nodeId).strValue;
paramValues[0] = nodeIdStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -3688,8 +3709,9 @@ monitor_stop_maintenance(Monitor *monitor, int64_t nodeId, bool *mayRetry)
int paramCount = 1;
Oid paramTypes[1] = { INT8OID };
const char *paramValues[1];
IntString nodeIdStr = intToString(nodeId);

paramValues[0] = intToString(nodeId).strValue;
paramValues[0] = nodeIdStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down Expand Up @@ -4854,10 +4876,12 @@ monitor_find_node_by_nodeid(Monitor *monitor,
const char *paramValues[3];

NodeAddressArrayParseContext parseContext = { { 0 }, nodesArray, false };
IntString groupIdStr = intToString(groupId);
IntString nodeIdStr = intToString(nodeId);

paramValues[0] = formation;
paramValues[1] = intToString(groupId).strValue;
paramValues[2] = intToString(nodeId).strValue;
paramValues[1] = groupIdStr.strValue;
paramValues[2] = nodeIdStr.strValue;

if (!pgsql_execute_with_params(pgsql, sql,
paramCount, paramTypes, paramValues,
Expand Down
4 changes: 3 additions & 1 deletion src/bin/pg_autoctl/pgctl.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1628,11 +1628,13 @@ pg_ctl_postgres(const char *pg_ctl, const char *pgdata, int pgport,
/* prepare startup.log file in PGDATA */
join_path_components(logfile, pgdata, "startup.log");

IntString pgportStr = intToString(pgport);

args[argsIndex++] = (char *) postgres;
args[argsIndex++] = "-D";
args[argsIndex++] = (char *) pgdata;
args[argsIndex++] = "-p";
args[argsIndex++] = (char *) intToString(pgport).strValue;
args[argsIndex++] = pgportStr.strValue;

if (listen)
{
Expand Down
Loading