Blob Blame Raw
From 4ff8602c62ed908a7b7d5f51e863ed0ab4de2659 Mon Sep 17 00:00:00 2001
From: Chris Leech <cleech@redhat.com>
Date: Tue, 16 Feb 2016 16:45:26 -0800
Subject: [PATCH] remove sysfs attr_list

The global cache is not well designed, it quickly can grow to the point
where lookups take much longer than just doing the sysfs read in the
first place.
---
 usr/host.c         |  1 +
 usr/session_info.c |  1 +
 usr/sysfs.c        | 62 ++++++++++++------------------------------------------
 3 files changed, 16 insertions(+), 48 deletions(-)

diff --git a/usr/host.c b/usr/host.c
index f2052d3..6333490 100644
--- a/usr/host.c
+++ b/usr/host.c
@@ -274,6 +274,7 @@ int host_info_print(int info_level, uint32_t host_no)
 			printf("iSCSI Transport Class version %s\n",
 			       version);
 			printf("version %s\n", ISCSI_VERSION_STR);
+			free(version);
 		}
 
 		flags |= SESSION_INFO_SCSI_DEVS;
diff --git a/usr/session_info.c b/usr/session_info.c
index 2f48e65..89422d8 100644
--- a/usr/session_info.c
+++ b/usr/session_info.c
@@ -390,6 +390,7 @@ int session_info_print(int info_level, struct session_info *info, int do_show)
 			printf("iSCSI Transport Class version %s\n",
 				version);
 			printf("version %s\n", ISCSI_VERSION_STR);
+			free(version);
 		}
 
 		flags |= (SESSION_INFO_SCSI_DEVS | SESSION_INFO_HOST_DEVS);
diff --git a/usr/sysfs.c b/usr/sysfs.c
index 6520bf6..efd4f74 100644
--- a/usr/sysfs.c
+++ b/usr/sysfs.c
@@ -63,15 +63,6 @@ char sysfs_path[PATH_SIZE];
 /* device cache */
 static LIST_HEAD(dev_list);
 
-/* attribute value cache */
-static LIST_HEAD(attr_list);
-
-struct sysfs_attr {
-	struct list_head node;
-	char path[PATH_SIZE];
-	char *value;			/* points to value_local if value is cached */
-	char value_local[NAME_SIZE];
-};
 int sysfs_init(void)
 {
 	const char *env;
@@ -85,22 +76,14 @@ int sysfs_init(void)
 	dbg("sysfs_path='%s'", sysfs_path);
 
 	INIT_LIST_HEAD(&dev_list);
-	INIT_LIST_HEAD(&attr_list);
 	return 0;
 }
 
 void sysfs_cleanup(void)
 {
-	struct sysfs_attr *attr_loop;
-	struct sysfs_attr *attr_temp;
 	struct sysfs_device *dev_loop;
 	struct sysfs_device *dev_temp;
 
-	list_for_each_entry_safe(attr_loop, attr_temp, &attr_list, node) {
-		list_del_init(&attr_loop->node);
-		free(attr_loop);
-	}
-
 	list_for_each_entry_safe(dev_loop, dev_temp, &dev_list, node) {
 		list_del_init(&dev_loop->node);
 		free(dev_loop);
@@ -355,10 +338,7 @@ struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device
 char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
 {
 	char path_full[PATH_SIZE];
-	const char *path;
 	char value[NAME_SIZE];
-	struct sysfs_attr *attr_loop;
-	struct sysfs_attr *attr;
 	struct stat statbuf;
 	int fd;
 	ssize_t size;
@@ -368,29 +348,10 @@ char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
 	sysfs_len = strlcpy(path_full, sysfs_path, sizeof(path_full));
 	if(sysfs_len >= sizeof(path_full))
 		sysfs_len = sizeof(path_full) - 1;
-	path = &path_full[sysfs_len];
 	strlcat(path_full, devpath, sizeof(path_full));
 	strlcat(path_full, "/", sizeof(path_full));
 	strlcat(path_full, attr_name, sizeof(path_full));
 
-	/* look for attribute in cache */
-	list_for_each_entry(attr_loop, &attr_list, node) {
-		if (strcmp(attr_loop->path, path) == 0) {
-			dbg("found in cache '%s'", attr_loop->path);
-			return attr_loop->value;
-		}
-	}
-
-	/* store attribute in cache (also negatives are kept in cache) */
-	dbg("new uncached attribute '%s'", path_full);
-	attr = malloc(sizeof(struct sysfs_attr));
-	if (attr == NULL)
-		return NULL;
-	memset(attr, 0x00, sizeof(struct sysfs_attr));
-	strlcpy(attr->path, path, sizeof(attr->path));
-	dbg("add to cache '%s'", path_full);
-	list_add(&attr->node, &attr_list);
-
 	if (lstat(path_full, &statbuf) != 0) {
 		dbg("stat '%s' failed: %s", path_full, strerror(errno));
 		goto out;
@@ -408,8 +369,7 @@ char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
 			pos = strrchr(link_target, '/');
 			if (pos != NULL) {
 				dbg("cache '%s' with link value '%s'", path_full, value);
-				strlcpy(attr->value_local, &pos[1], sizeof(attr->value_local));
-				attr->value = attr->value_local;
+				strlcpy(value, &pos[1], NAME_SIZE);
 			}
 		}
 		goto out;
@@ -439,12 +399,8 @@ char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
 	/* got a valid value, store and return it */
 	value[size] = '\0';
 	remove_trailing_chars(value, '\n');
-	dbg("cache '%s' with attribute value '%s'", path_full, value);
-	strlcpy(attr->value_local, value, sizeof(attr->value_local));
-	attr->value = attr->value_local;
-
 out:
-	return attr->value;
+	return strdup(value);
 }
 
 int sysfs_lookup_devpath_by_subsys_id(char *devpath_full, size_t len, const char *subsystem, const char *id)
@@ -567,8 +523,10 @@ char *sysfs_get_value(const char *id, char *subsys, char *param)
 	}
 
 	if (!strncmp(sysfs_value, "<NULL>", 6) ||
-	    !strncmp(sysfs_value, "(null)", 6))
+	    !strncmp(sysfs_value, "(null)", 6)) {
+		free(sysfs_value);
 		return NULL;
+	}
 
 	return sysfs_value;
 }
@@ -585,6 +543,7 @@ int sysfs_get_uint(char *id, char *subsys, char *param,
 
 	errno = 0;
 	*value = strtoul(sysfs_value, NULL, 0);
+	free(sysfs_value);
 	if (errno)
 		return errno;
 	return 0;
@@ -600,6 +559,7 @@ int sysfs_get_int(const char *id, char *subsys, char *param, int *value)
 		return EIO;
 
 	*value = atoi(sysfs_value);
+	free(sysfs_value);
 	return 0;
 }
 
@@ -619,6 +579,7 @@ int sysfs_get_str(char *id, char *subsys, char *param, char *value,
 		sysfs_value[len - 1] = '\0';
 	strncpy(value, sysfs_value, value_size);
 	value[value_size - 1] = '\0';
+	free(sysfs_value);
 	return 0;
 }
 
@@ -631,8 +592,11 @@ int sysfs_get_uint64(char *id, char *subsys, char *param, uint64_t *value)
 	if (!sysfs_value)
 		return EIO;
 
-	if (sscanf(sysfs_value, "%" PRIu64 "\n", value) != 1)
+	if (sscanf(sysfs_value, "%" PRIu64 "\n", value) != 1) {
+		free(sysfs_value);
 		return EINVAL;
+	}
+	free(sysfs_value);
 	return 0;
 }
 
@@ -647,6 +611,7 @@ int sysfs_get_uint8(char *id, char *subsys, char *param,
 		return EIO;
 
 	*value = (uint8_t)atoi(sysfs_value);
+	free(sysfs_value);
 	return 0;
 }
 
@@ -661,6 +626,7 @@ int sysfs_get_uint16(char *id, char *subsys, char *param,
 		return EIO;
 
 	*value = (uint16_t)atoi(sysfs_value);
+	free(sysfs_value);
 	return 0;
 }
 
-- 
2.5.0