Refactor/replace code for more portability (part 1)

libnvme/src/meson.build

@@ -5,7 +5,9 @@
 #
 # Authors: Martin Belanger <Martin.Belanger@dell.com>
 #
-sources = []
+sources = [
+    'nvme/log.c',
+]
 if host_system == 'windows'
     sources += [
         'nvme/mem-win.c',
@@ -19,7 +21,6 @@ else
         'nvme/ioctl.c',
         'nvme/lib.c',
         'nvme/linux.c',
-        'nvme/log.c',
         'nvme/mem-linux.c',
         'nvme/nvme-cmds.c',
         'nvme/sysfs.c',

libnvme/src/nvme/linux.c

@@ -1836,23 +1836,30 @@ static int uuid_from_dmi_entries(char *system_uuid)
 static int uuid_from_product_uuid(char *system_uuid)
 {
 	__cleanup_file FILE *stream = NULL;
-	ssize_t nread;
-	__cleanup_free char *line = NULL;
-	size_t len = 0;
 
 	stream = fopen(PATH_DMI_PROD_UUID, "re");
 	if (!stream)
 		return -ENXIO;
-	system_uuid[0] = '\0';
 
-	nread = getline(&line, &len, stream);
-	if (nread != NVME_UUID_LEN_STRING)
-		return -ENXIO;
+	system_uuid[0] = '\0';
 
 	/* The kernel is handling the byte swapping according DMTF
 	 * SMBIOS 3.0 Section 7.2.1 System UUID */
 
-	memcpy(system_uuid, line, NVME_UUID_LEN_STRING - 1);
+	/*
+	 * Expect exactly:
+	 * xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx
+	 */
+	if (!fgets(system_uuid, NVME_UUID_LEN_STRING, stream))
+		return -ENXIO;
+
+	if (strlen(system_uuid) != NVME_UUID_LEN_STRING - 1)
+		return -ENXIO;
+
+	if (system_uuid[8]  != '-' || system_uuid[13] != '-' ||
+	    system_uuid[18] != '-' || system_uuid[23] != '-')
+		return -ENXIO;
+
 	system_uuid[NVME_UUID_LEN_STRING - 1] = '\0';
 
 	return 0;

libnvme/src/nvme/log.c

@@ -8,9 +8,11 @@
  * This file implements basic logging functionality.
  */
 
+#include <errno.h>
 #include <stdarg.h>
 #include <stdbool.h>
 #include <stdio.h>
+#include <string.h>
 #include <time.h>
 #include <unistd.h>
 
@@ -25,6 +27,36 @@
 #define LOG_CLOCK CLOCK_MONOTONIC
 #endif
 
+static ssize_t write_all(int fd, const void *buf, size_t count)
+{
+	const char *p = buf;
+	size_t total = 0;
+
+	while (total < count) {
+		ssize_t n = write(fd, p + total, count - total);
+
+		if (n > 0) {
+			total += n;
+			continue;
+		}
+
+		if (n < 0) {
+			if (errno == EINTR)
+				continue;
+
+			if (errno == EAGAIN)
+				continue;
+
+			return -1;
+		}
+
+		errno = EIO;
+		return -1;
+	}
+
+	return total;
+}
+
 void __attribute__((format(printf, 4, 5)))
 __libnvme_msg(struct libnvme_global_ctx *ctx, int level,
 	   const char *func, const char *format, ...)
@@ -45,6 +77,7 @@ __libnvme_msg(struct libnvme_global_ctx *ctx, int level,
 	};
 	__cleanup_free char *header = NULL;
 	__cleanup_free char *message = NULL;
+	__cleanup_free char *log = NULL;
 	int idx = 0;
 
 	if (level > l->level)
@@ -78,9 +111,12 @@ __libnvme_msg(struct libnvme_global_ctx *ctx, int level,
 		message = NULL;
 	va_end(ap);
 
-	dprintf(l->fd, "%s%s",
-		header ? header : "<error>",
-		message ? message : "<error>");
+	if (asprintf(&log, "%s%s", header ? header : "<error>",
+			message ? message : "<error>") == -1)
+		return;
+
+	if (write_all(l->fd, log, strlen(log)) < 0)
+		perror("failed to write log entry");
 }
 
 __libnvme_public void libnvme_set_logging_level(

nvme-models.c

@@ -18,6 +18,8 @@ static char *_fmt3 = "/sys/class/nvme/nvme%d/device/vendor";
 static char *_fmt4 = "/sys/class/nvme/nvme%d/device/device";
 static char *_fmt5 = "/sys/class/nvme/nvme%d/device/class";
 
+#define LINE_BUF_SIZE 1024
+
 static char fmt1[78];
 static char fmt2[78];
 static char fmt3[78];
@@ -84,23 +86,23 @@ static void format_and_print(char *save)
 
 	if (!class_mid) {
 		if (device_final)
-			snprintf(save, 1024, "%s %s %s",
+			snprintf(save, LINE_BUF_SIZE, "%s %s %s",
 				 locate_info(device_top, false, false),
 				 locate_info(device_mid, false, false),
 				 locate_info(device_final, true, false));
 		else
-			snprintf(save, 1024, "%s %s",
+			snprintf(save, LINE_BUF_SIZE, "%s %s",
 				 locate_info(device_top, false, false),
 				 locate_info(device_mid, false, false));
 	} else {
 		if (device_final)
-			snprintf(save, 1024, "%s: %s %s %s",
+			snprintf(save, LINE_BUF_SIZE, "%s: %s %s %s",
 				 locate_info(class_mid, false, true),
 				 locate_info(device_top, false, false),
 				 locate_info(device_mid, false, false),
 				 locate_info(device_final, true, false));
 		else
-			snprintf(save, 1024, "%s: %s %s",
+			snprintf(save, LINE_BUF_SIZE, "%s: %s %s",
 				 locate_info(class_mid, false, true),
 				 locate_info(device_top, false, false),
 				 locate_info(device_mid, false, false));
@@ -113,18 +115,18 @@ static void format_all(char *save, char *vendor, char *device)
 		format_and_print(save);
 
 	else if (device_top && !device_mid && class_mid)
-		snprintf(save, 1024, "%s: %s Device %s",
+		snprintf(save, LINE_BUF_SIZE, "%s: %s Device %s",
 			 locate_info(class_mid, false, true),
 			 locate_info(device_top, false, false),
 			 device);
 
 	else if (!device_top && class_mid)
-		snprintf(save, 1024, "%s: Vendor %s Device %s",
+		snprintf(save, LINE_BUF_SIZE, "%s: Vendor %s Device %s",
 			 locate_info(class_mid, false, true),
 			 vendor,
 			 device);
 	else
-		snprintf(save, 1024, "Unknown device");
+		snprintf(save, LINE_BUF_SIZE, "Unknown device");
 }
 
 static int is_final_match(char *line, char *search)
@@ -179,61 +181,59 @@ static inline int is_class_info(char *line)
 	return !memcmp(line, "# C class", 9);
 }
 
-static void parse_vendor_device(char **line, FILE *file,
+static void parse_vendor_device(char *line, FILE *file,
 			       char *device, char *subdev,
 			       char *subven)
 {
 	bool device_single_found = false;
-	size_t amnt = 1024;
-	size_t found = 0;
-	char *newline;
+	size_t len;
 
-	while ((found = getline(line, &amnt, file)) != -1) {
-		newline = *line;
-		if (is_comment(newline))
+	while (fgets(line, LINE_BUF_SIZE, file) != NULL) {
+		len = strlen(line);
+		if (len > 0 && line[len - 1] == '\n')
+			line[len - 1] = '\0';
+		if (is_comment(line))
 			continue;
-		if (!is_tab(newline))
+		if (!is_tab(line))
 			return;
 
-		newline[found - 1] = '\0';
-		if (!device_single_found && is_mid_level_match(newline, device, false)) {
+		if (!device_single_found && is_mid_level_match(line, device, false)) {
 			device_single_found = true;
-			device_mid = strdup(newline);
+			device_mid = strdup(line);
 			continue;
 		}
 
-		if (device_single_found && is_inner_sub_vendev(newline, subven, subdev)) {
-			device_final = strdup(newline);
+		if (device_single_found && is_inner_sub_vendev(line, subven, subdev)) {
+			device_final = strdup(line);
 			break;
 		}
 	}
 }
 
-static void pull_class_info(char **_newline, FILE *file, char *class)
+static void pull_class_info(char *line, FILE *file, char *class)
 {
-	size_t amnt;
-	size_t size = 1024;
 	bool top_found = false;
 	bool mid_found = false;
-	char *newline;
-
-	while ((amnt = getline(_newline, &size, file)) != -1) {
-		newline = *_newline;
-		newline[amnt - 1] = '\0';
-		if (!top_found && is_top_level_match(newline, class, true)) {
-			class_top = strdup(newline);
+	size_t len;
+
+	while (fgets(line, LINE_BUF_SIZE, file) != NULL) {
+		len = strlen(line);
+		if (len > 0 && line[len - 1] == '\n')
+			line[len - 1] = '\0';
+		if (!top_found && is_top_level_match(line, class, true)) {
+			class_top = strdup(line);
 			top_found = true;
 			continue;
 		}
 		if (!mid_found && top_found &&
-		    is_mid_level_match(newline,  &class[4], true)) {
-			class_mid = strdup(newline);
+		    is_mid_level_match(line, &class[4], true)) {
+			class_mid = strdup(line);
 			mid_found = true;
 			continue;
 		}
 		if (top_found && mid_found &&
-		    is_final_match(newline, &class[6])) {
-			class_final = strdup(newline);
+		    is_final_match(line, &class[6])) {
+			class_final = strdup(line);
 			break;
 		}
 	}
@@ -297,17 +297,17 @@ static FILE *open_pci_ids(void)
 	return NULL;
 }
 
-char *nvme_product_name(int id)
+static char *__nvme_product_name(int id)
 {
-	char *line = NULL;
-	ssize_t amnt;
+	char readbuf[LINE_BUF_SIZE];
 	char vendor[7] = { 0 };
 	char device[7] = { 0 };
 	char sub_device[7] = { 0 };
 	char sub_vendor[7] = { 0 };
 	char class[13] = { 0 };
-	size_t size = 1024;
-	char ret;
+	size_t len;
+	int ret = 0;
+	char *result;
 	FILE *file = open_pci_ids();
 
 	if (!file)
@@ -327,33 +327,57 @@ char *nvme_product_name(int id)
 	if (ret)
 		goto error0;
 
-	line = malloc(1024);
-	if (!line) {
-		fprintf(stderr, "malloc: %s\n", libnvme_strerror(errno));
-		goto error0;
-	}
-
-	while ((amnt = getline(&line, &size, file)) != -1) {
-		if (is_comment(line) && !is_class_info(line))
+	while (fgets(readbuf, sizeof(readbuf), file) != NULL) {
+		len = strlen(readbuf);
+		if (len > 0 && readbuf[len - 1] == '\n')
+			readbuf[len - 1] = '\0';
+		if (is_comment(readbuf) && !is_class_info(readbuf))
 			continue;
-		if (is_top_level_match(line, vendor, false)) {
-			line[amnt - 1] = '\0';
+		if (is_top_level_match(readbuf, vendor, false)) {
 			free(device_top);
-			device_top = strdup(line);
-			parse_vendor_device(&line, file,
+			device_top = strdup(readbuf);
+			parse_vendor_device(readbuf, file,
 						device,
 						sub_device,
 						sub_vendor);
 		}
-		if (is_class_info(line))
-			pull_class_info(&line, file, class);
+		if (is_class_info(readbuf))
+			pull_class_info(readbuf, file, class);
 	}
 	fclose(file);
-	format_all(line, vendor, device);
+
+	result = malloc(LINE_BUF_SIZE);
+	if (!result) {
+		fprintf(stderr, "malloc: %s\n", libnvme_strerror(errno));
+		free_all();
+		return NULL;
+	}
+	format_all(result, vendor, device);
 	free_all();
-	return line;
+	return result;
 error0:
 	fclose(file);
 error1:
-	return strdup("NULL");
+	return NULL;
+}
+
+char *nvme_product_name(const char *devname)
+{
+	const char *base;
+	int id;
+
+	if (!devname)
+		return NULL;
+
+	base = strrchr(devname, '/');
+	if (base) {
+		if (!base[1])
+			return NULL;
+		devname = base + 1;
+	}
+
+	if (sscanf(devname, "nvme%d", &id) != 1)
+		return NULL;
+
+	return __nvme_product_name(id);
 }

nvme-models.h

@@ -2,6 +2,6 @@
 #ifndef NVME_MODEL_H
 #define NVME_MODEL_H
 
-char *nvme_product_name(int id);
+char *nvme_product_name(const char *devname);
 
 #endif

nvme-print-binary.c

@@ -137,7 +137,7 @@ static void binary_id_ns_descs(void *data, unsigned nsid)
 	d_raw((unsigned char *)data, 0x1000);
 }
 
-static void binary_id_ctrl(struct nvme_id_ctrl *ctrl,
+static void binary_id_ctrl(struct nvme_id_ctrl *ctrl, const char *product_name,
 	void (*vendor_show)(__u8 *vs, struct json_object *root))
 {
 	d_raw((unsigned char *)ctrl, sizeof(*ctrl));