sysfs: minimize heap allocations of sysfs paths

11a0918 ("nvme: allow to overwrite base sysfs path")
added support for changing the sysfs path via an environment variable.
Unfortunately, it added a heap allocation
every time a sysfs path was requested.

Modify the callers to not free the paths, which allows a string constant
to be returned if the path isn't overridden, avoiding an allocation.
Cache the path in a static variable so that if it is overridden,
the heap-allocated string only needs to be constructed once
and afterwards can be reused.

Create a file sysfs.c to consolidate this logic
instead of spreading them across 3 files.
Also introduce a helper to factor out the duplicated code.

Fixes: 11a0918 ("nvme: allow to overwrite base sysfs path")
Signed-off-by: Caleb Sander Mateos <[email protected]>

Author: calebsander

src/meson.build

@@ -12,6 +12,7 @@ sources = [
     'nvme/ioctl.c',
     'nvme/linux.c',
     'nvme/log.c',
+    'nvme/sysfs.c',
     'nvme/tree.c',
     'nvme/util.c',
     'nvme/base64.c',

src/nvme/fabrics.c

@@ -1186,29 +1186,12 @@ struct nvmf_discovery_log *nvmf_get_discovery_wargs(struct nvme_get_discovery_ar
 	return log;
 }
 
-#define PATH_UUID_IBM	"/proc/device-tree/ibm,partition-uuid"
-
-static char *uuid_ibm_filename(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_UUID_IBM);
-
-	if (!asprintf(&str, "%s" PATH_UUID_IBM, basepath))
-		return NULL;
-
-	return str;
-}
-
 static int uuid_from_device_tree(char *system_uuid)
 {
-	_cleanup_free_ char *filename = uuid_ibm_filename();
 	_cleanup_fd_ int f = -1;
 	ssize_t len;
 
-	f = open(filename, O_RDONLY);
+	f = open(nvme_uuid_ibm_filename(), O_RDONLY);
 	if (f < 0)
 		return -ENXIO;
 
@@ -1220,22 +1203,6 @@ static int uuid_from_device_tree(char *system_uuid)
 	return strlen(system_uuid) ? 0 : -ENXIO;
 }
 
-#define PATH_DMI_ENTRIES       "/sys/firmware/dmi/entries"
-
-static char *dmi_entries_dir(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_DMI_ENTRIES);
-
-	if (!asprintf(&str, "%s" PATH_DMI_ENTRIES, basepath))
-		return NULL;
-
-	return str;
-}
-
 /*
  * See System Management BIOS (SMBIOS) Reference Specification
  * https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.2.0.pdf
@@ -1264,7 +1231,7 @@ static bool is_dmi_uuid_valid(const char *buf, size_t len)
 static int uuid_from_dmi_entries(char *system_uuid)
 {
 	_cleanup_dir_ DIR *d = NULL;
-	_cleanup_free_ char *entries_dir = dmi_entries_dir();
+	const char *entries_dir = nvme_dmi_entries_dir();
 	int f;
 	struct dirent *de;
 	char buf[512] = {0};

src/nvme/filters.c

@@ -6,68 +6,12 @@
  * Authors: Keith Busch <[email protected]>
  * 	    Chaitanya Kulkarni <[email protected]>
  */
-#include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
 #include <dirent.h>
-#include <libgen.h>
-
-#include <sys/param.h>
-#include <sys/stat.h>
-#include <sys/types.h>
-#include <fcntl.h>
-#include <unistd.h>
 
 #include "filters.h"
-#include "types.h"
-#include "util.h"
-#include "cleanup.h"
-
-#define PATH_SYSFS_NVME			"/sys/class/nvme"
-#define PATH_SYSFS_NVME_SUBSYSTEM	"/sys/class/nvme-subsystem"
-#define PATH_SYSFS_BLOCK		"/sys/block"
-
-char *nvme_ctrl_sysfs_dir(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_SYSFS_NVME);
-
-	if (!asprintf(&str, "%s" PATH_SYSFS_NVME, basepath))
-		return NULL;
-
-	return str;
-}
-
-char *nvme_ns_sysfs_dir(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_SYSFS_BLOCK);
-
-	if (!asprintf(&str, "%s" PATH_SYSFS_BLOCK, basepath))
-		return NULL;
-
-	return str;
-}
-
-char *nvme_subsys_sysfs_dir(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_SYSFS_NVME_SUBSYSTEM);
-
-	if (!asprintf(&str, "%s" PATH_SYSFS_NVME_SUBSYSTEM, basepath))
-		return NULL;
-
-	return str;
-}
+#include "private.h"
 
 int nvme_namespace_filter(const struct dirent *d)
 {
@@ -132,7 +76,7 @@ int nvme_subsys_filter(const struct dirent *d)
 
 int nvme_scan_subsystems(struct dirent ***subsys)
 {
-	_cleanup_free_ char *dir = nvme_subsys_sysfs_dir();
+	const char *dir = nvme_subsys_sysfs_dir();
 
 	return scandir(dir, subsys, nvme_subsys_filter, alphasort);
 }
@@ -145,7 +89,7 @@ int nvme_scan_subsystem_namespaces(nvme_subsystem_t s, struct dirent ***ns)
 
 int nvme_scan_ctrls(struct dirent ***ctrls)
 {
-	_cleanup_free_ char *dir = nvme_ctrl_sysfs_dir();
+	const char *dir = nvme_ctrl_sysfs_dir();
 
 	return scandir(dir, ctrls, nvme_ctrls_filter, alphasort);
 }

src/nvme/private.h

@@ -17,9 +17,12 @@
 #include "mi.h"
 
 
-char *nvme_ctrl_sysfs_dir(void);
-char *nvme_subsys_sysfs_dir(void);
-char *nvme_ns_sysfs_dir(void);
+const char *nvme_subsys_sysfs_dir(void);
+const char *nvme_ctrl_sysfs_dir(void);
+const char *nvme_ns_sysfs_dir(void);
+const char *nvme_slots_sysfs_dir(void);
+const char *nvme_uuid_ibm_filename(void);
+const char *nvme_dmi_entries_dir(void);
 
 struct nvme_path {
 	struct list_node entry;

src/nvme/sysfs.c

@@ -0,0 +1,86 @@
+// SPDX-License-Identifier: LGPL-2.1-or-later
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "private.h"
+
+#define PATH_UUID_IBM			"/proc/device-tree/ibm,partition-uuid"
+#define PATH_SYSFS_BLOCK		"/sys/block"
+#define PATH_SYSFS_SLOTS		"/sys/bus/pci/slots"
+#define PATH_SYSFS_NVME_SUBSYSTEM	"/sys/class/nvme-subsystem"
+#define PATH_SYSFS_NVME			"/sys/class/nvme"
+#define PATH_DMI_ENTRIES		"/sys/firmware/dmi/entries"
+
+static const char *make_sysfs_dir(const char *path)
+{
+	char *basepath = getenv("LIBNVME_SYSFS_PATH");
+	char *str;
+
+	if (!basepath)
+		return path;
+
+	if (asprintf(&str, "%s%s", basepath, path) < 0)
+		return NULL;
+
+	return str;
+}
+
+const char *nvme_subsys_sysfs_dir(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_SYSFS_NVME_SUBSYSTEM);
+}
+
+const char *nvme_ctrl_sysfs_dir(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_SYSFS_NVME);
+}
+
+const char *nvme_ns_sysfs_dir(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_SYSFS_BLOCK);
+}
+
+const char *nvme_slots_sysfs_dir(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_SYSFS_SLOTS);
+}
+
+const char *nvme_uuid_ibm_filename(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_UUID_IBM);
+}
+
+const char *nvme_dmi_entries_dir(void)
+{
+	static const char *str;
+
+	if (str)
+		return str;
+
+	return str = make_sysfs_dir(PATH_DMI_ENTRIES);
+}

src/nvme/tree.c

@@ -61,22 +61,6 @@ struct candidate_args {
 };
 typedef bool (*ctrl_match_t)(struct nvme_ctrl *c, struct candidate_args *candidate);
 
-#define PATH_SYSFS_SLOTS "/sys/bus/pci/slots"
-
-static char *nvme_slots_sysfs_dir(void)
-{
-	char *basepath = getenv("LIBNVME_SYSFS_PATH");
-	char *str;
-
-	if (!basepath)
-		return strdup(PATH_SYSFS_SLOTS);
-
-	if (!asprintf(&str, "%s" PATH_SYSFS_SLOTS, basepath))
-		return NULL;
-
-	return str;
-}
-
 static struct nvme_host *default_host;
 
 static void __nvme_free_host(nvme_host_t h);
@@ -672,10 +656,9 @@ static int nvme_subsystem_scan_namespaces(nvme_root_t r, nvme_subsystem_t s,
 
 static int nvme_init_subsystem(nvme_subsystem_t s, const char *name)
 {
-	_cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
 	char *path;
 
-	if (asprintf(&path, "%s/%s", subsys_dir, name) < 0)
+	if (asprintf(&path, "%s/%s", nvme_subsys_sysfs_dir(), name) < 0)
 		return -1;
 
 	s->model = nvme_get_attr(path, "model");
@@ -716,12 +699,11 @@ static int nvme_scan_subsystem(struct nvme_root *r, const char *name,
 {
 	struct nvme_subsystem *s = NULL, *_s;
 	_cleanup_free_ char *path = NULL, *subsysnqn = NULL;
-	_cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
 	nvme_host_t h = NULL;
 	int ret;
 
 	nvme_msg(r, LOG_DEBUG, "scan subsystem %s\n", name);
-	ret = asprintf(&path, "%s/%s", subsys_dir, name);
+	ret = asprintf(&path, "%s/%s", nvme_subsys_sysfs_dir(), name);
 	if (ret < 0)
 		return ret;
 
@@ -1703,7 +1685,7 @@ static int nvme_ctrl_scan_namespaces(nvme_root_t r, struct nvme_ctrl *c)
 static char *nvme_ctrl_lookup_subsystem_name(nvme_root_t r,
 					     const char *ctrl_name)
 {
-	_cleanup_free_ char *subsys_dir = nvme_subsys_sysfs_dir();
+	const char *subsys_dir = nvme_subsys_sysfs_dir();
 	_cleanup_dirents_ struct dirents subsys = {};
 	int i;
 
@@ -1730,7 +1712,7 @@ static char *nvme_ctrl_lookup_subsystem_name(nvme_root_t r,
 
 static char *nvme_ctrl_lookup_phy_slot(nvme_root_t r, const char *address)
 {
-	_cleanup_free_ char *slots_sysfs_dir = nvme_slots_sysfs_dir();
+	const char *slots_sysfs_dir = nvme_slots_sysfs_dir();
 	_cleanup_free_ char *target_addr = NULL;
 	_cleanup_dir_ DIR *slots_dir = NULL;
 	int ret;
@@ -1827,7 +1809,6 @@ static int nvme_configure_ctrl(nvme_root_t r, nvme_ctrl_t c, const char *path,
 
 int nvme_init_ctrl(nvme_host_t h, nvme_ctrl_t c, int instance)
 {
-	_cleanup_free_ char *ctrl_dir = nvme_ctrl_sysfs_dir();
 	_cleanup_free_ char *subsys_name = NULL;
 	_cleanup_free_ char *name = NULL;
 	nvme_subsystem_t s;
@@ -1839,7 +1820,7 @@ int nvme_init_ctrl(nvme_host_t h, nvme_ctrl_t c, int instance)
 		errno = ENOMEM;
 		return -1;
 	}
-	ret = asprintf(&path, "%s/nvme%d", ctrl_dir, instance);
+	ret = asprintf(&path, "%s/%s", nvme_ctrl_sysfs_dir(), name);
 	if (ret < 0) {
 		errno = ENOMEM;
 		return ret;
@@ -1983,11 +1964,10 @@ nvme_ctrl_t nvme_scan_ctrl(nvme_root_t r, const char *name)
 	_cleanup_free_ char *path = NULL;
 	_cleanup_free_ char *hostnqn = NULL, *hostid = NULL;
 	_cleanup_free_ char *subsysnqn = NULL, *subsysname = NULL;
-	_cleanup_free_ char *ctrl_dir = nvme_ctrl_sysfs_dir();
 	int ret;
 
 	nvme_msg(r, LOG_DEBUG, "scan controller %s\n", name);
-	ret = asprintf(&path, "%s/%s", ctrl_dir, name);
+	ret = asprintf(&path, "%s/%s", nvme_ctrl_sysfs_dir(), name);
 	if (ret < 0) {
 		errno = ENOMEM;
 		return NULL;
@@ -2615,9 +2595,7 @@ static struct nvme_ns *__nvme_scan_namespace(const char *sysfs_dir, const char *
 
 nvme_ns_t nvme_scan_namespace(const char *name)
 {
-	_cleanup_free_ char *ns_dir = nvme_ns_sysfs_dir();
-
-	return __nvme_scan_namespace(ns_dir, name);
+	return __nvme_scan_namespace(nvme_ns_sysfs_dir(), name);
 }
 
 static int nvme_ctrl_scan_namespace(nvme_root_t r, struct nvme_ctrl *c,