git: 1ee9931d6e6f - main - ctld: Convert struct isns_req to a C++ class

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Mon, 04 Aug 2025 19:46:51 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=1ee9931d6e6f0f9fc4151efcac225d742de9e6fa

commit 1ee9931d6e6f0f9fc4151efcac225d742de9e6fa
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2025-08-04 19:38:07 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2025-08-04 19:38:07 +0000

    ctld: Convert struct isns_req to a C++ class
    
    Use a std::vector<> of chars to hold the iSNS packet.  Convert the
    various isns_req_* functions to be class methods instead.
    
    Sponsored by:   Chelsio Communications
    Pull Request:   https://github.com/freebsd/freebsd-src/pull/1794
---
 usr.sbin/ctld/ctld.cc | 113 +++++++++++++-------------------
 usr.sbin/ctld/isns.cc | 176 +++++++++++++++++++-------------------------------
 usr.sbin/ctld/isns.hh |  36 ++++++-----
 3 files changed, 130 insertions(+), 195 deletions(-)

diff --git a/usr.sbin/ctld/ctld.cc b/usr.sbin/ctld/ctld.cc
index bf700d5b4051..2c385baa8c5a 100644
--- a/usr.sbin/ctld/ctld.cc
+++ b/usr.sbin/ctld/ctld.cc
@@ -637,7 +637,7 @@ isns_do_connect(struct isns *isns)
 	return(s);
 }
 
-static int
+static void
 isns_do_register(struct isns *isns, int s, const char *hostname)
 {
 	struct conf *conf = isns->i_conf;
@@ -645,122 +645,100 @@ isns_do_register(struct isns *isns, int s, const char *hostname)
 	struct portal *portal;
 	struct portal_group *pg;
 	struct port *port;
-	struct isns_req *req;
-	int res = 0;
 	uint32_t error;
 
-	req = isns_req_create(ISNS_FUNC_DEVATTRREG, ISNS_FLAG_CLIENT);
-	isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name);
-	isns_req_add_delim(req);
-	isns_req_add_str(req, 1, hostname);
-	isns_req_add_32(req, 2, 2); /* 2 -- iSCSI */
-	isns_req_add_32(req, 6, conf->conf_isns_period);
+	isns_req req(ISNS_FUNC_DEVATTRREG, ISNS_FLAG_CLIENT);
+	req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name);
+	req.add_delim();
+	req.add_str(1, hostname);
+	req.add_32(2, 2); /* 2 -- iSCSI */
+	req.add_32(6, conf->conf_isns_period);
 	TAILQ_FOREACH(pg, &conf->conf_portal_groups, pg_next) {
 		if (pg->pg_unassigned)
 			continue;
 		TAILQ_FOREACH(portal, &pg->pg_portals, p_next) {
-			isns_req_add_addr(req, 16, portal->p_ai);
-			isns_req_add_port(req, 17, portal->p_ai);
+			req.add_addr(16, portal->p_ai);
+			req.add_port(17, portal->p_ai);
 		}
 	}
 	TAILQ_FOREACH(target, &conf->conf_targets, t_next) {
-		isns_req_add_str(req, 32, target->t_name);
-		isns_req_add_32(req, 33, 1); /* 1 -- Target*/
+		req.add_str(32, target->t_name);
+		req.add_32(33, 1); /* 1 -- Target*/
 		if (target->t_alias != NULL)
-			isns_req_add_str(req, 34, target->t_alias);
+			req.add_str(34, target->t_alias);
 		TAILQ_FOREACH(port, &target->t_ports, p_ts) {
 			if ((pg = port->p_portal_group) == NULL)
 				continue;
-			isns_req_add_32(req, 51, pg->pg_tag);
+			req.add_32(51, pg->pg_tag);
 			TAILQ_FOREACH(portal, &pg->pg_portals, p_next) {
-				isns_req_add_addr(req, 49, portal->p_ai);
-				isns_req_add_port(req, 50, portal->p_ai);
+				req.add_addr(49, portal->p_ai);
+				req.add_port(50, portal->p_ai);
 			}
 		}
 	}
-	res = isns_req_send(s, req);
-	if (res < 0) {
+	if (!req.send(s)) {
 		log_warn("send(2) failed for %s", isns->i_addr);
-		goto quit;
+		return;
 	}
-	res = isns_req_receive(s, req);
-	if (res < 0) {
+	if (!req.receive(s)) {
 		log_warn("receive(2) failed for %s", isns->i_addr);
-		goto quit;
+		return;
 	}
-	error = isns_req_get_status(req);
+	error = req.get_status();
 	if (error != 0) {
 		log_warnx("iSNS register error %d for %s", error, isns->i_addr);
-		res = -1;
 	}
-quit:
-	isns_req_free(req);
-	return (res);
 }
 
-static int
+static bool
 isns_do_check(struct isns *isns, int s, const char *hostname)
 {
 	struct conf *conf = isns->i_conf;
-	struct isns_req *req;
-	int res = 0;
 	uint32_t error;
 
-	req = isns_req_create(ISNS_FUNC_DEVATTRQRY, ISNS_FLAG_CLIENT);
-	isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name);
-	isns_req_add_str(req, 1, hostname);
-	isns_req_add_delim(req);
-	isns_req_add(req, 2, 0, NULL);
-	res = isns_req_send(s, req);
-	if (res < 0) {
+	isns_req req(ISNS_FUNC_DEVATTRQRY, ISNS_FLAG_CLIENT);
+	req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name);
+	req.add_str(1, hostname);
+	req.add_delim();
+	req.add(2, 0, NULL);
+	if (!req.send(s)) {
 		log_warn("send(2) failed for %s", isns->i_addr);
-		goto quit;
+		return (false);
 	}
-	res = isns_req_receive(s, req);
-	if (res < 0) {
+	if (!req.receive(s)) {
 		log_warn("receive(2) failed for %s", isns->i_addr);
-		goto quit;
+		return (false);
 	}
-	error = isns_req_get_status(req);
+	error = req.get_status();
 	if (error != 0) {
 		log_warnx("iSNS check error %d for %s", error, isns->i_addr);
-		res = -1;
+		return (false);
 	}
-quit:
-	isns_req_free(req);
-	return (res);
+	return (true);
 }
 
-static int
+static void
 isns_do_deregister(struct isns *isns, int s, const char *hostname)
 {
 	struct conf *conf = isns->i_conf;
-	struct isns_req *req;
-	int res = 0;
 	uint32_t error;
 
-	req = isns_req_create(ISNS_FUNC_DEVDEREG, ISNS_FLAG_CLIENT);
-	isns_req_add_str(req, 32, TAILQ_FIRST(&conf->conf_targets)->t_name);
-	isns_req_add_delim(req);
-	isns_req_add_str(req, 1, hostname);
-	res = isns_req_send(s, req);
-	if (res < 0) {
+	isns_req req(ISNS_FUNC_DEVDEREG, ISNS_FLAG_CLIENT);
+	req.add_str(32, TAILQ_FIRST(&conf->conf_targets)->t_name);
+	req.add_delim();
+	req.add_str(1, hostname);
+	if (!req.send(s)) {
 		log_warn("send(2) failed for %s", isns->i_addr);
-		goto quit;
+		return;
 	}
-	res = isns_req_receive(s, req);
-	if (res < 0) {
+	if (!req.receive(s)) {
 		log_warn("receive(2) failed for %s", isns->i_addr);
-		goto quit;
+		return;
 	}
-	error = isns_req_get_status(req);
+	error = req.get_status();
 	if (error != 0) {
 		log_warnx("iSNS deregister error %d for %s", error, isns->i_addr);
-		res = -1;
 	}
-quit:
-	isns_req_free(req);
-	return (res);
 }
 
 void
@@ -795,7 +773,7 @@ void
 isns_check(struct isns *isns)
 {
 	struct conf *conf = isns->i_conf;
-	int error, s, res;
+	int error, s;
 	char hostname[256];
 
 	if (TAILQ_EMPTY(&conf->conf_targets) ||
@@ -811,8 +789,7 @@ isns_check(struct isns *isns)
 	if (error != 0)
 		log_err(1, "gethostname");
 
-	res = isns_do_check(isns, s, hostname);
-	if (res < 0) {
+	if (!isns_do_check(isns, s, hostname)) {
 		isns_do_deregister(isns, s, hostname);
 		isns_do_register(isns, s, hostname);
 	}
diff --git a/usr.sbin/ctld/isns.cc b/usr.sbin/ctld/isns.cc
index 2f1e0094c0bf..9e27999d2bf9 100644
--- a/usr.sbin/ctld/isns.cc
+++ b/usr.sbin/ctld/isns.cc
@@ -27,7 +27,7 @@
  *
  */
 
-#include <sys/types.h>
+#include <sys/param.h>
 #include <sys/time.h>
 #include <sys/socket.h>
 #include <sys/wait.h>
@@ -43,129 +43,86 @@
 #include "ctld.hh"
 #include "isns.hh"
 
-struct isns_req *
-isns_req_alloc(void)
+isns_req::isns_req(uint16_t func, uint16_t flags)
 {
-	struct isns_req *req;
+	struct isns_hdr hdr;
 
-	req = reinterpret_cast<struct isns_req *>(calloc(1, sizeof(struct isns_req)));
-	if (req == NULL) {
-		log_err(1, "calloc");
-		return (NULL);
-	}
-	req->ir_buflen = sizeof(struct isns_hdr);
-	req->ir_usedlen = 0;
-	req->ir_buf = reinterpret_cast<uint8_t *>(calloc(1, req->ir_buflen));
-	if (req->ir_buf == NULL) {
-		free(req);
-		log_err(1, "calloc");
-		return (NULL);
-	}
-	return (req);
-}
-
-struct isns_req *
-isns_req_create(uint16_t func, uint16_t flags)
-{
-	struct isns_req *req;
-	struct isns_hdr *hdr;
-
-	req = isns_req_alloc();
-	req->ir_usedlen = sizeof(struct isns_hdr);
-	hdr = (struct isns_hdr *)req->ir_buf;
-	be16enc(hdr->ih_version, ISNS_VERSION);
-	be16enc(hdr->ih_function, func);
-	be16enc(hdr->ih_flags, flags);
-	return (req);
+	be16enc(&hdr.ih_version, ISNS_VERSION);
+	be16enc(&hdr.ih_function, func);
+	be16enc(&hdr.ih_flags, flags);
+	append(&hdr, sizeof(hdr));
 }
 
 void
-isns_req_free(struct isns_req *req)
+isns_req::getspace(uint32_t len)
 {
-
-	free(req->ir_buf);
-	free(req);
+	ir_buf.reserve(ir_buf.size() + len);
 }
 
-static int
-isns_req_getspace(struct isns_req *req, uint32_t len)
+void
+isns_req::append(const void *buf, size_t len)
 {
-	void *newbuf;
-	int newlen;
-
-	if (req->ir_usedlen + len <= req->ir_buflen)
-		return (0);
-	newlen = 1 << flsl(req->ir_usedlen + len);
-	newbuf = realloc(req->ir_buf, newlen);
-	if (newbuf == NULL) {
-		log_err(1, "realloc");
-		return (1);
-	}
-	req->ir_buf = reinterpret_cast<uint8_t *>(newbuf);
-	req->ir_buflen = newlen;
-	return (0);
+	const char *cp = reinterpret_cast<const char *>(buf);
+	ir_buf.insert(ir_buf.end(), cp, cp + len);
 }
 
 void
-isns_req_add(struct isns_req *req, uint32_t tag, uint32_t len,
-    const void *value)
+isns_req::add(uint32_t tag, uint32_t len, const void *value)
 {
-	struct isns_tlv *tlv;
+	struct isns_tlv tlv;
 	uint32_t vlen;
 
-	vlen = len + ((len & 3) ? (4 - (len & 3)) : 0);
-	isns_req_getspace(req, sizeof(*tlv) + vlen);
-	tlv = (struct isns_tlv *)&req->ir_buf[req->ir_usedlen];
-	be32enc(tlv->it_tag, tag);
-	be32enc(tlv->it_length, vlen);
-	memcpy(tlv->it_value, value, len);
+	vlen = roundup2(len, 4);
+	getspace(sizeof(tlv) + vlen);
+	be32enc(&tlv.it_tag, tag);
+	be32enc(&tlv.it_length, vlen);
+	append(&tlv, sizeof(tlv));
+	append(value, len);
 	if (vlen != len)
-		memset(&tlv->it_value[len], 0, vlen - len);
-	req->ir_usedlen += sizeof(*tlv) + vlen;
+		ir_buf.insert(ir_buf.end(), vlen - len, 0);
 }
 
 void
-isns_req_add_delim(struct isns_req *req)
+isns_req::add_delim()
 {
-
-	isns_req_add(req, 0, 0, NULL);
+	add(0, 0, nullptr);
 }
 
 void
-isns_req_add_str(struct isns_req *req, uint32_t tag, const char *value)
+isns_req::add_str(uint32_t tag, const char *value)
 {
 
-	isns_req_add(req, tag, strlen(value) + 1, value);
+	add(tag, strlen(value) + 1, value);
 }
 
 void
-isns_req_add_32(struct isns_req *req, uint32_t tag, uint32_t value)
+isns_req::add_32(uint32_t tag, uint32_t value)
 {
 	uint32_t beval;
 
 	be32enc(&beval, value);
-	isns_req_add(req, tag, sizeof(value), &beval);
+	add(tag, sizeof(value), &beval);
 }
 
 void
-isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai)
+isns_req::add_addr(uint32_t tag, const struct addrinfo *ai)
 {
-	struct sockaddr_in *in4;
-	struct sockaddr_in6 *in6;
+	const struct sockaddr_in *in4;
+	const struct sockaddr_in6 *in6;
 	uint8_t buf[16];
 
 	switch (ai->ai_addr->sa_family) {
 	case AF_INET:
-		in4 = (struct sockaddr_in *)(void *)ai->ai_addr;
+		in4 = (const struct sockaddr_in *)ai->ai_addr;
 		memset(buf, 0, 10);
 		buf[10] = 0xff;
 		buf[11] = 0xff;
 		memcpy(&buf[12], &in4->sin_addr, sizeof(in4->sin_addr));
-		isns_req_add(req, tag, sizeof(buf), buf);
+		add(tag, sizeof(buf), buf);
 		break;
 	case AF_INET6:
-		in6 = (struct sockaddr_in6 *)(void *)ai->ai_addr;
-		isns_req_add(req, tag, sizeof(in6->sin6_addr), &in6->sin6_addr);
+		in6 = (const struct sockaddr_in6 *)ai->ai_addr;
+		add(tag, sizeof(in6->sin6_addr), &in6->sin6_addr);
 		break;
 	default:
 		log_errx(1, "Unsupported address family %d",
@@ -174,22 +131,22 @@ isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai)
 }
 
 void
-isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai)
+isns_req::add_port(uint32_t tag, const struct addrinfo *ai)
 {
-	struct sockaddr_in *in4;
-	struct sockaddr_in6 *in6;
+	const struct sockaddr_in *in4;
+	const struct sockaddr_in6 *in6;
 	uint32_t buf;
 
 	switch (ai->ai_addr->sa_family) {
 	case AF_INET:
-		in4 = (struct sockaddr_in *)(void *)ai->ai_addr;
+		in4 = (const struct sockaddr_in *)ai->ai_addr;
 		be32enc(&buf, ntohs(in4->sin_port));
-		isns_req_add(req, tag, sizeof(buf), &buf);
+		add(tag, sizeof(buf), &buf);
 		break;
 	case AF_INET6:
-		in6 = (struct sockaddr_in6 *)(void *)ai->ai_addr;
+		in6 = (const struct sockaddr_in6 *)ai->ai_addr;
 		be32enc(&buf, ntohs(in6->sin6_port));
-		isns_req_add(req, tag, sizeof(buf), &buf);
+		add(tag, sizeof(buf), &buf);
 		break;
 	default:
 		log_errx(1, "Unsupported address family %d",
@@ -197,55 +154,54 @@ isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai)
 	}
 }
 
-int
-isns_req_send(int s, struct isns_req *req)
+bool
+isns_req::send(int s)
 {
 	struct isns_hdr *hdr;
 	int res;
 
-	hdr = (struct isns_hdr *)req->ir_buf;
-	be16enc(hdr->ih_length, req->ir_usedlen - sizeof(*hdr));
+	hdr = (struct isns_hdr *)ir_buf.data();
+	be16enc(hdr->ih_length, ir_buf.size() - sizeof(*hdr));
 	be16enc(hdr->ih_flags, be16dec(hdr->ih_flags) |
 	    ISNS_FLAG_LAST | ISNS_FLAG_FIRST);
 	be16enc(hdr->ih_transaction, 0);
 	be16enc(hdr->ih_sequence, 0);
 
-	res = write(s, req->ir_buf, req->ir_usedlen);
-	return ((res < 0) ? -1 : 0);
+	res = write(s, ir_buf.data(), ir_buf.size());
+	return (res > 0 && (size_t)res == ir_buf.size());
 }
 
-int
-isns_req_receive(int s, struct isns_req *req)
+bool
+isns_req::receive(int s)
 {
 	struct isns_hdr *hdr;
 	ssize_t res, len;
 
-	req->ir_usedlen = 0;
-	isns_req_getspace(req, sizeof(*hdr));
-	res = read(s, req->ir_buf, sizeof(*hdr));
-	if (res < (ssize_t)sizeof(*hdr))
-		return (-1);
-	req->ir_usedlen = sizeof(*hdr);
-	hdr = (struct isns_hdr *)req->ir_buf;
+	ir_buf.resize(sizeof(*hdr));
+	res = read(s, ir_buf.data(), sizeof(*hdr));
+	if (res < (ssize_t)sizeof(*hdr)) {
+		ir_buf.clear();
+		return (false);
+	}
+	hdr = (struct isns_hdr *)ir_buf.data();
 	if (be16dec(hdr->ih_version) != ISNS_VERSION)
-		return (-1);
+		return (false);
 	if ((be16dec(hdr->ih_flags) & (ISNS_FLAG_LAST | ISNS_FLAG_FIRST)) !=
 	    (ISNS_FLAG_LAST | ISNS_FLAG_FIRST))
-		return (-1);
+		return (false);
 	len = be16dec(hdr->ih_length);
-	isns_req_getspace(req, len);
-	res = read(s, &req->ir_buf[req->ir_usedlen], len);
+	ir_buf.resize(sizeof(*hdr) + len);
+	res = read(s, ir_buf.data() + sizeof(*hdr), len);
 	if (res < len)
-		return (-1);
-	req->ir_usedlen += len;
-	return (0);
+		return (false);
+	return (res == len);
 }
 
 uint32_t
-isns_req_get_status(struct isns_req *req)
+isns_req::get_status()
 {
 
-	if (req->ir_usedlen < sizeof(struct isns_hdr) + 4)
+	if (ir_buf.size() < sizeof(struct isns_hdr) + 4)
 		return (-1);
-	return (be32dec(&req->ir_buf[sizeof(struct isns_hdr)]));
+	return (be32dec(&ir_buf[sizeof(struct isns_hdr)]));
 }
diff --git a/usr.sbin/ctld/isns.hh b/usr.sbin/ctld/isns.hh
index 259a98f8b6be..79a288f7d133 100644
--- a/usr.sbin/ctld/isns.hh
+++ b/usr.sbin/ctld/isns.hh
@@ -27,6 +27,8 @@
 #ifndef	__ISNS_HH__
 #define	__ISNS_HH__
 
+#include <vector>
+
 #define	ISNS_VERSION		0x0001
 
 #define	ISNS_FUNC_DEVATTRREG	0x0001
@@ -68,23 +70,23 @@ struct isns_tlv {
 };
 
 struct isns_req {
-	u_int	ir_buflen;
-	u_int	ir_usedlen;
-	uint8_t	*ir_buf;
-};
+	isns_req() {}
+	isns_req(uint16_t func, uint16_t flags);
 
-struct isns_req * isns_req_alloc(void);
-struct isns_req * isns_req_create(uint16_t func, uint16_t flags);
-void isns_req_free(struct isns_req *req);
-void isns_req_add(struct isns_req *req, uint32_t tag, uint32_t len,
-    const void *value);
-void isns_req_add_delim(struct isns_req *req);
-void isns_req_add_str(struct isns_req *req, uint32_t tag, const char *value);
-void isns_req_add_32(struct isns_req *req, uint32_t tag, uint32_t value);
-void isns_req_add_addr(struct isns_req *req, uint32_t tag, struct addrinfo *ai);
-void isns_req_add_port(struct isns_req *req, uint32_t tag, struct addrinfo *ai);
-int isns_req_send(int s, struct isns_req *req);
-int isns_req_receive(int s, struct isns_req *req);
-uint32_t isns_req_get_status(struct isns_req *req);
+	void add(uint32_t tag, uint32_t len, const void *value);
+	void add_delim();
+	void add_str(uint32_t tag, const char *value);
+	void add_32(uint32_t tag, uint32_t value);
+	void add_addr(uint32_t tag, const struct addrinfo *ai);
+	void add_port(uint32_t tag, const struct addrinfo *ai);
+	bool send(int s);
+	bool receive(int s);
+	uint32_t get_status();
+private:
+	void getspace(uint32_t len);
+	void append(const void *buf, size_t len);
+
+	std::vector<char> ir_buf;
+};
 
 #endif /* __ISNS_HH__ */