Skip to content

Commit 92d50bc

Browse files
JelteFJelteF
committed
Check result code of sformat
1 parent 2f5ce98 commit 92d50bc

File tree

18 files changed

+138
-143
lines changed

18 files changed

+138
-143
lines changed

‎src/bin/pg_autoctl/file_utils.c‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -808,16 +808,16 @@ fformat(FILE *stream, const char *fmt, ...)
808808
/*
809809
* sformat is a secured down version of pg_snprintf
810810
*/
811-
int
812-
sformat(char *str, size_t count, const char *fmt, ...)
811+
bool
812+
sformat(char *str, size_t count, const char *result_name, const char *fmt, ...)
813813
{
814814
int len;
815815
va_list args;
816816

817817
if (str == NULL || fmt == NULL)
818818
{
819819
log_error("BUG: sformat is called with a NULL target or format string");
820-
return -1;
820+
return false;
821821
}
822822

823823
va_start(args, fmt);
@@ -826,10 +826,11 @@ sformat(char *str, size_t count, const char *fmt, ...)
826826

827827
if (len >= count)
828828
{
829-
log_error("BUG: sformat needs %d bytes to expend format string \"%s\", "
830-
"and a target string of %lu bytes only has been given.",
831-
len, fmt, count);
829+
log_error("BUG: the %s requires %d bytes to expand format string \"%s\", "
830+
"and pg_auto_failover only supports up to %lu bytes.",
831+
result_name, len, fmt, count);
832+
return false;
832833
}
833834

834-
return len;
835+
return true;
835836
}

‎src/bin/pg_autoctl/file_utils.h‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,17 @@ bool normalize_filename(const char *filename, char *dst, int size);
4343
int fformat(FILE *stream, const char *fmt, ...)
4444
__attribute__((format(printf, 2, 3)));
4545

46-
int sformat(char *str, size_t count, const char *fmt, ...)
47-
__attribute__((format(printf, 3, 4)));
46+
bool sformat(char *str, size_t count, const char *result_name, const char *fmt, ...)
47+
__attribute__((format(printf, 4, 5)));
48+
49+
#define sformat_fail(str, count, result_name, fmt, ...) \
50+
if (!sformat(str, count, result_name, fmt, __VA_ARGS__)) { \
51+
return false; \
52+
}
53+
54+
#define sformat_exit(str, count, result_name, fmt, ...) \
55+
if (!sformat(str, count, result_name, fmt, __VA_ARGS__)) { \
56+
exit(EXIT_CODE_BAD_CONFIG); \
57+
}
4858

4959
#endif /* FILE_UTILS_H */

‎src/bin/pg_autoctl/fsm_transition.c‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -733,10 +733,11 @@ fsm_init_standby(Keeper *keeper)
733733
replicationSource.sslOptions = config->pgSetup.ssl;
734734

735735
/* prepare our application_name */
736-
sformat(applicationName, BUFSIZE,
737-
"%s%d",
738-
REPLICATION_APPLICATION_NAME_PREFIX,
739-
keeper->state.current_node_id);
736+
(void) sformat(applicationName, BUFSIZE,
737+
"replication application name",
738+
"%s%d",
739+
REPLICATION_APPLICATION_NAME_PREFIX,
740+
keeper->state.current_node_id);
740741
replicationSource.applicationName = applicationName;
741742

742743
if (!standby_init_database(postgres, &replicationSource, config->nodename))
@@ -796,10 +797,11 @@ fsm_rewind_or_init(Keeper *keeper)
796797
replicationSource.sslOptions = config->pgSetup.ssl;
797798

798799
/* prepare our application_name */
799-
sformat(applicationName, BUFSIZE,
800-
"%s%d",
801-
REPLICATION_APPLICATION_NAME_PREFIX,
802-
keeper->state.current_node_id);
800+
(void) sformat(applicationName, BUFSIZE,
801+
"replication application name",
802+
"%s%d",
803+
REPLICATION_APPLICATION_NAME_PREFIX,
804+
keeper->state.current_node_id);
803805
replicationSource.applicationName = applicationName;
804806

805807
if (!primary_rewind_to_standby(postgres, &replicationSource))

‎src/bin/pg_autoctl/ini_file.c‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,14 @@ ini_validate_options(IniOption *optionList)
139139
int n;
140140
char optionName[BUFSIZE];
141141

142-
n = sformat(optionName, BUFSIZE, "%s.%s", option->section, option->name);
142+
sformat_fail(optionName, BUFSIZE, "ini option name", "%s.%s", option->section,
143+
option->name);
144+
n = strlen(optionName);
143145

144146
if (option->optName)
145147
{
146-
sformat(optionName + n, BUFSIZE - n, " (--%s)", option->optName);
148+
sformat_fail(optionName + n, BUFSIZE - n, "ini commandline option name",
149+
" (--%s)", option->optName);
147150
}
148151

149152
switch (option->type)
@@ -304,8 +307,8 @@ ini_option_to_string(IniOption *option, char *dest, size_t size)
304307

305308
case INI_INT_T:
306309
{
307-
sformat(dest, size, "%d", *(option->intValue));
308-
return true;
310+
return sformat(dest, size, "ini stringified option", "%d",
311+
*(option->intValue));
309312
}
310313

311314
default:

‎src/bin/pg_autoctl/ipaddr.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ fetchLocalIPAddress(char *localIpAddress, int size,
9595

9696
if (ipAddr != NULL)
9797
{
98-
sformat(localIpAddress, size, "%s", buffer);
98+
sformat_fail(localIpAddress, size, "local ip address", "%s", buffer);
9999
}
100100
else
101101
{
@@ -253,7 +253,7 @@ fetchLocalCIDR(const char *localIpAddress, char *localCIDR, int size)
253253
return false;
254254
}
255255

256-
sformat(localCIDR, size, "%s/%d", network, prefix);
256+
sformat_fail(localCIDR, size, "local CIDR", "%s/%d", network, prefix);
257257

258258
return true;
259259
}
@@ -585,7 +585,7 @@ findHostnameFromLocalIpAddress(char *localIpAddress, char *hostname, int size)
585585
return false;
586586
}
587587

588-
sformat(hostname, size, "%s", hbuf);
588+
sformat_fail(hostname, size, "hostname", "%s", hbuf);
589589

590590
/* stop at the first hostname found */
591591
break;

‎src/bin/pg_autoctl/keeper_config.c‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -602,7 +602,11 @@ keeper_config_set_groupId_and_slot_name(KeeperConfig *config,
602602
char buffer[BUFSIZE] = { 0 };
603603
char *replicationSlotName = NULL;
604604

605-
sformat(buffer, BUFSIZE, "%s_%d", REPLICATION_SLOT_NAME_DEFAULT, nodeId);
605+
if (postgres_sprintf_replicationSlotName(nodeId, buffer, BUFSIZE))
606+
{
607+
/* we already logged about it */
608+
return false;
609+
}
606610
replicationSlotName = strdup(buffer);
607611

608612
config->groupId = groupId;
@@ -893,7 +897,7 @@ keeper_config_set_backup_directory(KeeperConfig *config, int nodeId)
893897
char absoluteBackupDirectory[PATH_MAX];
894898

895899
/* build the default nodename based backup directory path */
896-
sformat(subdirs, MAXPGPATH, "backup/%s", config->nodename);
900+
sformat_fail(subdirs, MAXPGPATH, "backup path", "backup/%s", config->nodename);
897901
path_in_same_directory(pgdata, subdirs, backupDirectory);
898902

899903
/*
@@ -914,7 +918,7 @@ keeper_config_set_backup_directory(KeeperConfig *config, int nodeId)
914918
/* we might be able to use the nodeId, better than the nodename */
915919
if (nodeId > 0)
916920
{
917-
sformat(subdirs, MAXPGPATH, "backup/node_%d", nodeId);
921+
sformat_fail(subdirs, MAXPGPATH, "backup path", "backup/node_%d", nodeId);
918922
path_in_same_directory(pgdata, subdirs, backupDirectory);
919923
}
920924

‎src/bin/pg_autoctl/keeper_pg_init.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,8 @@ create_database_and_extension(Keeper *keeper)
583583
log_trace("create_database_and_extension");
584584

585585
/* we didn't start PostgreSQL yet, also we just ran initdb */
586-
sformat(hbaFilePath, MAXPGPATH, "%s/pg_hba.conf", pgSetup->pgdata);
586+
sformat_fail(hbaFilePath, MAXPGPATH, "pg_hba.conf path", "%s/pg_hba.conf",
587+
pgSetup->pgdata);
587588

588589
/*
589590
* The Postgres URI given to the user by our facility is going to use

‎src/bin/pg_autoctl/loop.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ create_pidfile(const char *pidfile, pid_t pid)
583583

584584
log_trace("create_pidfile(%d): \"%s\"", pid, pidfile);
585585

586-
sformat(content, BUFSIZE, "%d", pid);
586+
sformat_fail(content, BUFSIZE, "PID value", "%d", pid);
587587

588588
return write_file(content, strlen(content), pidfile);
589589
}

‎src/bin/pg_autoctl/monitor.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1763,7 +1763,7 @@ printLastEvents(void *ctx, PGresult *result)
17631763
char node[BUFSIZE];
17641764

17651765
/* for our grid alignment output it's best to have a single col here */
1766-
sformat(node, BUFSIZE, "%s/%s", groupId, nodeId);
1766+
(void) sformat(node, BUFSIZE, "groupid and nodeid", "%s/%s", groupId, nodeId);
17671767

17681768
fformat(stdout, "%30s | %10s | %6s | %18s | %18s | %s\n",
17691769
eventTime, formation, node,

‎src/bin/pg_autoctl/monitor_config.c‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -475,41 +475,45 @@ monitor_config_get_postgres_uri(MonitorConfig *config, char *connectionString,
475475
* sslcrl connection parameters when using sslmode=verify-ca or
476476
* sslmode=verify-full.
477477
*/
478-
connStringEnd += sformat(connStringEnd,
479-
size - (connStringEnd - connectionString),
480-
"postgres://%s@%s:%d/%s",
481-
config->pgSetup.username,
482-
host,
483-
config->pgSetup.pgport,
484-
config->pgSetup.dbname);
478+
sformat_fail(connStringEnd,
479+
size - (connStringEnd - connectionString),
480+
"monitor connection string",
481+
"postgres://%s@%s:%d/%s",
482+
config->pgSetup.username,
483+
host,
484+
config->pgSetup.pgport,
485+
config->pgSetup.dbname);
486+
connStringEnd += strlen(connStringEnd);
485487

486488
if (config->pgSetup.ssl.sslMode >= SSL_MODE_PREFER)
487489
{
488490
char *sslmode = pgsetup_sslmode_to_string(config->pgSetup.ssl.sslMode);
489491

490-
connStringEnd += sformat(connStringEnd,
491-
size - (connStringEnd - connectionString),
492-
"?sslmode=%s",
493-
sslmode);
492+
sformat_fail(connStringEnd,
493+
size - (connStringEnd - connectionString),
494+
"monitor sslmode option",
495+
"?sslmode=%s",
496+
sslmode);
497+
connStringEnd += strlen(connStringEnd);
494498

495499
if (config->pgSetup.ssl.sslMode >= SSL_MODE_VERIFY_CA)
496500
{
497-
if (IS_EMPTY_STRING_BUFFER(config->pgSetup.ssl.crlFile))
498-
{
499-
connStringEnd +=
500-
sformat(connStringEnd,
501-
size - (connStringEnd - connectionString),
502-
"&sslrootcert=%s",
503-
config->pgSetup.ssl.caFile);
504-
}
505-
else
501+
sformat_fail(connStringEnd,
502+
size - (connStringEnd - connectionString),
503+
"monitor sslrootcert option",
504+
"&sslrootcert=%s",
505+
config->pgSetup.ssl.caFile);
506+
connStringEnd += strlen(connStringEnd);
507+
508+
if (!IS_EMPTY_STRING_BUFFER(config->pgSetup.ssl.crlFile))
506509
{
507-
connStringEnd +=
508-
sformat(connStringEnd,
509-
size - (connStringEnd - connectionString),
510-
"&sslrootcert=%s&sslcrl=%s",
511-
config->pgSetup.ssl.caFile,
512-
config->pgSetup.ssl.crlFile);
510+
sformat_fail(connStringEnd,
511+
size - (connStringEnd - connectionString),
512+
"monitor sslcrl option",
513+
"&sslrootcert=%s&sslcrl=%s",
514+
config->pgSetup.ssl.caFile,
515+
config->pgSetup.ssl.crlFile);
516+
connStringEnd += strlen(connStringEnd);
513517
}
514518
}
515519
}

0 commit comments

Comments
 (0)