git: 326e20fc1252 - main - nvmecontrol: Refactor devlist implementation

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Tue, 05 Nov 2024 01:29:08 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=326e20fc1252577f96df0e53360507180cc9d153

commit 326e20fc1252577f96df0e53360507180cc9d153
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2024-11-05 01:28:26 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2024-11-05 01:28:26 +0000

    nvmecontrol: Refactor devlist implementation
    
    Split out helper functions for scan_namespace and scan_controller.
    While here, replace sprintf() calls with snprintf() and avoid
    leaking the contoller fd if read_controller_data() fails.
    
    Reviewed by:    chuck, imp
    Sponsored by:   Chelsio Communications
    Differential Revision:  https://reviews.freebsd.org/D47354
---
 sbin/nvmecontrol/devlist.c | 99 +++++++++++++++++++++++++++-------------------
 1 file changed, 59 insertions(+), 40 deletions(-)

diff --git a/sbin/nvmecontrol/devlist.c b/sbin/nvmecontrol/devlist.c
index 2b34899d3aea..b2816339bc80 100644
--- a/sbin/nvmecontrol/devlist.c
+++ b/sbin/nvmecontrol/devlist.c
@@ -89,16 +89,68 @@ ns_get_sector_size(struct nvme_namespace_data *nsdata)
 }
 
 static void
-devlist(const struct cmd *f, int argc, char *argv[])
+scan_namespace(int fd, int ctrlr, uint32_t nsid)
+{
+	struct nvme_namespace_data 	nsdata;
+	char				name[64];
+	uint8_t				buf[7];
+	uint64_t			size;
+
+	if (read_namespace_data(fd, nsid, &nsdata) != 0)
+		return;
+	if (nsdata.nsze == 0)
+		return;
+	snprintf(name, sizeof(name), "%s%d%s%d", NVME_CTRLR_PREFIX, ctrlr,
+	    NVME_NS_PREFIX, nsid);
+	size = nsdata.nsze * (uint64_t)ns_get_sector_size(&nsdata);
+	if (opt.human) {
+		humanize_number(buf, sizeof(buf), size, "B",
+		    HN_AUTOSCALE, HN_B | HN_NOSPACE | HN_DECIMAL);
+		printf("  %10s (%s)\n", name, buf);
+	} else {
+		printf("  %10s (%juMB)\n", name, (uintmax_t)size / 1024 / 1024);
+	}
+}
+
+static bool
+scan_controller(int ctrlr)
 {
 	struct nvme_controller_data	cdata;
-	struct nvme_namespace_data	nsdata;
 	char				name[64];
 	uint8_t				mn[64];
-	uint8_t				buf[7];
 	uint32_t			i;
-	uint64_t			size;
-	int				ctrlr, fd, found, ret;
+	int				fd, ret;
+
+	snprintf(name, sizeof(name), "%s%d", NVME_CTRLR_PREFIX, ctrlr);
+
+	ret = open_dev(name, &fd, 0, 0);
+
+	if (ret == EACCES) {
+		warnx("could not open "_PATH_DEV"%s\n", name);
+		return (false);
+	} else if (ret != 0)
+		return (false);
+
+	if (read_controller_data(fd, &cdata) != 0) {
+		close(fd);
+		return (true);
+	}
+
+	nvme_strvis(mn, cdata.mn, sizeof(mn), NVME_MODEL_NUMBER_LENGTH);
+	printf("%6s: %s\n", name, mn);
+
+	for (i = 0; i < cdata.nn; i++) {
+		scan_namespace(fd, ctrlr, i + 1);
+	}
+
+	close(fd);
+	return (true);
+}
+
+static void
+devlist(const struct cmd *f, int argc, char *argv[])
+{
+	int				ctrlr, found;
 
 	if (arg_parse(argc, argv, f))
 		return;
@@ -108,41 +160,8 @@ devlist(const struct cmd *f, int argc, char *argv[])
 
 	while (ctrlr < NVME_MAX_UNIT) {
 		ctrlr++;
-		sprintf(name, "%s%d", NVME_CTRLR_PREFIX, ctrlr);
-
-		ret = open_dev(name, &fd, 0, 0);
-
-		if (ret == EACCES) {
-			warnx("could not open "_PATH_DEV"%s\n", name);
-			continue;
-		} else if (ret != 0)
-			continue;
-
-		found++;
-		if (read_controller_data(fd, &cdata))
-			continue;
-		nvme_strvis(mn, cdata.mn, sizeof(mn), NVME_MODEL_NUMBER_LENGTH);
-		printf("%6s: %s\n", name, mn);
-
-		for (i = 0; i < cdata.nn; i++) {
-			if (read_namespace_data(fd, i + 1, &nsdata))
-				continue;
-			if (nsdata.nsze == 0)
-				continue;
-			sprintf(name, "%s%d%s%d", NVME_CTRLR_PREFIX, ctrlr,
-			    NVME_NS_PREFIX, i + 1);
-			size = nsdata.nsze * (uint64_t)ns_get_sector_size(&nsdata);
-			if (opt.human) {
-				humanize_number(buf, sizeof(buf), size, "B",
-				    HN_AUTOSCALE, HN_B | HN_NOSPACE | HN_DECIMAL);
-				printf("  %10s (%s)\n", name, buf);
-
-			} else {
-				printf("  %10s (%juMB)\n", name, (uintmax_t)size / 1024 / 1024);
-			}
-		}
-
-		close(fd);
+		if (scan_controller(ctrlr))
+			found++;
 	}
 
 	if (found == 0) {