1 files changed, 159 insertions, 236 deletions

diff --git a/src/nvme/fabrics.c b/src/nvme/fabrics.c
index f0a06e8..4e042d8 100644
--- a/src/nvme/fabrics.c
+++ b/src/nvme/fabrics.c
@@ -32,6 +32,7 @@
 #include <ccan/array_size/array_size.h>
 #include <ccan/str/str.h>
 
+#include "cleanup.h"
 #include "fabrics.h"
 #include "linux.h"
 #include "ioctl.h"
@@ -47,7 +48,7 @@
 const char *nvmf_dev = "/dev/nvme-fabrics";
 
 /**
- * strchomp() - Strip trailing white space
+ * strchomp() - Strip trailing spaces
  * @str: String to strip
  * @max: Maximum length of string
  */
@@ -55,11 +56,8 @@ static void strchomp(char *str, int max)
 {
 	int i;
 
-	for (i = max - 1; i >= 0; i--) {
-		if (str[i] != '\0' && str[i] != ' ')
-			return;
-		else
-			str[i] = '\0';
+	for (i = max - 1; i >= 0 && str[i] == ' '; i--) {
+		str[i] = '\0';
 	}
 }
 
@@ -357,10 +355,16 @@ static int __add_argument(char **argstr, const char *tok, const char *arg)
 	return 0;
 }
 
+static int __nvmf_supported_options(nvme_root_t r);
+#define nvmf_check_option(r, tok)					\
+({									\
+	!__nvmf_supported_options(r) && (r)->options->tok;		\
+})
+
 #define add_bool_argument(o, argstr, tok, arg)				\
 ({									\
 	int ret;							\
-	if (r->options->tok) {						\
+	if (nvmf_check_option(r, tok)) {				\
 		ret = __add_bool_argument(argstr,			\
 					  stringify(tok),		\
 					  arg);				\
@@ -376,7 +380,7 @@ static int __add_argument(char **argstr, const char *tok, const char *arg)
 #define add_int_argument(o, argstr, tok, arg, allow_zero) \
 ({									\
 	int ret;							\
-	if (r->options->tok) {						\
+	if (nvmf_check_option(r, tok)) {				\
 		ret = __add_int_argument(argstr,			\
 					stringify(tok),			\
 					arg,				\
@@ -393,7 +397,7 @@ static int __add_argument(char **argstr, const char *tok, const char *arg)
 #define add_int_or_minus_one_argument(o, argstr, tok, arg)		\
 ({									\
 	int ret;							\
-	if (r->options->tok) {						\
+	if (nvmf_check_option(r, tok)) {				\
 		ret = __add_int_or_minus_one_argument(argstr,		\
 						     stringify(tok),	\
 						     arg);		\
@@ -409,7 +413,7 @@ static int __add_argument(char **argstr, const char *tok, const char *arg)
 #define add_argument(r, argstr, tok, arg)				\
 ({									\
 	int ret;							\
-	if (r->options->tok) {						\
+	if (nvmf_check_option(r, tok)) {				\
 		ret = __add_argument(argstr,				\
 				     stringify(tok),			\
 				     arg);				\
@@ -442,7 +446,6 @@ static int inet4_pton(const char *src, uint16_t port,
 static int inet6_pton(nvme_root_t r, const char *src, uint16_t port,
 		      struct sockaddr_storage *addr)
 {
-	int ret = -EINVAL;
 	struct sockaddr_in6 *addr6 = (struct sockaddr_in6 *)addr;
 	const char *scope = NULL;
 	char *p;
@@ -450,7 +453,7 @@ static int inet6_pton(nvme_root_t r, const char *src, uint16_t port,
 	if (strlen(src) > INET6_ADDRSTRLEN)
 		return -EINVAL;
 
-	char  *tmp = strdup(src);
+	_cleanup_free_ char *tmp = strdup(src);
 	if (!tmp) {
 		nvme_msg(r, LOG_ERR, "cannot copy: %s\n", src);
 		return -ENOMEM;
@@ -463,24 +466,20 @@ static int inet6_pton(nvme_root_t r, const char *src, uint16_t port,
 	}
 
 	if (inet_pton(AF_INET6, tmp, &addr6->sin6_addr) != 1)
-		goto free_tmp;
+		return -EINVAL;
 
 	if (IN6_IS_ADDR_LINKLOCAL(&addr6->sin6_addr) && scope) {
 		addr6->sin6_scope_id = if_nametoindex(scope);
 		if (addr6->sin6_scope_id == 0) {
 			nvme_msg(r, LOG_ERR,
 				 "can't find iface index for: %s (%m)\n", scope);
-			goto free_tmp;
+			return -EINVAL;
 		}
 	}
 
 	addr6->sin6_family = AF_INET6;
 	addr6->sin6_port = htons(port);
-	ret = 0;
-
-free_tmp:
-	free(tmp);
-	return ret;
+	return 0;
 }
 
 /**
@@ -655,7 +654,7 @@ static int build_options(nvme_host_t h, nvme_ctrl_t c, char **argstr)
 static  int __nvmf_supported_options(nvme_root_t r)
 {
 	char buf[0x1000], *options, *p, *v;
-	int fd, ret;
+	_cleanup_fd_ int fd = -1;
 	ssize_t len;
 
 	if (r->options)
@@ -684,14 +683,12 @@ static  int __nvmf_supported_options(nvme_root_t r)
 			         "Cannot read %s, using default options\n",
 			         nvmf_dev);
 			*r->options = default_supported_options;
-			ret = 0;
-			goto out_close;
+			return 0;
 		}
 
 		nvme_msg(r, LOG_ERR, "Failed to read from %s: %s\n",
 			 nvmf_dev, strerror(errno));
-		ret = -ENVME_CONNECT_READ;
-		goto out_close;
+		return -ENVME_CONNECT_READ;
 	}
 
 	buf[len] = '\0';
@@ -738,16 +735,13 @@ static  int __nvmf_supported_options(nvme_root_t r)
 		parse_option(r, v, trsvcid);
 	}
 	nvme_msg(r, LOG_DEBUG, "\n");
-	ret = 0;
-
-out_close:
-	close(fd);
-	return ret;
+	return 0;
 }
 
 static int __nvmf_add_ctrl(nvme_root_t r, const char *argstr)
 {
-	int ret, fd, len = strlen(argstr);
+	_cleanup_fd_ int fd;
+	int ret, len = strlen(argstr);
 	char buf[0x1000], *options, *p;
 
 	fd = open(nvmf_dev, O_RDWR);
@@ -765,31 +759,22 @@ static int __nvmf_add_ctrl(nvme_root_t r, const char *argstr)
 			 nvmf_dev, strerror(errno));
 		switch (errno) {
 		case EALREADY:
-			ret = -ENVME_CONNECT_ALREADY;
-			break;
+			return -ENVME_CONNECT_ALREADY;
 		case EINVAL:
-			ret = -ENVME_CONNECT_INVAL;
-			break;
+			return -ENVME_CONNECT_INVAL;
 		case EADDRINUSE:
-			ret = -ENVME_CONNECT_ADDRINUSE;
-			break;
+			return -ENVME_CONNECT_ADDRINUSE;
 		case ENODEV:
-			ret = -ENVME_CONNECT_NODEV;
-			break;
+			return -ENVME_CONNECT_NODEV;
 		case EOPNOTSUPP:
-			ret = -ENVME_CONNECT_OPNOTSUPP;
-			break;
+			return -ENVME_CONNECT_OPNOTSUPP;
 		case ECONNREFUSED:
-			ret = -ENVME_CONNECT_CONNREFUSED;
-			break;
+			return -ENVME_CONNECT_CONNREFUSED;
 		case EADDRNOTAVAIL:
-			ret = -ENVME_CONNECT_ADDRNOTAVAIL;
-			break;
+			return -ENVME_CONNECT_ADDRNOTAVAIL;
 		default:
-			ret = -ENVME_CONNECT_WRITE;
-			break;
+			return -ENVME_CONNECT_WRITE;
 		}
-		goto out_close;
 	}
 
 	memset(buf, 0x0, sizeof(buf));
@@ -797,8 +782,7 @@ static int __nvmf_add_ctrl(nvme_root_t r, const char *argstr)
 	if (len < 0) {
 		nvme_msg(r, LOG_ERR, "Failed to read from %s: %s\n",
 			 nvmf_dev, strerror(errno));
-		ret = -ENVME_CONNECT_READ;
-		goto out_close;
+		return -ENVME_CONNECT_READ;
 	}
 	nvme_msg(r, LOG_DEBUG, "connect ctrl, response '%.*s'\n",
 		 (int)strcspn(buf, "\n"), buf);
@@ -808,14 +792,33 @@ static int __nvmf_add_ctrl(nvme_root_t r, const char *argstr)
 		if (!*p)
 			continue;
 		if (sscanf(p, "instance=%d", &ret) == 1)
-			goto out_close;
+			return ret;
 	}
 
 	nvme_msg(r, LOG_ERR, "Failed to parse ctrl info for \"%s\"\n", argstr);
-	ret = -ENVME_CONNECT_PARSE;
-out_close:
-	close(fd);
-	return ret;
+	return -ENVME_CONNECT_PARSE;
+}
+
+static const char *lookup_context(nvme_root_t r, nvme_ctrl_t c)
+{
+
+	nvme_host_t h;
+	nvme_subsystem_t s;
+
+	nvme_for_each_host(r, h) {
+		nvme_for_each_subsystem(h, s) {
+			if (__nvme_lookup_ctrl(s, nvme_ctrl_get_transport(c),
+					       nvme_ctrl_get_traddr(c),
+					       NULL,
+					       NULL,
+					       nvme_ctrl_get_trsvcid(c),
+					       NULL,
+					       NULL))
+				return nvme_subsystem_get_application(s);
+		}
+	}
+
+	return NULL;
 }
 
 int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
@@ -823,7 +826,7 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
 {
 	nvme_subsystem_t s;
 	const char *root_app, *app;
-	char *argstr;
+	_cleanup_free_ char *argstr = NULL;
 	int ret;
 
 	/* highest prio have configs from command line */
@@ -839,6 +842,7 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
 					nvme_ctrl_get_host_traddr(c),
 					nvme_ctrl_get_host_iface(c),
 					nvme_ctrl_get_trsvcid(c),
+					NULL,
 					NULL);
 		if (fc) {
 			const char *key;
@@ -862,24 +866,9 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
 	root_app = nvme_root_get_application(h->r);
 	if (root_app) {
 		app = nvme_subsystem_get_application(s);
-		if (!app && nvme_ctrl_is_discovery_ctrl(c)) {
-			nvme_subsystem_t s;
-			nvme_ctrl_t fc;
-
-			nvme_for_each_subsystem(h, s) {
-				fc = __nvme_lookup_ctrl(s, nvme_ctrl_get_transport(c),
-							nvme_ctrl_get_traddr(c),
-							NULL,
-							NULL,
-							nvme_ctrl_get_trsvcid(c),
-							NULL);
-
-				if (fc) {
-					app = nvme_subsystem_get_application(s);
-					break;
-				}
-			}
-		}
+		if (!app && nvme_ctrl_is_discovery_ctrl(c))
+			app = lookup_context(h->r, c);
+
 		/*
 		 * configuration is managed by an application,
 		 * refuse to act on subsystems which either have
@@ -907,15 +896,11 @@ int nvmf_add_ctrl(nvme_host_t h, nvme_ctrl_t c,
 		free(traddr);
 	}
 
-	ret = __nvmf_supported_options(h->r);
-	if (ret)
-		return ret;
 	ret = build_options(h, c, &argstr);
 	if (ret)
 		return ret;
 
 	ret = __nvmf_add_ctrl(h->r, argstr);
-	free(argstr);
 	if (ret < 0) {
 		errno = -ret;
 		return -1;
@@ -1020,12 +1005,12 @@ nvme_ctrl_t nvmf_connect_disc_entry(nvme_host_t h,
 		return NULL;
 	}
 
-	if (e->treq & NVMF_TREQ_DISABLE_SQFLOW)
+	if (e->treq & NVMF_TREQ_DISABLE_SQFLOW &&
+	    nvmf_check_option(h->r, disable_sqflow))
 		c->cfg.disable_sqflow = true;
 
 	if (e->trtype == NVMF_TRTYPE_TCP &&
-	    (e->treq & NVMF_TREQ_REQUIRED ||
-	     e->treq & NVMF_TREQ_NOT_REQUIRED))
+	    e->tsas.tcp.sectype != NVMF_TCP_SECTYPE_NONE)
 		c->cfg.tls = true;
 
 	ret = nvmf_add_ctrl(h, c, cfg);
@@ -1046,45 +1031,55 @@ nvme_ctrl_t nvmf_connect_disc_entry(nvme_host_t h,
 	return NULL;
 }
 
-static struct nvmf_discovery_log *nvme_discovery_log(nvme_ctrl_t c,
-						     struct nvme_get_log_args *args,
-						     int max_retries)
+/*
+ * Most of nvmf_discovery_log is reserved, so only fetch the initial bytes.
+ * 8 bytes for GENCTR, 8 for NUMREC, and 2 for RECFMT.
+ * Since only multiples of 4 bytes are allowed, round 18 up to 20.
+ */
+#define DISCOVERY_HEADER_LEN 20
+
+static struct nvmf_discovery_log *nvme_discovery_log(
+	const struct nvme_get_discovery_args *args)
 {
-	nvme_root_t r = c->s && c->s->h ? c->s->h->r : NULL;
-	struct nvmf_discovery_log *log = NULL;
-	int ret, retries = 0;
-	const char *name = nvme_ctrl_get_name(c);
+	nvme_root_t r = root_from_ctrl(args->c);
+	struct nvmf_discovery_log *log;
+	int retries = 0;
+	const char *name = nvme_ctrl_get_name(args->c);
 	uint64_t genctr, numrec;
-	unsigned int size;
-	int fd = nvme_ctrl_get_fd(c);
-
-	args->fd = fd;
+	int fd = nvme_ctrl_get_fd(args->c);
+	struct nvme_get_log_args log_args = {
+		.result = args->result,
+		.args_size = sizeof(log_args),
+		.timeout = args->timeout,
+		.lid = NVME_LOG_LID_DISCOVER,
+		.nsid = NVME_NSID_NONE,
+		.csi = NVME_CSI_NVM,
+		.lsi = NVME_LOG_LSI_NONE,
+		.lsp = args->lsp,
+		.uuidx = NVME_UUID_NONE,
+	};
 
-	do {
-		size = sizeof(struct nvmf_discovery_log);
+	log = __nvme_alloc(sizeof(*log));
+	if (!log) {
+		nvme_msg(r, LOG_ERR,
+			 "could not allocate memory for discovery log header\n");
+		errno = ENOMEM;
+		return NULL;
+	}
 
-		free(log);
-		log = calloc(1, size);
-		if (!log) {
-			nvme_msg(r, LOG_ERR,
-				 "could not allocate memory for discovery log header\n");
-			errno = ENOMEM;
-			return NULL;
-		}
+	nvme_msg(r, LOG_DEBUG, "%s: get header (try %d/%d)\n",
+		 name, retries, args->max_retries);
+	log_args.log = log;
+	log_args.len = DISCOVERY_HEADER_LEN;
+	if (nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, &log_args)) {
+		nvme_msg(r, LOG_INFO,
+			 "%s: discover try %d/%d failed, error %d\n",
+			 name, retries, args->max_retries, errno);
+		goto out_free_log;
+	}
 
-		nvme_msg(r, LOG_DEBUG, "%s: get header (try %d/%d)\n",
-			name, retries, max_retries);
-		args->rae = true;
-		args->lpo = 0;
-		args->len = size;
-		args->log = log;
-		ret = nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, args);
-		if (ret) {
-			nvme_msg(r, LOG_INFO,
-				 "%s: discover try %d/%d failed, error %d\n",
-				 name, retries, max_retries, errno);
-			goto out_free_log;
-		}
+	do {
+		size_t entries_size;
 
 		numrec = le64_to_cpu(log->numrec);
 		genctr = le64_to_cpu(log->genctr);
@@ -1092,11 +1087,9 @@ static struct nvmf_discovery_log *nvme_discovery_log(nvme_ctrl_t c,
 		if (numrec == 0)
 			break;
 
-		size = sizeof(struct nvmf_discovery_log) +
-			sizeof(struct nvmf_disc_log_entry) * numrec;
-
 		free(log);
-		log = calloc(1, size);
+		entries_size = sizeof(*log->entries) * numrec;
+		log = __nvme_alloc(sizeof(*log) + entries_size);
 		if (!log) {
 			nvme_msg(r, LOG_ERR,
 				 "could not alloc memory for discovery log page\n");
@@ -1105,19 +1098,16 @@ static struct nvmf_discovery_log *nvme_discovery_log(nvme_ctrl_t c,
 		}
 
 		nvme_msg(r, LOG_DEBUG,
-			 "%s: get %" PRIu64
-			 " records (length %d genctr %" PRIu64 ")\n",
-			 name, numrec, size, genctr);
-
-		args->rae = true;
-		args->lpo = sizeof(struct nvmf_discovery_log);
-		args->len = size - sizeof(struct nvmf_discovery_log);
-		args->log = log->entries;
-		ret = nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, args);
-		if (ret) {
+			 "%s: get %" PRIu64 " records (genctr %" PRIu64 ")\n",
+			 name, numrec, genctr);
+
+		log_args.lpo = sizeof(*log);
+		log_args.log = log->entries;
+		log_args.len = entries_size;
+		if (nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, &log_args)) {
 			nvme_msg(r, LOG_INFO,
 				 "%s: discover try %d/%d failed, error %d\n",
-				 name, retries, max_retries, errno);
+				 name, retries, args->max_retries, errno);
 			goto out_free_log;
 		}
 
@@ -1127,19 +1117,17 @@ static struct nvmf_discovery_log *nvme_discovery_log(nvme_ctrl_t c,
 		 */
 		nvme_msg(r, LOG_DEBUG, "%s: get header again\n", name);
 
-		args->rae = false;
-		args->lpo = 0;
-		args->len = sizeof(struct nvmf_discovery_log);
-		args->log = log;
-		ret = nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, args);
-		if (ret) {
+		log_args.lpo = 0;
+		log_args.log = log;
+		log_args.len = DISCOVERY_HEADER_LEN;
+		if (nvme_get_log_page(fd, NVME_LOG_PAGE_PDU_SIZE, &log_args)) {
 			nvme_msg(r, LOG_INFO,
 				 "%s: discover try %d/%d failed, error %d\n",
-				 name, retries, max_retries, errno);
+				 name, retries, args->max_retries, errno);
 			goto out_free_log;
 		}
 	} while (genctr != le64_to_cpu(log->genctr) &&
-		 ++retries < max_retries);
+		 ++retries < args->max_retries);
 
 	if (genctr != le64_to_cpu(log->genctr)) {
 		nvme_msg(r, LOG_INFO, "%s: discover genctr mismatch\n", name);
@@ -1159,87 +1147,31 @@ out_free_log:
 	return NULL;
 }
 
-static void sanitize_discovery_log_entry(struct nvmf_disc_log_entry  *e)
+static void sanitize_discovery_log_entry(struct nvmf_disc_log_entry *e)
 {
-	switch (e->trtype) {
-	case NVMF_TRTYPE_RDMA:
-	case NVMF_TRTYPE_TCP:
-		switch (e->adrfam) {
-		case NVMF_ADDR_FAMILY_IP4:
-		case NVMF_ADDR_FAMILY_IP6:
-			strchomp(e->traddr, NVMF_TRADDR_SIZE);
-			strchomp(e->trsvcid, NVMF_TRSVCID_SIZE);
-			break;
-		}
-		break;
-        case NVMF_TRTYPE_FC:
-		switch (e->adrfam) {
-		case NVMF_ADDR_FAMILY_FC:
-			strchomp(e->traddr, NVMF_TRADDR_SIZE);
-			break;
-		}
-		break;
-	case NVMF_TRTYPE_LOOP:
-		strchomp(e->traddr, NVMF_TRADDR_SIZE);
-		break;
-	}
+	strchomp(e->trsvcid, sizeof(e->trsvcid));
+	strchomp(e->traddr, sizeof(e->traddr));
 }
 
 int nvmf_get_discovery_log(nvme_ctrl_t c, struct nvmf_discovery_log **logp,
 			   int max_retries)
 {
-	struct nvmf_discovery_log *log;
-
-	struct nvme_get_log_args args = {
-		.args_size = sizeof(args),
-		.fd = nvme_ctrl_get_fd(c),
-		.nsid = NVME_NSID_NONE,
-		.lsp = NVMF_LOG_DISC_LSP_NONE,
-		.lsi = NVME_LOG_LSI_NONE,
-		.uuidx = NVME_UUID_NONE,
+	struct nvme_get_discovery_args args = {
+		.c = c,
+		.max_retries = max_retries,
 		.timeout = NVME_DEFAULT_IOCTL_TIMEOUT,
-		.result = NULL,
-		.lid = NVME_LOG_LID_DISCOVER,
-		.log = NULL,
-		.len = 0,
-		.csi = NVME_CSI_NVM,
-		.rae = false,
-		.ot = false,
+		.lsp = NVMF_LOG_DISC_LSP_NONE,
 	};
 
-	log = nvme_discovery_log(c, &args, max_retries);
-	if (!log)
-		return -1;
-
-	for (int i = 0; i < le64_to_cpu(log->numrec); i++)
-		sanitize_discovery_log_entry(&log->entries[i]);
-
-	*logp = log;
-	return 0;
+	*logp = nvmf_get_discovery_wargs(&args);
+	return *logp ? 0 : -1;
 }
 
 struct nvmf_discovery_log *nvmf_get_discovery_wargs(struct nvme_get_discovery_args *args)
 {
 	struct nvmf_discovery_log *log;
 
-	struct nvme_get_log_args _args = {
-		.args_size = sizeof(_args),
-		.fd = nvme_ctrl_get_fd(args->c),
-		.nsid = NVME_NSID_NONE,
-		.lsp = args->lsp,
-		.lsi = NVME_LOG_LSI_NONE,
-		.uuidx = NVME_UUID_NONE,
-		.timeout = args->timeout,
-		.result = args->result,
-		.lid = NVME_LOG_LID_DISCOVER,
-		.log = NULL,
-		.len = 0,
-		.csi = NVME_CSI_NVM,
-		.rae = false,
-		.ot = false,
-	};
-
-	log = nvme_discovery_log(args->c, &_args, args->max_retries);
+	log = nvme_discovery_log(args);
 	if (!log)
 		return NULL;
 
@@ -1254,7 +1186,7 @@ struct nvmf_discovery_log *nvmf_get_discovery_wargs(struct nvme_get_discovery_ar
 static int uuid_from_device_tree(char *system_uuid)
 {
 	ssize_t len;
-	int f;
+	_cleanup_fd_ int f;
 
 	f = open(PATH_UUID_IBM, O_RDONLY);
 	if (f < 0)
@@ -1262,7 +1194,6 @@ static int uuid_from_device_tree(char *system_uuid)
 
 	memset(system_uuid, 0, NVME_UUID_LEN_STRING);
 	len = read(f, system_uuid, NVME_UUID_LEN_STRING - 1);
-	close(f);
 	if (len < 0)
 		return -ENXIO;
 
@@ -1299,7 +1230,7 @@ static bool is_dmi_uuid_valid(const char *buf, size_t len)
 static int uuid_from_dmi_entries(char *system_uuid)
 {
 	int f;
-	DIR *d;
+	_cleanup_dir_ DIR *d;
 	struct dirent *de;
 	char buf[512] = {0};
 
@@ -1350,7 +1281,6 @@ static int uuid_from_dmi_entries(char *system_uuid)
 			(uint8_t)buf[8 + 14], (uint8_t)buf[8 + 15]);
 		break;
 	}
-	closedir(d);
 	return strlen(system_uuid) ? 0 : -ENXIO;
 }
 
@@ -1364,10 +1294,9 @@ static int uuid_from_dmi_entries(char *system_uuid)
  */
 static int uuid_from_product_uuid(char *system_uuid)
 {
-	FILE *stream;
+	_cleanup_file_ FILE *stream;
 	ssize_t nread;
-	int ret;
-	char *line = NULL;
+	_cleanup_free_ char *line = NULL;
 	size_t len = 0;
 
 	stream = fopen(PATH_DMI_PROD_UUID, "re");
@@ -1376,10 +1305,8 @@ static int uuid_from_product_uuid(char *system_uuid)
 	system_uuid[0] = '\0';
 
 	nread = getline(&line, &len, stream);
-	if (nread != NVME_UUID_LEN_STRING) {
-		ret = -ENXIO;
-		goto out;
-	}
+	if (nread != NVME_UUID_LEN_STRING)
+		return -ENXIO;
 
 	/* The kernel is handling the byte swapping according DMTF
 	 * SMBIOS 3.0 Section 7.2.1 System UUID */
@@ -1387,13 +1314,7 @@ static int uuid_from_product_uuid(char *system_uuid)
 	memcpy(system_uuid, line, NVME_UUID_LEN_STRING - 1);
 	system_uuid[NVME_UUID_LEN_STRING - 1] = '\0';
 
-	ret = 0;
-
-out:
-	free(line);
-	fclose(stream);
-
-	return ret;
+	return 0;
 }
 
 /**
@@ -1443,7 +1364,8 @@ char *nvmf_hostnqn_generate()
 static char *nvmf_read_file(const char *f, int len)
 {
 	char buf[len];
-	int ret, fd;
+	_cleanup_fd_ int fd;
+	int ret;
 
 	fd = open(f, O_RDONLY);
 	if (fd < 0)
@@ -1451,7 +1373,6 @@ static char *nvmf_read_file(const char *f, int len)
 
 	memset(buf, 0, len);
 	ret = read(fd, buf, len - 1);
-	close (fd);
 
 	if (ret < 0 || !strlen(buf))
 		return NULL;
@@ -1575,7 +1496,7 @@ static int nvmf_dim(nvme_ctrl_t c, enum nvmf_dim_tas tas, __u8 trtype,
 		    __u32 *result)
 {
 	nvme_root_t r = c->s && c->s->h ? c->s->h->r : NULL;
-	struct nvmf_dim_data *dim;
+	_cleanup_free_ struct nvmf_dim_data *dim = NULL;
 	struct nvmf_ext_die  *die;
 	__u32 tdl;
 	__u32 tel;
@@ -1662,11 +1583,7 @@ static int nvmf_dim(nvme_ctrl_t c, enum nvmf_dim_tas tas, __u8 trtype,
 
 	args.data_len = tdl;
 	args.data = dim;
-	ret = nvme_dim_send(&args);
-
-	free(dim);
-
-	return ret;
+	return nvme_dim_send(&args);
 }
 
 /**
@@ -1720,25 +1637,31 @@ static const char *dctype_str[] = {
  */
 static int nvme_fetch_cntrltype_dctype_from_id(nvme_ctrl_t c)
 {
-	struct nvme_id_ctrl id = { 0 };
+	_cleanup_free_ struct nvme_id_ctrl *id;
 	int ret;
 
-	ret = nvme_ctrl_identify(c, &id);
+	id = __nvme_alloc(sizeof(*id));
+	if (!id) {
+		errno = ENOMEM;
+		return -1;
+	}
+
+	ret = nvme_ctrl_identify(c, id);
 	if (ret)
 		return ret;
 
 	if (!c->cntrltype) {
-		if (id.cntrltype > NVME_CTRL_CNTRLTYPE_ADMIN || !cntrltype_str[id.cntrltype])
+		if (id->cntrltype > NVME_CTRL_CNTRLTYPE_ADMIN || !cntrltype_str[id->cntrltype])
 			c->cntrltype = strdup("reserved");
 		else
-			c->cntrltype = strdup(cntrltype_str[id.cntrltype]);
+			c->cntrltype = strdup(cntrltype_str[id->cntrltype]);
 	}
 
-	if (!c->dctype)	{
-		if (id.dctype > NVME_CTRL_DCTYPE_CDC || !dctype_str[id.dctype])
+	if (!c->dctype) {
+		if (id->dctype > NVME_CTRL_DCTYPE_CDC || !dctype_str[id->dctype])
 			c->dctype = strdup("reserved");
 		else
-			c->dctype = strdup(dctype_str[id.dctype]);
+			c->dctype = strdup(dctype_str[id->dctype]);
 	}
 	return 0;
 }