Check result code of sformat

Author: JelteF

src/bin/pg_autoctl/file_utils.c

@@ -808,16 +808,16 @@ fformat(FILE *stream, const char *fmt, ...)
 /*
  * sformat is a secured down version of pg_snprintf
  */
-int
-sformat(char *str, size_t count, const char *fmt, ...)
+bool
+sformat(char *str, size_t count, const char *result_name, const char *fmt, ...)
 {
 	int len;
 	va_list args;
 
 	if (str == NULL || fmt == NULL)
 	{
 		log_error("BUG: sformat is called with a NULL target or format string");
-		return -1;
+		return false;
 	}
 
 	va_start(args, fmt);
@@ -826,10 +826,11 @@ sformat(char *str, size_t count, const char *fmt, ...)
 
 	if (len >= count)
 	{
-		log_error("BUG: sformat needs %d bytes to expend format string \"%s\", "
-				  "and a target string of %lu bytes only has been given.",
-				  len, fmt, count);
+		log_error("BUG: the %s requires %d bytes to expand format string \"%s\", "
+				  "and pg_auto_failover only supports up to %lu bytes.",
+				  result_name, len, fmt, count);
+		return false;
 	}
 
-	return len;
+	return true;
 }

src/bin/pg_autoctl/file_utils.h

@@ -43,7 +43,19 @@ bool normalize_filename(const char *filename, char *dst, int size);
 int fformat(FILE *stream, const char *fmt, ...)
 __attribute__((format(printf, 2, 3)));
 
-int sformat(char *str, size_t count, const char *fmt, ...)
-__attribute__((format(printf, 3, 4)));
+bool sformat(char *str, size_t count, const char *result_name, const char *fmt, ...)
+__attribute__((format(printf, 4, 5)));
+
+#define sformat_fail(str, count, result_name, fmt, ...) \
+	if (!sformat(str, count, result_name, fmt, __VA_ARGS__)) { \
+		log_debug("lineinfo for string formatting failure"); \
+		return false; \
+	}
+
+#define sformat_exit(str, count, result_name, fmt, ...) \
+	if (!sformat(str, count, result_name, fmt, __VA_ARGS__)) { \
+		log_debug("lineinfo for string formatting failure"); \
+		exit(EXIT_CODE_BAD_CONFIG); \
+	}
 
 #endif /* FILE_UTILS_H */

src/bin/pg_autoctl/fsm_transition.c

@@ -733,10 +733,11 @@ fsm_init_standby(Keeper *keeper)
 	replicationSource.sslOptions = config->pgSetup.ssl;
 
 	/* prepare our application_name */
-	sformat(applicationName, BUFSIZE,
-			"%s%d",
-			REPLICATION_APPLICATION_NAME_PREFIX,
-			keeper->state.current_node_id);
+	(void) sformat(applicationName, BUFSIZE,
+				   "replication application name",
+				   "%s%d",
+				   REPLICATION_APPLICATION_NAME_PREFIX,
+				   keeper->state.current_node_id);
 	replicationSource.applicationName = applicationName;
 
 	if (!standby_init_database(postgres, &replicationSource, config->nodename))
@@ -796,10 +797,11 @@ fsm_rewind_or_init(Keeper *keeper)
 	replicationSource.sslOptions = config->pgSetup.ssl;
 
 	/* prepare our application_name */
-	sformat(applicationName, BUFSIZE,
-			"%s%d",
-			REPLICATION_APPLICATION_NAME_PREFIX,
-			keeper->state.current_node_id);
+	(void) sformat(applicationName, BUFSIZE,
+				   "replication application name",
+				   "%s%d",
+				   REPLICATION_APPLICATION_NAME_PREFIX,
+				   keeper->state.current_node_id);
 	replicationSource.applicationName = applicationName;
 
 	if (!primary_rewind_to_standby(postgres, &replicationSource))

src/bin/pg_autoctl/ini_file.c

@@ -139,11 +139,14 @@ ini_validate_options(IniOption *optionList)
 		int n;
 		char optionName[BUFSIZE];
 
-		n = sformat(optionName, BUFSIZE, "%s.%s", option->section, option->name);
+		sformat_fail(optionName, BUFSIZE, "ini option name", "%s.%s", option->section,
+					 option->name);
+		n = strlen(optionName);
 
 		if (option->optName)
 		{
-			sformat(optionName + n, BUFSIZE - n, " (--%s)", option->optName);
+			sformat_fail(optionName + n, BUFSIZE - n, "ini commandline option name",
+						 " (--%s)", option->optName);
 		}
 
 		switch (option->type)
@@ -304,8 +307,8 @@ ini_option_to_string(IniOption *option, char *dest, size_t size)
 
 		case INI_INT_T:
 		{
-			sformat(dest, size, "%d", *(option->intValue));
-			return true;
+			return sformat(dest, size, "ini stringified option", "%d",
+						   *(option->intValue));
 		}
 
 		default:

src/bin/pg_autoctl/ipaddr.c

@@ -95,7 +95,7 @@ fetchLocalIPAddress(char *localIpAddress, int size,
 
 	if (ipAddr != NULL)
 	{
-		sformat(localIpAddress, size, "%s", buffer);
+		sformat_fail(localIpAddress, size, "local ip address", "%s", buffer);
 	}
 	else
 	{
@@ -253,7 +253,7 @@ fetchLocalCIDR(const char *localIpAddress, char *localCIDR, int size)
 		return false;
 	}
 
-	sformat(localCIDR, size, "%s/%d", network, prefix);
+	sformat_fail(localCIDR, size, "local CIDR", "%s/%d", network, prefix);
 
 	return true;
 }
@@ -585,7 +585,7 @@ findHostnameFromLocalIpAddress(char *localIpAddress, char *hostname, int size)
 			return false;
 		}
 
-		sformat(hostname, size, "%s", hbuf);
+		sformat_fail(hostname, size, "hostname", "%s", hbuf);
 
 		/* stop at the first hostname found */
 		break;

src/bin/pg_autoctl/keeper_config.c

@@ -602,7 +602,11 @@ keeper_config_set_groupId_and_slot_name(KeeperConfig *config,
 	char buffer[BUFSIZE] = { 0 };
 	char *replicationSlotName = NULL;
 
-	sformat(buffer, BUFSIZE, "%s_%d", REPLICATION_SLOT_NAME_DEFAULT, nodeId);
+	if (!postgres_sprintf_replicationSlotName(nodeId, buffer, BUFSIZE))
+	{
+		/* we already logged about it */
+		return false;
+	}
 	replicationSlotName = strdup(buffer);
 
 	config->groupId = groupId;
@@ -893,7 +897,7 @@ keeper_config_set_backup_directory(KeeperConfig *config, int nodeId)
 	char absoluteBackupDirectory[PATH_MAX];
 
 	/* build the default nodename based backup directory path */
-	sformat(subdirs, MAXPGPATH, "backup/%s", config->nodename);
+	sformat_fail(subdirs, MAXPGPATH, "backup path", "backup/%s", config->nodename);
 	path_in_same_directory(pgdata, subdirs, backupDirectory);
 
 	/*
@@ -914,7 +918,7 @@ keeper_config_set_backup_directory(KeeperConfig *config, int nodeId)
 		/* we might be able to use the nodeId, better than the nodename */
 		if (nodeId > 0)
 		{
-			sformat(subdirs, MAXPGPATH, "backup/node_%d", nodeId);
+			sformat_fail(subdirs, MAXPGPATH, "backup path", "backup/node_%d", nodeId);
 			path_in_same_directory(pgdata, subdirs, backupDirectory);
 		}
 

src/bin/pg_autoctl/keeper_pg_init.c

@@ -583,7 +583,8 @@ create_database_and_extension(Keeper *keeper)
 	log_trace("create_database_and_extension");
 
 	/* we didn't start PostgreSQL yet, also we just ran initdb */
-	sformat(hbaFilePath, MAXPGPATH, "%s/pg_hba.conf", pgSetup->pgdata);
+	sformat_fail(hbaFilePath, MAXPGPATH, "pg_hba.conf path", "%s/pg_hba.conf",
+				 pgSetup->pgdata);
 
 	/*
 	 * The Postgres URI given to the user by our facility is going to use

src/bin/pg_autoctl/loop.c

@@ -583,7 +583,7 @@ create_pidfile(const char *pidfile, pid_t pid)
 
 	log_trace("create_pidfile(%d): \"%s\"", pid, pidfile);
 
-	sformat(content, BUFSIZE, "%d", pid);
+	sformat_fail(content, BUFSIZE, "PID value", "%d", pid);
 
 	return write_file(content, strlen(content), pidfile);
 }

src/bin/pg_autoctl/monitor.c

@@ -1763,7 +1763,7 @@ printLastEvents(void *ctx, PGresult *result)
 		char node[BUFSIZE];
 
 		/* for our grid alignment output it's best to have a single col here */
-		sformat(node, BUFSIZE, "%s/%s", groupId, nodeId);
+		(void) sformat(node, BUFSIZE, "groupid and nodeid", "%s/%s", groupId, nodeId);
 
 		fformat(stdout, "%30s | %10s | %6s | %18s | %18s | %s\n",
 				eventTime, formation, node,

src/bin/pg_autoctl/monitor_config.c

@@ -475,41 +475,45 @@ monitor_config_get_postgres_uri(MonitorConfig *config, char *connectionString,
 	 * sslcrl connection parameters when using sslmode=verify-ca or
 	 * sslmode=verify-full.
 	 */
-	connStringEnd += sformat(connStringEnd,
-							 size - (connStringEnd - connectionString),
-							 "postgres://%s@%s:%d/%s",
-							 config->pgSetup.username,
-							 host,
-							 config->pgSetup.pgport,
-							 config->pgSetup.dbname);
+	sformat_fail(connStringEnd,
+				 size - (connStringEnd - connectionString),
+				 "monitor connection string",
+				 "postgres://%s@%s:%d/%s",
+				 config->pgSetup.username,
+				 host,
+				 config->pgSetup.pgport,
+				 config->pgSetup.dbname);
+	connStringEnd += strlen(connStringEnd);
 
 	if (config->pgSetup.ssl.sslMode >= SSL_MODE_PREFER)
 	{
 		char *sslmode = pgsetup_sslmode_to_string(config->pgSetup.ssl.sslMode);
 
-		connStringEnd += sformat(connStringEnd,
-								 size - (connStringEnd - connectionString),
-								 "?sslmode=%s",
-								 sslmode);
+		sformat_fail(connStringEnd,
+					 size - (connStringEnd - connectionString),
+					 "monitor sslmode option",
+					 "?sslmode=%s",
+					 sslmode);
+		connStringEnd += strlen(connStringEnd);
 
 		if (config->pgSetup.ssl.sslMode >= SSL_MODE_VERIFY_CA)
 		{
-			if (IS_EMPTY_STRING_BUFFER(config->pgSetup.ssl.crlFile))
-			{
-				connStringEnd +=
-					sformat(connStringEnd,
-							size - (connStringEnd - connectionString),
-							"&sslrootcert=%s",
-							config->pgSetup.ssl.caFile);
-			}
-			else
+			sformat_fail(connStringEnd,
+						 size - (connStringEnd - connectionString),
+						 "monitor sslrootcert option",
+						 "&sslrootcert=%s",
+						 config->pgSetup.ssl.caFile);
+			connStringEnd += strlen(connStringEnd);
+
+			if (!IS_EMPTY_STRING_BUFFER(config->pgSetup.ssl.crlFile))
 			{
-				connStringEnd +=
-					sformat(connStringEnd,
-							size - (connStringEnd - connectionString),
-							"&sslrootcert=%s&sslcrl=%s",
-							config->pgSetup.ssl.caFile,
-							config->pgSetup.ssl.crlFile);
+				sformat_fail(connStringEnd,
+							 size - (connStringEnd - connectionString),
+							 "monitor sslcrl option",
+							 "&sslrootcert=%s&sslcrl=%s",
+							 config->pgSetup.ssl.caFile,
+							 config->pgSetup.ssl.crlFile);
+				connStringEnd += strlen(connStringEnd);
 			}
 		}
 	}