From 0561fd5232dfd6a1a4c6c3748bfedde981e8ddcc Mon Sep 17 00:00:00 2001
From: Vladimir Davydov <vdavydov@tarantool.org>
Date: Wed, 19 Apr 2023 15:01:29 +0300
Subject: [PATCH] box: make box_check_uri_set return parsed uri set

To handle the case when box.cfg.replication isn't changed (required for
SSL certificate update), we need the uri set in box_set_replication.
We could obtain it with cfg_get_uri_set, but calling this function right
after box_check_replication, which already creates the uri set looks
ugly. Let's make box_check_uri_set, box_check_replication, and
box_check_listen return the uri_set, like box_check_uri does.

While we are at it, fix a memory leak in box_check_config, where we
forgot to destroy the uri returned by box_check_bootstrap_leader.
The leak is insignificant and may only happen once so this doesn't
deserve to be mentioned in the changelog.

Needed for tarantool/tarantool-ee#432

NO_DOC=refactoring
NO_TEST=refactoring
NO_CHANGELOG=refactoring
---
 src/box/box.cc | 55 ++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/src/box/box.cc b/src/box/box.cc
index 4c125704cd..f44a14df7e 100644
--- a/src/box/box.cc
+++ b/src/box/box.cc
@@ -1008,7 +1008,10 @@ bad_uri:;
 	return -1;
 }
 
-/** Check validity of a uri passed in configuration option. */
+/**
+ * Check validity of a uri passed in a configuration option.
+ * On success stores the uri in @a uri.
+ */
 static int
 box_check_uri(struct uri *uri, const char *option_name, bool set_diag)
 {
@@ -1027,31 +1030,32 @@ box_check_uri(struct uri *uri, const char *option_name, bool set_diag)
 	return -1;
 }
 
+/**
+ * Check validity of a uri set passed in a configuration option.
+ * On success stores the uri set in @a uri_set.
+ */
 static int
-box_check_uri_set(const char *option_name)
+box_check_uri_set(struct uri_set *uri_set, const char *option_name)
 {
-	struct uri_set uri_set;
-	if (cfg_get_uri_set(option_name, &uri_set) != 0) {
+	if (cfg_get_uri_set(option_name, uri_set) != 0) {
 		diag_set(ClientError, ER_CFG, option_name,
 			 diag_last_error(diag_get())->errmsg);
 		return -1;
 	}
-	int rc = 0;
-	for (int i = 0; i < uri_set.uri_count; i++) {
-		const struct uri *uri = &uri_set.uris[i];
+	for (int i = 0; i < uri_set->uri_count; i++) {
+		const struct uri *uri = &uri_set->uris[i];
 		if (check_uri(uri, option_name, true) != 0) {
-			rc = -1;
-			break;
+			uri_set_destroy(uri_set);
+			return -1;
 		}
 	}
-	uri_set_destroy(&uri_set);
-	return rc;
+	return 0;
 }
 
 static int
-box_check_replication(void)
+box_check_replication(struct uri_set *uri_set)
 {
-	return box_check_uri_set("replication");
+	return box_check_uri_set(uri_set, "replication");
 }
 
 static int
@@ -1085,9 +1089,9 @@ box_check_bootstrap_strategy(void)
 }
 
 static int
-box_check_listen(void)
+box_check_listen(struct uri_set *uri_set)
 {
-	return box_check_uri_set("listen");
+	return box_check_uri_set(uri_set, "listen");
 }
 
 static double
@@ -1613,12 +1617,14 @@ box_check_config(void)
 {
 	struct tt_uuid uuid;
 	struct uri uri;
+	struct uri_set uri_set;
 	box_check_say();
 	box_check_audit();
 	if (box_check_flightrec() != 0)
 		diag_raise();
-	if (box_check_listen() != 0)
+	if (box_check_listen(&uri_set) != 0)
 		diag_raise();
+	uri_set_destroy(&uri_set);
 	if (box_check_auth_type() == NULL)
 		diag_raise();
 	if (box_check_instance_uuid(&uuid) != 0)
@@ -1631,8 +1637,9 @@ box_check_config(void)
 		diag_raise();
 	if (box_check_election_fencing_mode() == ELECTION_FENCING_MODE_INVALID)
 		diag_raise();
-	if (box_check_replication() != 0)
+	if (box_check_replication(&uri_set) != 0)
 		diag_raise();
+	uri_set_destroy(&uri_set);
 	box_check_replication_timeout();
 	box_check_replication_connect_timeout();
 	box_check_replication_connect_quorum();
@@ -1648,6 +1655,7 @@ box_check_config(void)
 		diag_raise();
 	if (box_check_bootstrap_leader(&uri, &uuid) != 0)
 		diag_raise();
+	uri_destroy(&uri);
 	box_check_readahead(cfg_geti("readahead"));
 	box_check_checkpoint_count(cfg_geti("checkpoint_count"));
 	box_check_wal_max_size(cfg_geti64("wal_max_size"));
@@ -1764,9 +1772,10 @@ box_set_replication(void)
 		 */
 		return;
 	}
-
-	if (box_check_replication() != 0)
+	struct uri_set uri_set;
+	if (box_check_replication(&uri_set) != 0)
 		diag_raise();
+	uri_set_destroy(&uri_set);
 	/*
 	 * Try to connect to all replicas within the timeout period.
 	 * Stay in orphan mode in case we fail to connect to at least
@@ -2586,12 +2595,10 @@ box_demote(void)
 int
 box_listen(void)
 {
-	if (box_check_listen() != 0)
-		return -1;
 	struct uri_set uri_set;
-	int rc = cfg_get_uri_set("listen", &uri_set);
-	assert(rc == 0);
-	rc = iproto_listen(&uri_set);
+	if (box_check_listen(&uri_set) != 0)
+		return -1;
+	int rc = iproto_listen(&uri_set);
 	uri_set_destroy(&uri_set);
 	return rc;
 }
-- 
GitLab