This avoids lifetime issues with the returned string, for subsequent changes
to caching in sysfs. All callers are changed accordingly. Many callers
copied the result to a buffer already, so this actually simplifies some code.
diff --git a/extras/usb_id/usb_id.c b/extras/usb_id/usb_id.c
index 92492c8..fd4d2f2 100644
--- a/extras/usb_id/usb_id.c
+++ b/extras/usb_id/usb_id.c
@@ -224,9 +224,11 @@ static int usb_id(const char *devpath)
struct sysfs_device *dev;
struct sysfs_device *dev_interface;
struct sysfs_device *dev_usb;
- const char *if_class, *if_subclass;
+ char if_class[NAME_SIZE];
+ char if_subclass[NAME_SIZE];
int if_class_num;
int protocol = 0;
+ int status;
dbg("devpath %s\n", devpath);
@@ -244,15 +246,17 @@ static int usb_id(const char *devpath)
return 1;
}
- if_class = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceClass");
- if (!if_class) {
+ status = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceClass",
+ if_class, sizeof(if_class));
+ if (!status) {
info("%s: cannot get bInterfaceClass attribute\n", dev_interface->kernel);
return 1;
}
if_class_num = strtoul(if_class, NULL, 16);
if (if_class_num == 8) {
- if_subclass = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceSubClass");
- if (if_subclass != NULL)
+ status = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceSubClass",
+ if_subclass, sizeof(if_subclass));
+ if (!status)
protocol = set_usb_mass_storage_ifsubtype(type_str, if_subclass, sizeof(type_str)-1);
} else
set_usb_iftype(type_str, if_class_num, sizeof(type_str)-1);
@@ -269,7 +273,10 @@ static int usb_id(const char *devpath)
/* mass storage */
if (protocol == 6 && !use_usb_info) {
struct sysfs_device *dev_scsi;
- const char *scsi_model, *scsi_vendor, *scsi_type, *scsi_rev;
+ char scsi_model[NAME_SIZE];
+ char scsi_vendor[NAME_SIZE];
+ char scsi_type[NAME_SIZE];
+ char scsi_rev[NAME_SIZE];
int host, bus, target, lun;
/* get scsi device */
@@ -284,29 +291,33 @@ static int usb_id(const char *devpath)
}
/* Generic SPC-2 device */
- scsi_vendor = sysfs_attr_get_value(dev_scsi->devpath, "vendor");
- if (!scsi_vendor) {
+ status = sysfs_attr_get_value(dev_scsi->devpath, "vendor",
+ scsi_vendor, sizeof(scsi_vendor));
+ if (!status) {
info("%s: cannot get SCSI vendor attribute\n", dev_scsi->kernel);
goto fallback;
}
set_str(vendor_str, scsi_vendor, sizeof(vendor_str)-1);
- scsi_model = sysfs_attr_get_value(dev_scsi->devpath, "model");
- if (!scsi_model) {
+ status = sysfs_attr_get_value(dev_scsi->devpath, "model",
+ scsi_model, sizeof(scsi_model));
+ if (!status) {
info("%s: cannot get SCSI model attribute\n", dev_scsi->kernel);
goto fallback;
}
set_str(model_str, scsi_model, sizeof(model_str)-1);
- scsi_type = sysfs_attr_get_value(dev_scsi->devpath, "type");
- if (!scsi_type) {
+ status = sysfs_attr_get_value(dev_scsi->devpath, "type",
+ scsi_type, sizeof(scsi_type));
+ if (!status) {
info("%s: cannot get SCSI type attribute\n", dev_scsi->kernel);
goto fallback;
}
set_scsi_type(type_str, scsi_type, sizeof(type_str)-1);
- scsi_rev = sysfs_attr_get_value(dev_scsi->devpath, "rev");
- if (!scsi_rev) {
+ status = sysfs_attr_get_value(dev_scsi->devpath, "rev",
+ scsi_rev, sizeof(scsi_rev));
+ if (!status) {
info("%s: cannot get SCSI revision attribute\n", dev_scsi->kernel);
goto fallback;
}
@@ -322,15 +333,18 @@ static int usb_id(const char *devpath)
fallback:
/* fallback to USB vendor & device */
if (vendor_str[0] == '\0') {
- const char *usb_vendor = NULL;
+ char usb_vendor[NAME_SIZE];
+ status = 0;
if (!use_num_info)
- usb_vendor = sysfs_attr_get_value(dev_usb->devpath, "manufacturer");
+ status = sysfs_attr_get_value(dev_usb->devpath, "manufacturer",
+ usb_vendor, sizeof(usb_vendor));
- if (!usb_vendor)
- usb_vendor = sysfs_attr_get_value(dev_usb->devpath, "idVendor");
+ if (!status)
+ status = sysfs_attr_get_value(dev_usb->devpath, "idVendor",
+ usb_vendor, sizeof(usb_vendor));
- if (!usb_vendor) {
+ if (!status) {
info("No USB vendor information available\n");
return 1;
}
@@ -338,15 +352,18 @@ fallback:
}
if (model_str[0] == '\0') {
- const char *usb_model = NULL;
+ char usb_model[NAME_SIZE];
+ status = 0;
if (!use_num_info)
- usb_model = sysfs_attr_get_value(dev_usb->devpath, "product");
+ status = sysfs_attr_get_value(dev_usb->devpath, "product",
+ usb_model, sizeof(usb_model));
- if (!usb_model)
- usb_model = sysfs_attr_get_value(dev_usb->devpath, "idProduct");
+ if (!status)
+ status = sysfs_attr_get_value(dev_usb->devpath, "idProduct",
+ usb_model, sizeof(usb_model));
- if (!usb_model) {
+ if (!status) {
dbg("No USB model information available\n");
return 1;
}
@@ -354,18 +371,20 @@ fallback:
}
if (revision_str[0] == '\0') {
- const char *usb_rev;
+ char usb_rev[NAME_SIZE];
- usb_rev = sysfs_attr_get_value(dev_usb->devpath, "bcdDevice");
- if (usb_rev)
+ status = sysfs_attr_get_value(dev_usb->devpath, "bcdDevice",
+ usb_rev, sizeof(usb_rev));
+ if (status)
set_str(revision_str, usb_rev, sizeof(revision_str)-1);
}
if (serial_str[0] == '\0') {
- const char *usb_serial;
+ char usb_serial[NAME_SIZE];
- usb_serial = sysfs_attr_get_value(dev_usb->devpath, "serial");
- if (usb_serial)
+ status = sysfs_attr_get_value(dev_usb->devpath, "serial",
+ usb_serial, sizeof(usb_serial));
+ if (status)
set_str(serial_str, usb_serial, sizeof(serial_str)-1);
}
return 0;
diff --git a/udev/udev.h b/udev/udev.h
index da86365..28b59db 100644
--- a/udev/udev.h
+++ b/udev/udev.h
@@ -117,7 +117,7 @@ extern void sysfs_device_set_values(struct sysfs_device *dev, const char *devpat
extern struct sysfs_device *sysfs_device_get(const char *devpath);
extern struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev);
extern struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem);
-extern char *sysfs_attr_get_value(const char *devpath, const char *attr_name);
+extern int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value, size_t len);
extern int sysfs_resolve_link(char *path, size_t size);
extern int sysfs_lookup_devpath_by_subsys_id(char *devpath, size_t len, const char *subsystem, const char *id);
diff --git a/udev/udev_device.c b/udev/udev_device.c
index 130c714..13000b0 100644
--- a/udev/udev_device.c
+++ b/udev/udev_device.c
@@ -71,12 +71,11 @@ void udev_device_cleanup(struct udevice *udev)
dev_t udev_device_get_devt(struct udevice *udev)
{
- const char *attr;
+ char attr[NAME_SIZE];
unsigned int maj, min;
/* read it from sysfs */
- attr = sysfs_attr_get_value(udev->dev->devpath, "dev");
- if (attr != NULL) {
+ if (sysfs_attr_get_value(udev->dev->devpath, "dev", attr, sizeof(attr))) {
if (sscanf(attr, "%u:%u", &maj, &min) == 2)
return makedev(maj, min);
}
diff --git a/udev/udev_node.c b/udev/udev_node.c
index 33183c8..a29c4b7 100644
--- a/udev/udev_node.c
+++ b/udev/udev_node.c
@@ -376,12 +376,11 @@ int udev_node_add(struct udevice *udev)
/* create all_partitions if requested */
if (udev->partitions) {
char partitionname[PATH_SIZE];
- char *attr;
+ char attr[NAME_SIZE];
int range;
/* take the maximum registered minor range */
- attr = sysfs_attr_get_value(udev->dev->devpath, "range");
- if (attr != NULL) {
+ if (sysfs_attr_get_value(udev->dev->devpath, "range", attr, sizeof(attr))) {
range = atoi(attr);
if (range > 1)
udev->partitions = range-1;
diff --git a/udev/udev_rules.c b/udev/udev_rules.c
index f7acd9c..53c483a 100644
--- a/udev/udev_rules.c
+++ b/udev/udev_rules.c
@@ -829,40 +829,40 @@ found:
else {
char devpath[PATH_SIZE];
char *attrib;
- const char *value = NULL;
size_t size;
+ int found = 0;
if (attr_get_by_subsys_id(attr, devpath, sizeof(devpath), &attrib)) {
if (attrib != NULL)
- value = sysfs_attr_get_value(devpath, attrib);
+ found = sysfs_attr_get_value(devpath, attrib,
+ temp2, sizeof(temp2));
else
break;
}
/* try the current device, other matches may have selected */
- if (value == NULL && udev->dev_parent != NULL && udev->dev_parent != udev->dev)
- value = sysfs_attr_get_value(udev->dev_parent->devpath, attr);
-
+ if (!found && udev->dev_parent != NULL && udev->dev_parent != udev->dev)
+ found = sysfs_attr_get_value(udev->dev_parent->devpath, attr,
+ temp2, sizeof(temp2));
/* look at all devices along the chain of parents */
- if (value == NULL) {
+ if (!found) {
struct sysfs_device *dev_parent = udev->dev;
do {
dbg("looking at '%s'\n", dev_parent->devpath);
- value = sysfs_attr_get_value(dev_parent->devpath, attr);
- if (value != NULL)
+ found = sysfs_attr_get_value(dev_parent->devpath, attr,
+ temp2, sizeof(temp2));
+ if (found)
break;
dev_parent = sysfs_device_get_parent(dev_parent);
} while (dev_parent != NULL);
}
- if (value == NULL)
+ if (!found)
break;
/* strip trailing whitespace, and replace unwanted characters */
- size = strlcpy(temp2, value, sizeof(temp2));
- if (size >= sizeof(temp2))
- size = sizeof(temp2)-1;
+ size = strlen(temp2);
while (size > 0 && isspace(temp2[size-1]))
temp2[--size] = '\0';
count = replace_chars(temp2, ALLOWED_CHARS_INPUT);
@@ -1136,21 +1136,22 @@ static int match_rule(struct udevice *udev, struct udev_rule *rule)
const char *key_value = key_val(rule, &pair->key);
char devpath[PATH_SIZE];
char *attrib;
- const char *value = NULL;
char val[VALUE_SIZE];
size_t len;
+ int found = 0;
if (attr_get_by_subsys_id(key_name, devpath, sizeof(devpath), &attrib)) {
if (attrib != NULL)
- value = sysfs_attr_get_value(devpath, attrib);
+ found = sysfs_attr_get_value(devpath, attrib,
+ val, sizeof(val));
else
goto nomatch;
}
- if (value == NULL)
- value = sysfs_attr_get_value(udev->dev->devpath, key_name);
- if (value == NULL)
+ if (!found)
+ found = sysfs_attr_get_value(udev->dev->devpath, key_name,
+ val, sizeof(val));
+ if (!found)
goto nomatch;
- strlcpy(val, value, sizeof(val));
/* strip trailing whitespace of value, if not asked to match for it */
len = strlen(key_value);
@@ -1189,16 +1190,17 @@ static int match_rule(struct udevice *udev, struct udev_rule *rule)
pair->key.operation == KEY_OP_NOMATCH) {
const char *key_name = key_pair_name(rule, pair);
const char *key_value = key_val(rule, &pair->key);
- const char *value;
char val[VALUE_SIZE];
size_t len;
-
- value = sysfs_attr_get_value(udev->dev_parent->devpath, key_name);
- if (value == NULL)
- value = sysfs_attr_get_value(udev->dev->devpath, key_name);
- if (value == NULL)
+ int found;
+
+ found = sysfs_attr_get_value(udev->dev_parent->devpath, key_name,
+ val, sizeof(val));
+ if (!found)
+ found = sysfs_attr_get_value(udev->dev->devpath, key_name,
+ val, sizeof(val));
+ if (!found)
goto try_parent;
- strlcpy(val, value, sizeof(val));
/* strip trailing whitespace of value, if not asked to match for it */
len = strlen(key_value);
diff --git a/udev/udev_sysfs.c b/udev/udev_sysfs.c
index 91f5997..2d2457f 100644
--- a/udev/udev_sysfs.c
+++ b/udev/udev_sysfs.c
@@ -174,11 +174,11 @@ struct sysfs_device *sysfs_device_get(const char *devpath)
/* look for device already in cache (we never put an untranslated path in the cache) */
list_for_each_entry(dev_loop, &dev_list, node) {
- if (strcmp(dev_loop->devpath, devpath_real) == 0) {
- dbg("found in cache '%s'\n", dev_loop->devpath);
- return dev_loop;
+ if (strcmp(dev_loop->devpath, devpath_real) == 0) {
+ dbg("found in cache '%s'\n", dev_loop->devpath);
+ return dev_loop;
+ }
}
- }
/* if we got a link, resolve it to the real device */
strlcpy(path, sysfs_path, sizeof(path));
@@ -193,12 +193,12 @@ struct sysfs_device *sysfs_device_get(const char *devpath)
/* now look for device in cache after path translation */
list_for_each_entry(dev_loop, &dev_list, node) {
- if (strcmp(dev_loop->devpath, devpath_real) == 0) {
- dbg("found in cache '%s'\n", dev_loop->devpath);
- return dev_loop;
+ if (strcmp(dev_loop->devpath, devpath_real) == 0) {
+ dbg("found in cache '%s'\n", dev_loop->devpath);
+ return dev_loop;
+ }
}
}
- }
/* it is a new device */
dbg("new uncached device '%s'\n", devpath_real);
@@ -323,11 +323,11 @@ struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device
return NULL;
}
-char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
+int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value, size_t len)
{
char path_full[PATH_SIZE];
const char *path;
- char value[NAME_SIZE];
+ char val[NAME_SIZE];
struct sysfs_attr *attr_loop;
struct sysfs_attr *attr;
struct stat statbuf;
@@ -346,76 +346,85 @@ char *sysfs_attr_get_value(const char *devpath, const char *attr_name)
/* 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'\n", attr_loop->path);
- return attr_loop->value;
+ if (strcmp(attr_loop->path, path) == 0) {
+ dbg("found in cache '%s'\n", attr_loop->path);
+ if (!attr_loop->value)
+ return 0;
+
+ attr = attr_loop;
+ goto out;
+ }
}
- }
- /* store attribute in cache (also negatives are kept in cache) */
- dbg("new uncached attribute '%s'\n", 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'\n", path_full);
+ /* store attribute in cache (also negatives are kept in cache) */
+ dbg("new uncached attribute '%s'\n", path_full);
+ attr = malloc(sizeof(struct sysfs_attr));
+ if (attr == NULL)
+ return 0;
+ memset(attr, 0x00, sizeof(struct sysfs_attr));
+ strlcpy(attr->path, path, sizeof(attr->path));
+ dbg("add to cache '%s'\n", path_full);
list_add(&attr->node, &attr_list);
if (lstat(path_full, &statbuf) != 0) {
dbg("stat '%s' failed: %s\n", path_full, strerror(errno));
- goto out;
+ return 0;
}
if (S_ISLNK(statbuf.st_mode)) {
/* links return the last element of the target path */
char link_target[PATH_SIZE];
- int len;
const char *pos;
- len = readlink(path_full, link_target, sizeof(link_target));
- if (len > 0) {
- link_target[len] = '\0';
- pos = strrchr(link_target, '/');
- if (pos != NULL) {
- dbg("cache '%s' with link value '%s'\n", path_full, value);
- strlcpy(attr->value_local, &pos[1], sizeof(attr->value_local));
- attr->value = attr->value_local;
- }
- }
+ size = readlink(path_full, link_target, sizeof(link_target));
+ if (size < 0)
+ return 0;
+ if (size == sizeof(link_target))
+ return 0;
+
+ link_target[size] = '\0';
+ pos = strrchr(link_target, '/');
+ if (pos == NULL)
+ return 0;
+
+ strlcpy(val, &pos[1], sizeof(val));
goto out;
}
/* skip directories */
if (S_ISDIR(statbuf.st_mode))
- goto out;
+ return 0;
/* skip non-readable files */
if ((statbuf.st_mode & S_IRUSR) == 0)
- goto out;
+ return 0;
/* read attribute value */
fd = open(path_full, O_RDONLY);
if (fd < 0) {
dbg("attribute '%s' can not be opened\n", path_full);
- goto out;
+ return 0;
}
- size = read(fd, value, sizeof(value));
+ size = read(fd, val, sizeof(val));
close(fd);
if (size < 0)
- goto out;
- if (size == sizeof(value))
- goto out;
+ return 0;
+ if (size == sizeof(val))
+ return 0;
/* got a valid value, store and return it */
- value[size] = '\0';
- remove_trailing_chars(value, '\n');
- dbg("cache '%s' with attribute value '%s'\n", path_full, value);
- strlcpy(attr->value_local, value, sizeof(attr->value_local));
- attr->value = attr->value_local;
+ val[size] = '\0';
+ remove_trailing_chars(val, '\n');
out:
- return attr->value;
+ strlcpy(attr->value_local, val, sizeof(attr->value_local));
+ attr->value = attr->value_local;
+ dbg("cache '%s' with link value '%s'\n", path_full, attr->value);
+
+ if (strlcpy(value, val, len) > len-1)
+ return 0;
+
+ return 1;
}
int sysfs_lookup_devpath_by_subsys_id(char *devpath_full, size_t len, const char *subsystem, const char *id)
diff --git a/udev/udevinfo.c b/udev/udevinfo.c
index 9b75364..b76d11d 100644
--- a/udev/udevinfo.c
+++ b/udev/udevinfo.c
@@ -47,7 +47,6 @@ static void print_all_attributes(const char *devpath, const char *key)
for (dent = readdir(dir); dent != NULL; dent = readdir(dir)) {
struct stat statbuf;
char filename[PATH_SIZE];
- char *attr_value;
char value[NAME_SIZE];
size_t len;
@@ -67,12 +66,9 @@ static void print_all_attributes(const char *devpath, const char *key)
if (S_ISLNK(statbuf.st_mode))
continue;
- attr_value = sysfs_attr_get_value(devpath, dent->d_name);
- if (attr_value == NULL)
+ if (!sysfs_attr_get_value(devpath, dent->d_name, value, sizeof(value)))
continue;
- len = strlcpy(value, attr_value, sizeof(value));
- if(len >= sizeof(value))
- len = sizeof(value) - 1;
+ len = strlen(value);
dbg("attr '%s'='%s'(%zi)\n", dent->d_name, value, len);
/* skip nonprintable attributes */
--
To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
