Refactor: call find_site_by_name just once, up the stream

Respective logic was duplicated for all "booth list/peers/grant/revoke" and "geostore list/get/set/del" separately, so utilize a natural control flow to carry this once-resolved target site from here, sharing it with the special case of "daemon" role invoked with "-s site" (debug mode). Side effect: simpler, terser code.
ClusterLabs · Sep 13, 2016 · 8898484 · 8898484
1 parent b6d6960
commit 8898484
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 51 deletions.
diff --git a/src/attr.c b/src/attr.c
@@ -149,24 +149,13 @@ static int read_server_reply(
 	return rv;
 }
 
-int do_attr_command(cmd_request_t cmd)
+int do_attr_command(cmd_request_t cmd, struct booth_site *site)
 {
-	struct booth_site *site = NULL;
 	struct boothc_header *header;
 	struct booth_transport const *tpt;
 	int len, rv = -1;
 	char *msg = NULL;
 
-	if (!*cl.site)
-		site = local;
-	else {
-		if (!find_site_by_name(cl.site, &site, 1)) {
-			log_error("Site \"%s\" not configured.", cl.site);
-			rv = -ENOENT;
-			goto out_close;
-		}
-	}
-
 	if (site->type == ARBITRATOR) {
 		if (site == local) {
 			log_error("We're just an arbitrator, no attributes here.");

diff --git a/src/attr.h b/src/attr.h
@@ -31,7 +31,7 @@
 
 void print_geostore_usage(void);
 int test_attr_reply(cmd_result_t reply_code, cmd_request_t cmd);
-int do_attr_command(cmd_request_t cmd);
+int do_attr_command(cmd_request_t cmd, struct booth_site *site);
 int process_attr_request(struct client *req_client, void *buf);
 int attr_recv(void *buf, struct booth_site *source);
 int store_geo_attr(struct ticket_config *tk, const char *name, const char *val, int notime);

diff --git a/src/main.c b/src/main.c
@@ -346,7 +346,7 @@ int update_authkey()
 	return 0;
 }
 
-static int setup_config(int type)
+static int setup_config(int type, struct booth_site **site)
 {
 	int rv;
 
@@ -370,14 +370,19 @@ static int setup_config(int type)
 	}
 
 	/* Set "local" pointer, ignoring errors. */
-	if (cl.type == DAEMON && cl.site[0]) {
-		if (!find_site_by_name(cl.site, &local, 1)) {
-			log_error("Cannot find \"%s\" (myself) in the configuration.",
-				  cl.site);
+	if ((cl.type == DAEMON || site) && cl.site[0]) {
+		if (!find_site_by_name(cl.site,
+				       cl.type == DAEMON ? &local : site, 1)) {
+			log_error("Cannot find \"%s\"%s in the configuration.",
+				  cl.site, cl.type == DAEMON ? " (myself)" : "");
 			return -EINVAL;
 		}
-		local->local = 1;
-	} else if (!find_myself(NULL, type == CLIENT || type == GEOSTORE)) {
+		if (cl.type == DAEMON) {
+			local->local = 1;
+			if (site)
+				*site = local;
+		}
+	} else if (!find_myself(site, type == CLIENT || type == GEOSTORE)) {
 		log_error("Cannot find myself in the configuration.");
 		return -EINVAL;
 	}
@@ -635,9 +640,8 @@ static int test_reply(cmd_result_t reply_code, cmd_request_t cmd)
 	return rv;
 }
 
-static int query_get_string_answer(cmd_request_t cmd)
+static int query_get_string_answer(cmd_request_t cmd, struct booth_site *site)
 {
-	struct booth_site *site;
 	struct boothc_hdr_msg reply;
 	struct boothc_header *header;
 	char *data;
@@ -662,14 +666,6 @@ static int query_get_string_answer(cmd_request_t cmd)
 
 	init_header(header, cmd, 0, cl.options, 0, 0, msg_size);
 
-	if (!*cl.site)
-		site = local;
-	else if (!find_site_by_name(cl.site, &site, 1)) {
-		log_error("cannot find site \"%s\"", cl.site);
-		rv = -ENOENT;
-		goto out;
-	}
-
 	tpt = booth_transport + TCP;
 	rv = tpt->open(site);
 	if (rv < 0)
@@ -709,16 +705,14 @@ static int query_get_string_answer(cmd_request_t cmd)
 	rv = test_reply_f(ntohl(reply.header.result), cmd);
 out_close:
 	tpt->close(site);
-out:
 	if (data)
 		free(data);
 	return rv;
 }
 
 
-static int do_command(cmd_request_t cmd)
+static int do_command(cmd_request_t cmd, struct booth_site *site)
 {
-	struct booth_site *site;
 	struct boothc_ticket_msg reply;
 	struct booth_transport const *tpt;
 	uint32_t leader_id;
@@ -737,16 +731,6 @@ static int do_command(cmd_request_t cmd)
 	/* Always use TCP for client - at least for now. */
 	tpt = booth_transport + TCP;
 
-	if (!*cl.site)
-		site = local;
-	else {
-		if (!find_site_by_name(cl.site, &site, 1)) {
-			log_error("Site \"%s\" not configured.", cl.site);
-			rv = -ENOENT;
-			goto out_close;
-		}
-	}
-
 	if (site->type == ARBITRATOR) {
 		if (site == local) {
 			log_error("We're just an arbitrator, cannot grant/revoke tickets here.");
@@ -1296,7 +1280,7 @@ static int do_status(int type)
 
 	ret = PCMK_OCF_NOT_RUNNING;
 
-	rv = setup_config(type);
+	rv = setup_config(type, NULL);
 	if (rv) {
 		reason = "Error reading configuration.";
 		ret = PCMK_OCF_UNKNOWN_ERROR;
@@ -1412,7 +1396,7 @@ static int do_server(int type)
 	int rv = -1;
 	static char log_ent[128] = DAEMON_NAME "-";
 
-	rv = setup_config(type);
+	rv = setup_config(type, NULL);
 	if (rv < 0)
 		return rv;
 
@@ -1478,8 +1462,9 @@ static int do_server(int type)
 static int do_client(void)
 {
 	int rv;
+	struct booth_site *site;
 
-	rv = setup_config(CLIENT);
+	rv = setup_config(CLIENT, &site);
 	if (rv < 0) {
 		log_error("cannot read config");
 		goto out;
@@ -1488,12 +1473,12 @@ static int do_client(void)
 	switch (cl.op) {
 	case CMD_LIST:
 	case CMD_PEERS:
-		rv = query_get_string_answer(cl.op);
+		rv = query_get_string_answer(cl.op, site);
 		break;
 
 	case CMD_GRANT:
 	case CMD_REVOKE:
-		rv = do_command(cl.op);
+		rv = do_command(cl.op, site);
 		break;
 	}
 
@@ -1504,8 +1489,9 @@ static int do_client(void)
 static int do_attr(void)
 {
 	int rv = -1;
+	struct booth_site *site;
 
-	rv = setup_config(GEOSTORE);
+	rv = setup_config(GEOSTORE, &site);
 	if (rv < 0) {
 		log_error("cannot read config");
 		goto out;
@@ -1529,12 +1515,12 @@ static int do_attr(void)
 	switch (cl.op) {
 	case ATTR_LIST:
 	case ATTR_GET:
-		rv = query_get_string_answer(cl.op);
+		rv = query_get_string_answer(cl.op, site);
 		break;
 
 	case ATTR_SET:
 	case ATTR_DEL:
-		rv = do_attr_command(cl.op);
+		rv = do_attr_command(cl.op, site);
 		break;
 	}