From 4ff8602c62ed908a7b7d5f51e863ed0ab4de2659 Mon Sep 17 00:00:00 2001 From: Chris Leech 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, "", 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