Skip to content

Commit 92d50bc

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

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
}

‎src/bin/pg_autoctl/parsing.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,8 @@ parse_controldata_field_uint32(const char *controlDataString,
162162
char regex[BUFSIZE];
163163
char *match;
164164

165-
sformat(regex, BUFSIZE, "^%s: *([0-9]+)$", fieldName);
165+
sformat_fail(regex, BUFSIZE, "controldata uint32 parsing regex", "^%s: *([0-9]+)$",
166+
fieldName);
166167
match = regexp_first_match(controlDataString, regex);
167168

168169
if (match == NULL)
@@ -195,7 +196,8 @@ parse_controldata_field_uint64(const char *controlDataString,
195196
char regex[BUFSIZE];
196197
char *match;
197198

198-
sformat(regex, BUFSIZE, "^%s: *([0-9]+)$", fieldName);
199+
sformat_fail(regex, BUFSIZE, "controldata uint64 parsing regex", "^%s: *([0-9]+)$",
200+
fieldName);
199201
match = regexp_first_match(controlDataString, regex);
200202

201203
if (match == NULL)

‎src/bin/pg_autoctl/pgctl.c

Lines changed: 18 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -799,7 +799,8 @@ pg_ctl_start(const char *pg_ctl,
799799
int commandSize = 0;
800800

801801
join_path_components(logfile, pgdata, "startup.log");
802-
sformat(pgport_option, sizeof(pgport_option), "\"-p %d\"", pgport);
802+
sformat_fail(pgport_option, sizeof(pgport_option), "pg_ctl port option", "\"-p %d\"",
803+
pgport);
803804

804805
args[argsIndex++] = (char *) pg_ctl;
805806
args[argsIndex++] = "--pgdata";
@@ -809,8 +810,9 @@ pg_ctl_start(const char *pg_ctl,
809810

810811
if (!IS_EMPTY_STRING_BUFFER(listen_addresses))
811812
{
812-
sformat(listen_addresses_option, sizeof(listen_addresses_option),
813-
"\"-h %s\"", listen_addresses);
813+
sformat_fail(listen_addresses_option, sizeof(listen_addresses_option),
814+
"pg_ctl listen adress option",
815+
"\"-h %s\"", listen_addresses);
814816

815817
args[argsIndex++] = "--options";
816818
args[argsIndex++] = (char *) listen_addresses_option;
@@ -824,10 +826,11 @@ pg_ctl_start(const char *pg_ctl,
824826
/* errors have already been logged */
825827
return false;
826828
}
827-
sformat(option_unix_socket_directory,
828-
sizeof(option_unix_socket_directory),
829-
"\"-k \"%s\"\"",
830-
env_pg_regress_sock_dir);
829+
sformat_fail(option_unix_socket_directory,
830+
sizeof(option_unix_socket_directory),
831+
"pg_ctl unix socket directory option",
832+
"\"-k \"%s\"\"",
833+
env_pg_regress_sock_dir);
831834

832835
/* pg_ctl --options can be specified multiple times */
833836
args[argsIndex++] = "--options";
@@ -1264,7 +1267,6 @@ prepare_primary_conninfo(char *primaryConnInfo,
12641267
SSLOptions sslOptions,
12651268
bool escape)
12661269
{
1267-
int size = 0;
12681270
char escaped[BUFSIZE];
12691271
PQExpBuffer buffer = NULL;
12701272

@@ -1311,15 +1313,9 @@ prepare_primary_conninfo(char *primaryConnInfo,
13111313
}
13121314

13131315
/* now copy the buffer into primaryConnInfo for the caller */
1314-
size = sformat(primaryConnInfo, primaryConnInfoSize, "%s", escaped);
1315-
1316-
if (size == -1 || size > primaryConnInfoSize)
1317-
{
1318-
log_error("BUG: the escaped primary_conninfo requires %d bytes and "
1319-
"pg_auto_failover only support up to %d bytes",
1320-
size, primaryConnInfoSize);
1321-
return false;
1322-
}
1316+
sformat_fail(primaryConnInfo, primaryConnInfoSize, "escaped primary_conninfo",
1317+
"%s",
1318+
escaped);
13231319
}
13241320
else
13251321
{
@@ -1481,19 +1477,11 @@ pg_create_self_signed_cert(PostgresSetup *pgSetup, const char *nodename)
14811477
return false;
14821478
}
14831479

1484-
size = sformat(pgSetup->ssl.serverKey, MAXPGPATH,
1485-
"%s/server.key", pgSetup->pgdata);
1486-
1487-
if (size == -1 || size > MAXPGPATH)
1488-
{
1489-
log_error("BUG: the ssl server key file path requires %d bytes and "
1490-
"pg_auto_failover only support up to %d bytes",
1491-
size, MAXPGPATH);
1492-
return false;
1493-
}
1480+
sformat_fail(pgSetup->ssl.serverKey, MAXPGPATH, "ssl server key file path",
1481+
"%s/server.key", pgSetup->pgdata);
14941482

1495-
size = sformat(pgSetup->ssl.serverCert, MAXPGPATH,
1496-
"%s/server.crt", pgSetup->pgdata);
1483+
sformat_fail(pgSetup->ssl.serverCert, MAXPGPATH, "ssl server cert file",
1484+
"%s/server.crt", pgSetup->pgdata);
14971485

14981486
if (size == -1 || size > MAXPGPATH)
14991487
{
@@ -1503,15 +1491,7 @@ pg_create_self_signed_cert(PostgresSetup *pgSetup, const char *nodename)
15031491
return false;
15041492
}
15051493

1506-
size = sformat(subject, BUFSIZE, "/CN=%s", nodename);
1507-
1508-
if (size == -1 || size > BUFSIZE)
1509-
{
1510-
log_error("BUG: the ssl subject \"/CN=%s\" requires %d bytes and"
1511-
"pg_auto_failover only support up to %d bytes",
1512-
nodename, size, BUFSIZE);
1513-
return false;
1514-
}
1494+
sformat_fail(subject, BUFSIZE, "ssl subject", "/CN=%s", nodename);
15151495

15161496
log_info("Running %s req -new -x509 -days 365 -nodes -text "
15171497
"-out %s -keyout %s -subj \"%s\"",

0 commit comments

Comments
 (0)