repmgrd: clean up PQExpBuffer handling

Unless the PQExpBuffer is required for the duration of the function,
ensure it's always a variable local to the relevant code block. This
mitigates the risk of accidentally accessing a generically named
PQExpBuffer which hasn't been initialised or was previously terminated.

repmgrd-bdr.c

@@ -68,7 +68,6 @@ monitor_bdr(void)
 	t_bdr_node_info bdr_node_info = T_BDR_NODE_INFO_INITIALIZER;
 	RecordStatus record_status;
 	NodeInfoListCell *cell;
-	PQExpBufferData event_details;
 	instr_time	log_status_interval_start;
 
 	/* sanity check local database */
@@ -229,6 +228,7 @@ monitor_bdr(void)
 								if (cell->node_info->node_status == NODE_STATUS_UP)
 								{
 									int			node_unreachable_elapsed = calculate_elapsed(node_unreachable_start);
+									PQExpBufferData event_details;
 
 									initPQExpBuffer(&event_details);
 
@@ -366,7 +366,6 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
 {
 	PGconn	   *next_node_conn = NULL;
 	NodeInfoListCell *cell;
-	PQExpBufferData event_details;
 	t_event_info event_info = T_EVENT_INFO_INITIALIZER;
 	t_node_info target_node = T_NODE_INFO_INITIALIZER;
 	t_node_info failed_node = T_NODE_INFO_INITIALIZER;
@@ -460,6 +459,9 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
 
 	log_debug("this node is the failover handler");
 
+	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 
 		event_info.conninfo_str = target_node.conninfo;
@@ -499,6 +501,7 @@ do_bdr_failover(NodeInfoList *nodes, t_node_info *monitored_node)
 		log_info("%s", event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	unset_bdr_failover_handler(next_node_conn);
 
@@ -513,7 +516,6 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
 {
 	PGconn	   *recovered_node_conn;
 
-	PQExpBufferData event_details;
 	t_event_info event_info = T_EVENT_INFO_INITIALIZER;
 	int			i;
 	bool		slot_reactivated = false;
@@ -543,6 +545,8 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
 	 */
 	if (PQstatus(local_conn) != CONNECTION_OK)
 	{
+		PQExpBufferData event_details;
+
 		local_conn = NULL;
 		log_warning(_("unable to reconnect to local node"));
 
@@ -613,6 +617,8 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
 	node_recovery_elapsed = calculate_elapsed(degraded_monitoring_start);
 	monitored_node->monitoring_state = MS_NORMAL;
 
+	{
+		PQExpBufferData event_details;
 
 		initPQExpBuffer(&event_details);
 
@@ -641,8 +647,7 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
 			event_info.conninfo_str = monitored_node->conninfo;
 			event_info.node_name = monitored_node->node_name;
 
-		create_event_notification_extended(
+			create_event_notification_extended(local_conn,
-										   local_conn,
 											   &config_file_options,
 											   config_file_options.node_id,
 											   "bdr_recovery",
@@ -651,11 +656,11 @@ do_bdr_recovery(NodeInfoList *nodes, t_node_info *monitored_node)
 											   &event_info);
 		}
 
+		termPQExpBuffer(&event_details);
+	}
 
 	update_node_record_set_active(local_conn, monitored_node->node_id, true);
 
-	termPQExpBuffer(&event_details);
-
 	PQfinish(recovered_node_conn);
 
 	return;

repmgrd-physical.c

@@ -229,10 +229,11 @@ void
 monitor_streaming_primary(void)
 {
 	instr_time	log_status_interval_start;
-	PQExpBufferData event_details;
 
 	reset_node_voting_status();
 
+	{
+		PQExpBufferData event_details;
 		initPQExpBuffer(&event_details);
 
 		appendPQExpBuffer(&event_details,
@@ -240,7 +241,6 @@ monitor_streaming_primary(void)
 						  local_node_info.node_name,
 						  local_node_info.node_id);
 
-
 		/* Log startup event */
 		if (startup_event_logged == false)
 		{
@@ -266,6 +266,7 @@ monitor_streaming_primary(void)
 		log_notice("%s", event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	INSTR_TIME_SET_CURRENT(log_status_interval_start);
 	local_node_info.node_status = NODE_STATUS_UP;
@@ -287,11 +288,13 @@ monitor_streaming_primary(void)
 			/* local node is down, we were expecting it to be up */
 			if (local_node_info.node_status == NODE_STATUS_UP)
 			{
-				PQExpBufferData event_details;
+
 				instr_time	local_node_unreachable_start;
 
 				INSTR_TIME_SET_CURRENT(local_node_unreachable_start);
 
+				{
+					PQExpBufferData event_details;
 					initPQExpBuffer(&event_details);
 
 					appendPQExpBufferStr(&event_details,
@@ -299,7 +302,6 @@ monitor_streaming_primary(void)
 
 					log_warning("%s", event_details.data);
 
-				local_node_info.node_status = NODE_STATUS_UNKNOWN;
 
 					/*
 					 * as we're monitoring the primary, no point in trying to
@@ -315,6 +317,9 @@ monitor_streaming_primary(void)
 											  event_details.data);
 
 					termPQExpBuffer(&event_details);
+				}
+
+				local_node_info.node_status = NODE_STATUS_UNKNOWN;
 
 				try_reconnect(&local_conn, &local_node_info);
 
@@ -322,6 +327,7 @@ monitor_streaming_primary(void)
 				{
 					int			local_node_unreachable_elapsed = calculate_elapsed(local_node_unreachable_start);
 					int 		stored_local_node_id = UNKNOWN_NODE_ID;
+					PQExpBufferData event_details;
 
 					initPQExpBuffer(&event_details);
 
@@ -375,6 +381,8 @@ monitor_streaming_primary(void)
 			if (config_file_options.degraded_monitoring_timeout > 0
 				&& degraded_monitoring_elapsed > config_file_options.degraded_monitoring_timeout)
 			{
+				PQExpBufferData event_details;
+
 				initPQExpBuffer(&event_details);
 
 				appendPQExpBuffer(&event_details,
@@ -476,7 +484,6 @@ loop:
 bool
 check_primary_status(int degraded_monitoring_elapsed)
 {
-	PQExpBufferData event_details;
 	PGconn *new_primary_conn;
 	RecordStatus record_status;
 	bool resume_monitoring = true;
@@ -494,6 +501,8 @@ check_primary_status(int degraded_monitoring_elapsed)
 	{
 		if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
 		{
+			PQExpBufferData event_details;
+
 			monitoring_state = MS_NORMAL;
 
 			initPQExpBuffer(&event_details);
@@ -517,6 +526,8 @@ check_primary_status(int degraded_monitoring_elapsed)
 
 	/* the node is now a standby */
 
+	{
+		PQExpBufferData event_details;
 		initPQExpBuffer(&event_details);
 
 		if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
@@ -533,6 +544,7 @@ check_primary_status(int degraded_monitoring_elapsed)
 
 		log_notice("%s", event_details.data);
 		termPQExpBuffer(&event_details);
+	}
 
 	primary_node_id = UNKNOWN_NODE_ID;
 
@@ -567,6 +579,8 @@ check_primary_status(int degraded_monitoring_elapsed)
 	 */
 	if (record_status == RECORD_NOT_FOUND)
 	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 
 		appendPQExpBuffer(&event_details,
@@ -622,6 +636,7 @@ check_primary_status(int degraded_monitoring_elapsed)
 
 	if (resume_monitoring == true)
 	{
+		PQExpBufferData event_details;
 		initPQExpBuffer(&event_details);
 
 		if (degraded_monitoring_elapsed != NO_DEGRADED_MONITORING_ELAPSED)
@@ -671,7 +686,6 @@ monitor_streaming_standby(void)
 {
 	RecordStatus record_status;
 	instr_time	log_status_interval_start;
-	PQExpBufferData event_details;
 
 	MonitoringState local_monitoring_state = MS_NORMAL;
 	instr_time	local_degraded_monitoring_start;
@@ -875,10 +889,13 @@ monitor_streaming_standby(void)
 
 				INSTR_TIME_SET_CURRENT(upstream_node_unreachable_start);
 
-				initPQExpBuffer(&event_details);
 
 				upstream_node_info.node_status = NODE_STATUS_UNKNOWN;
 
+				{
+					PQExpBufferData event_details;
+					initPQExpBuffer(&event_details);
+
 					appendPQExpBuffer(&event_details,
 									  _("unable to connect to upstream node \"%s\" (node ID: %i)"),
 									  upstream_node_info.node_name, upstream_node_info.node_id);
@@ -906,6 +923,7 @@ monitor_streaming_standby(void)
 
 					log_warning("%s", event_details.data);
 					termPQExpBuffer(&event_details);
+				}
 
 				/*
 				 * if local node is unreachable, make a last-minute attempt to reconnect
@@ -923,6 +941,7 @@ monitor_streaming_standby(void)
 				if (upstream_node_info.node_status == NODE_STATUS_UP)
 				{
 					int			upstream_node_unreachable_elapsed = calculate_elapsed(upstream_node_unreachable_start);
+					PQExpBufferData event_details;
 
 					initPQExpBuffer(&event_details);
 
@@ -939,6 +958,10 @@ monitor_streaming_standby(void)
 						{
 							ExecStatusType ping_result;
 
+							/*
+							 * we're returning at the end of this block and no longer require the
+							 * event details buffer
+							 */
 							termPQExpBuffer(&event_details);
 
 							log_notice(_("current upstream node \"%s\" (node ID: %i) is not primary, restarting monitoring"),
@@ -1038,6 +1061,8 @@ monitor_streaming_standby(void)
 			if (config_file_options.degraded_monitoring_timeout > 0
 				&& degraded_monitoring_elapsed > config_file_options.degraded_monitoring_timeout)
 			{
+				PQExpBufferData event_details;
+
 				initPQExpBuffer(&event_details);
 
 				appendPQExpBuffer(&event_details,
@@ -1068,6 +1093,7 @@ monitor_streaming_standby(void)
 
 				if (PQstatus(upstream_conn) == CONNECTION_OK)
 				{
+					PQExpBufferData event_details;
 
 					log_debug("upstream node %i has recovered",
 							  upstream_node_info.node_id);
@@ -1140,6 +1166,7 @@ monitor_streaming_standby(void)
 						int			degraded_monitoring_elapsed;
 						int			former_upstream_node_id = local_node_info.upstream_node_id;
 						NodeInfoList sibling_nodes = T_NODE_INFO_LIST_INITIALIZER;
+						PQExpBufferData event_details;
 
 						update_node_record_set_primary(local_conn,  local_node_info.node_id);
 						record_status = get_node_record(local_conn, local_node_info.node_id, &local_node_info);
@@ -1286,6 +1313,7 @@ loop:
 
 				log_info("%s", monitoring_summary.data);
 				termPQExpBuffer(&monitoring_summary);
+
 				if (monitoring_state == MS_DEGRADED && config_file_options.failover == FAILOVER_AUTOMATIC)
 				{
 					if (PQstatus(local_conn) == CONNECTION_OK && repmgrd_is_paused(local_conn))
@@ -1527,7 +1555,6 @@ monitor_streaming_witness(void)
 	instr_time	log_status_interval_start;
 	instr_time	witness_sync_interval_start;
 
-	PQExpBufferData event_details;
 	RecordStatus record_status;
 
 	int primary_node_id = UNKNOWN_NODE_ID;
@@ -1654,10 +1681,13 @@ monitor_streaming_witness(void)
 
 				INSTR_TIME_SET_CURRENT(upstream_node_unreachable_start);
 
-				initPQExpBuffer(&event_details);
-
 				upstream_node_info.node_status = NODE_STATUS_UNKNOWN;
 
+				{
+					PQExpBufferData event_details;
+
+					initPQExpBuffer(&event_details);
+
 					appendPQExpBuffer(&event_details,
 									  _("unable to connect to primary node \"%s\" (node ID: %i)"),
 									  upstream_node_info.node_name, upstream_node_info.node_id);
@@ -1668,6 +1698,8 @@ monitor_streaming_witness(void)
 										"repmgrd_upstream_disconnect",
 										true,
 										event_details.data);
+					termPQExpBuffer(&event_details);
+				}
 
 				try_reconnect(&primary_conn, &upstream_node_info);
 
@@ -1675,6 +1707,7 @@ monitor_streaming_witness(void)
 				if (upstream_node_info.node_status == NODE_STATUS_UP)
 				{
 					int			upstream_node_unreachable_elapsed = calculate_elapsed(upstream_node_unreachable_start);
+					PQExpBufferData event_details;
 
 					initPQExpBuffer(&event_details);
 
@@ -1742,6 +1775,8 @@ monitor_streaming_witness(void)
 
 				if (PQstatus(primary_conn) == CONNECTION_OK)
 				{
+					PQExpBufferData event_details;
+
 					upstream_node_info.node_status = NODE_STATUS_UP;
 					monitoring_state = MS_NORMAL;
 
@@ -2517,7 +2552,6 @@ update_monitoring_history(void)
 static bool
 do_upstream_standby_failover(void)
 {
-	PQExpBufferData event_details;
 	t_node_info primary_node_info = T_NODE_INFO_INITIALIZER;
 	RecordStatus record_status = RECORD_NOT_FOUND;
 	RecoveryType primary_type = RECTYPE_UNKNOWN;
@@ -2599,6 +2633,8 @@ do_upstream_standby_failover(void)
 
 	if (standby_follow_result != 0)
 	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 
 		appendPQExpBuffer(&event_details,
@@ -2676,6 +2712,8 @@ do_upstream_standby_failover(void)
 											local_node_info.node_id,
 											primary_node_info.node_id) == false)
 		{
+			PQExpBufferData event_details;
+
 			initPQExpBuffer(&event_details);
 			appendPQExpBuffer(&event_details,
 							  _("unable to set node %i's new upstream ID to %i"),
@@ -2710,6 +2748,9 @@ do_upstream_standby_failover(void)
 		local_node_info.upstream_node_id = primary_node_info.node_id;
 	}
 
+	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 
 		appendPQExpBuffer(&event_details,
@@ -2727,6 +2768,7 @@ do_upstream_standby_failover(void)
 								  event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	/* keep the primary connection open */
 
@@ -2737,7 +2779,6 @@ do_upstream_standby_failover(void)
 static FailoverState
 promote_self(void)
 {
-	PQExpBufferData event_details;
 	char	   *promote_command;
 	int			r;
 
@@ -2802,10 +2843,13 @@ promote_self(void)
 		int			primary_node_id;
 
 		upstream_conn = get_primary_connection(local_conn,
-											   &primary_node_id, NULL);
+											   &primary_node_id,
+											   NULL);
 
 		if (PQstatus(upstream_conn) == CONNECTION_OK && primary_node_id == failed_primary.node_id)
 		{
+			PQExpBufferData event_details;
+
 			log_notice(_("original primary (id: %i) reappeared before this standby was promoted - no action taken"),
 					   failed_primary.node_id);
 
@@ -2833,16 +2877,13 @@ promote_self(void)
 
 
 		log_error(_("promote command failed"));
-		initPQExpBuffer(&event_details);
 
-		create_event_notification(
+		create_event_notification(NULL,
-								  NULL,
 								  &config_file_options,
 								  local_node_info.node_id,
 								  "repmgrd_promote_error",
 								  true,
-								  event_details.data);
+								  "");
-		termPQExpBuffer(&event_details);
 
 		return FAILOVER_STATE_PROMOTION_FAILED;
 	}
@@ -2850,7 +2891,8 @@ promote_self(void)
 	/* bump the electoral term */
 	increment_current_term(local_conn);
 
-	initPQExpBuffer(&event_details);
+	{
+		PQExpBufferData event_details;
 
 		/* update own internal node record */
 		record_status = get_node_record(local_conn, local_node_info.node_id, &local_node_info);
@@ -2858,6 +2900,8 @@ promote_self(void)
 		/*
 		 * XXX here we're assuming the promote command updated metadata
 		 */
+		initPQExpBuffer(&event_details);
+
 		appendPQExpBuffer(&event_details,
 						  _("node %i promoted to primary; old primary %i marked as failed"),
 						  local_node_info.node_id,
@@ -2872,6 +2916,7 @@ promote_self(void)
 								  event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	return FAILOVER_STATE_PROMOTED;
 }
@@ -2968,8 +3013,6 @@ static FailoverState
 follow_new_primary(int new_primary_id)
 {
 	char		parsed_follow_command[MAXPGPATH] = "";
-
-	PQExpBufferData event_details;
 	int			i, r;
 
 	/* Store details of the failed node here */
@@ -3069,6 +3112,8 @@ follow_new_primary(int new_primary_id)
 
 			if (upstream_recovery_type == RECTYPE_PRIMARY)
 			{
+				PQExpBufferData event_details;
+
 				initPQExpBuffer(&event_details);
 				appendPQExpBufferStr(&event_details,
 									 _("original primary reappeared - no action taken"));
@@ -3148,6 +3193,9 @@ follow_new_primary(int new_primary_id)
 	repmgrd_set_local_node_id(local_conn, config_file_options.node_id);
 	repmgrd_set_pid(local_conn, getpid(), pid_file);
 
+	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 		appendPQExpBuffer(&event_details,
 						  _("node %i now following new upstream node %i"),
@@ -3164,6 +3212,7 @@ follow_new_primary(int new_primary_id)
 								  event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	return FAILOVER_STATE_FOLLOWED_NEW_PRIMARY;
 }
@@ -3172,8 +3221,6 @@ follow_new_primary(int new_primary_id)
 static FailoverState
 witness_follow_new_primary(int new_primary_id)
 {
-	PQExpBufferData event_details;
-
 	t_node_info new_primary = T_NODE_INFO_INITIALIZER;
 	RecordStatus record_status = RECORD_NOT_FOUND;
 	bool		new_primary_ok = false;
@@ -3250,6 +3297,9 @@ witness_follow_new_primary(int new_primary_id)
 		return FAILOVER_STATE_FOLLOW_FAIL;
 	}
 
+	{
+		PQExpBufferData event_details;
+
 		initPQExpBuffer(&event_details);
 		appendPQExpBuffer(&event_details,
 						  _("witness node %i now following new primary node %i"),
@@ -3258,8 +3308,7 @@ witness_follow_new_primary(int new_primary_id)
 
 		log_notice("%s", event_details.data);
 
-	create_event_notification(
+		create_event_notification(upstream_conn,
-							  upstream_conn,
 								  &config_file_options,
 								  local_node_info.node_id,
 								  "repmgrd_failover_follow",
@@ -3267,6 +3316,7 @@ witness_follow_new_primary(int new_primary_id)
 								  event_details.data);
 
 		termPQExpBuffer(&event_details);
+	}
 
 	return FAILOVER_STATE_FOLLOWED_NEW_PRIMARY;
 }
@@ -3696,6 +3746,8 @@ do_election(NodeInfoList *sibling_nodes, int *new_primary_id)
 
 		reset_node_voting_status();
 
+		termPQExpBuffer(&nodes_with_primary_visible);
+
 		return ELECTION_CANCELLED;
 	}