gmane.linux.hotplug.devel

Re: [PATCH] threadsafe rules iteration

Alan Jenkins — 2008-09-05T15:08:55

Sorry, don't apply that. It should be struct udev_rules_iter iter; I think we should be building with -Werror, but I'm not sure how to tell autoconf/automake. udev_rules.c: In function ‘udev_rules_get_run’: udev_rules.c:1571: warning: passing argument 1 of ‘udev_rules_iter_init’ from incompatible pointer type udev_rules.c:1573: warning: passing argument 1 of ‘udev_rules_iter_next’ from incompatible pointer type udev_rules.c:1624: warning: passing argument 1 of ‘udev_rules_iter_label’ from incompatible pointer type -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH] fix bug in name_list_key_add()

Alan Jenkins — 2008-09-05T13:49:34

The search for an existing key e.g. FOO would match longer keys e.g. FOOBAR. Reuse the correct code from name_list_remove(). diff --git a/udev/udev_utils.c b/udev/udev_utils.c index f63a9fa..9b6ba24 100644 --- a/udev/udev_utils.c +++ b/udev/udev_utils.c < at >< at > -89,14 +89,17 < at >< at > struct name_entry *name_list_key_add(struct list_head *name_list, const char *ke { struct name_entry *name_loop; struct name_entry *name_new; +size_t keylen = strlen(key); list_for_each_entry(name_loop, name_list, node) { -if (strncmp(name_loop->name, key, strlen(key)) == 0) { -dbg("key already present '%s', replace it\n", name_loop->name); -snprintf(name_loop->name, sizeof(name_loop->name), "%s=%s", key, value); -name_loop->name[sizeof(name_loop->name)-1] = '\0'; -return name_loop; -} +if (strncmp(name_loop->name, key, keylen) != 0) +continue; +if (name_loop->name[keylen] != '=') +continue; +dbg("key already present '%s', replace it\n", name_loop->name); +snprintf(name_loop->name, sizeof(name_loop->name), "%s=%s", key, value); +name_loop->name[sizeof(name_loop->name)-1] = '\0'; +return name_loop; } name_new = malloc(sizeof(struct name_entry)); -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH] threadsafe rules iteration

Alan Jenkins — 2008-09-05T11:11:56

Move ->current out of struct udev_rules and into a new struct udev_rules_iter. Signed-off-by: Alan Jenkins tuffmail.co.uk> diff --git a/udev/udev_rules.c b/udev/udev_rules.c index da7a62a..c21d762 100644 --- a/udev/udev_rules.c +++ b/udev/udev_rules.c < at >< at > -1378,6 +1378,7 < at >< at > nomatch: int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev) { +struct udev_rules_iter iter; struct udev_rule *rule; int name_set = 0; < at >< at > -1385,9 +1386,9 < at >< at > int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev) dbg("udev->dev->kernel='%s'\n", udev->dev->kernel); /* look for a matching rule to apply */ -udev_rules_iter_init(rules); +udev_rules_iter_init(&iter, rules); while (1) { -rule = udev_rules_iter_next(rules); +rule = udev_rules_iter_next(&iter); if (rule == NULL) break; < at >< at > -1540,7 +1541,7 < at >< at > int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev) if (rule->goto_label.operation != KEY_OP_UNSET) { dbg("moving forward to label '%s'\n", key_val(rule, &rule->goto_label)); -udev_rules_iter_label(rules, key_val(rule, &rule->goto_label)); +udev_rules_iter_label(&iter, key_val(rule, &rule->goto_label)); } } } < at >< at > -1561,14 +1562,15 < at >< at > int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev) int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev) { +struct udev_rules_iter *iter; struct udev_rule *rule; dbg("udev->kernel='%s'\n", udev->dev->kernel); /* look for a matching rule to apply */ -udev_rules_iter_init(rules); +udev_rules_iter_init(&iter, rules); while (1) { -rule = udev_rules_iter_next(rules); +rule = udev_rules_iter_next(&iter); if (rule == NULL) break; < at >< at > -1619,7 +1621,7 < at >< at > int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev) if (rule->goto_label.operation != KEY_OP_UNSET) { dbg("moving forward to label '%s'\n", key_val(rule, &rule->goto_label)); -udev_rules_iter_label(rules, key_val(rule, &rule->goto_label)); +udev_rules_iter_label(&iter, key_val(rule, &rule->goto_label)); } } } diff --git a/udev/udev_rules.h b/udev/udev_rules.h index 9a41ccb..fd82f5d 100644 --- a/udev/udev_rules.h +++ b/udev/udev_rules.h < at >< at > -110,16 +110,20 < at >< at > struct udev_rule { struct udev_rules { char *buf; size_t bufsize; -size_t current; int resolve_names; }; +struct udev_rules_iter { +struct udev_rules *rules; +size_t current; +}; + extern int udev_rules_init(struct udev_rules *rules, int resolve_names); extern void udev_rules_cleanup(struct udev_rules *rules); -extern void udev_rules_iter_init(struct udev_rules *rules); -extern struct udev_rule *udev_rules_iter_next(struct udev_rules *rules); -extern struct udev_rule *udev_rules_iter_label(struct udev_rules *rules, const char *label); +extern void udev_rules_iter_init(struct udev_rules_iter *iter, struct udev_rules *rules); +extern struct udev_rule *udev_rules_iter_next(struct udev_rules_iter *iter); +extern struct udev_rule *udev_rules_iter_label(struct udev_rules_iter *iter, const char *label); extern int udev_rules_get_name(struct udev_rules *rules, struct udevice *udev); extern int udev_rules_get_run(struct udev_rules *rules, struct udevice *udev); diff --git a/udev/udev_rules_parse.c b/udev/udev_rules_parse.c index 90b139b..a9a483c 100644 --- a/udev/udev_rules_parse.c +++ b/udev/udev_rules_parse.c < at >< at > -33,49 +33,53 < at >< at > #include "udev_selinux.h" -void udev_rules_iter_init(struct udev_rules *rules) +void udev_rules_iter_init(struct udev_rules_iter *iter, struct udev_rules *rules) { dbg("bufsize=%zi\n", rules->bufsize); -rules->current = 0; +iter->rules = rules; +iter->current = 0; } -struct udev_rule *udev_rules_iter_next(struct udev_rules *rules) +struct udev_rule *udev_rules_iter_next(struct udev_rules_iter *iter) { +struct udev_rules *rules; struct udev_rule *rule; +rules = iter->rules; if (!rules) return NULL; -dbg("current=%zi\n", rules->current); -if (rules->current >= rules->bufsize) { +dbg("current=%zi\n", iter->current); +if (iter->current >= rules->bufsize) { dbg("no more rules\n"); return NULL; } /* get next rule */ -rule = (struct udev_rule *) (rules->buf + rules->current); -rules->current += sizeof(struct udev_rule) + rule->bufsize; +rule = (struct udev_rule *) (rules->buf + iter->current); +iter->current += sizeof(struct udev_rule) + rule->bufsize; return rule; } -struct udev_rule *udev_rules_iter_label(struct udev_rules *rules, const char *label) +struct udev_rule *udev_rules_iter_label(struct udev_rules_iter *iter, const char *label) { struct udev_rule *rule; -size_t start = rules->current; - +struct udev_rules *rules = iter->rules; +size_t start = iter->current; + next: -dbg("current=%zi\n", rules->current); -if (rules->current >= rules->bufsize) { +dbg("current=%zi\n", iter->current); +if (iter->current >= rules->bufsize) { err("LABEL='%s' not found, GOTO will be ignored\n", label); -rules->current = start; +iter->current = start; return NULL; } -rule = (struct udev_rule *) (rules->buf + rules->current); +rule = (struct udev_rule *) (rules->buf + iter->current); if (strcmp(&rule->buf[rule->label.val_off], label) != 0) { dbg("moving forward, looking for label '%s'\n", label); -rules->current += sizeof(struct udev_rule) + rule->bufsize; +iter->current += sizeof(struct udev_rule) + rule->bufsize; goto next; } -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 3/3] sysfs: add explicit sysfs_cache object

Alan Jenkins — 2008-09-04T14:22:54

The implicit caching in sysfs is unsuitable for long-lived programs; it would fill up with stale data. Fortunately libudev doesn't use the relevant functions, but this is not very clear. Add an explicit sysfs_cache parameter to the relevant functions (sysfs_attr_get_value, sysfs_device_get and variants). The parameter can be NULL for no caching. sysfs_device objects returned by these functions must now be explicitly released by calling sysfs_device_cleanup(). This makes traversing parent devices quite hairy, so we hide the details in udev_device_parent_iter_*. diff --git a/extras/usb_id/usb_id.c b/extras/usb_id/usb_id.c index fd4d2f2..f8e38f0 100644 --- a/extras/usb_id/usb_id.c +++ b/extras/usb_id/usb_id.c < at >< at > -228,33 +228,34 < at >< at > static int usb_id(const char *devpath) char if_subclass[NAME_SIZE]; int if_class_num; int protocol = 0; +int retval = 1; int status; dbg("devpath %s\n", devpath); /* get all usb specific information: dev_interface, if_class, dev_usb */ -dev = sysfs_device_get(devpath); +dev = sysfs_device_get(NULL, devpath); if (dev == NULL) { err("unable to access '%s'\n", devpath); -return 1; +goto out; } /* usb interface directory */ -dev_interface = sysfs_device_get_parent_with_subsystem(dev, "usb"); +dev_interface = sysfs_device_get_parent_with_subsystem(NULL, dev, "usb"); if (dev_interface == NULL) { info("unable to access usb_interface device of '%s'\n", devpath); -return 1; +goto out; } -status = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceClass", +status = sysfs_attr_get_value(NULL, dev_interface->devpath, "bInterfaceClass", if_class, sizeof(if_class)); if (!status) { info("%s: cannot get bInterfaceClass attribute\n", dev_interface->kernel); -return 1; +goto out; } if_class_num = strtoul(if_class, NULL, 16); if (if_class_num == 8) { -status = sysfs_attr_get_value(dev_interface->devpath, "bInterfaceSubClass", +status = sysfs_attr_get_value(NULL, 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); < at >< at > -264,10 +265,10 < at >< at > static int usb_id(const char *devpath) info("%s: if_class %d protocol %d\n", dev_interface->devpath, if_class_num, protocol); /* usb device directory */ -dev_usb = sysfs_device_get_parent_with_subsystem(dev_interface, "usb"); +dev_usb = sysfs_device_get_parent_with_subsystem(NULL, dev_interface, "usb"); if (!dev_usb) { info("unable to find parent 'usb' device of '%s'\n", devpath); -return 1; +goto out; } /* mass storage */ < at >< at > -280,7 +281,7 < at >< at > static int usb_id(const char *devpath) int host, bus, target, lun; /* get scsi device */ -dev_scsi = sysfs_device_get_parent_with_subsystem(dev, "scsi"); +dev_scsi = sysfs_device_get_parent_with_subsystem(NULL, dev, "scsi"); if (dev_scsi == NULL) { info("unable to find parent 'scsi' device of '%s'\n", devpath); goto fallback; < at >< at > -291,7 +292,7 < at >< at > static int usb_id(const char *devpath) } /* Generic SPC-2 device */ -status = sysfs_attr_get_value(dev_scsi->devpath, "vendor", +status = sysfs_attr_get_value(NULL, dev_scsi->devpath, "vendor", scsi_vendor, sizeof(scsi_vendor)); if (!status) { info("%s: cannot get SCSI vendor attribute\n", dev_scsi->kernel); < at >< at > -299,7 +300,7 < at >< at > static int usb_id(const char *devpath) } set_str(vendor_str, scsi_vendor, sizeof(vendor_str)-1); -status = sysfs_attr_get_value(dev_scsi->devpath, "model", +status = sysfs_attr_get_value(NULL, dev_scsi->devpath, "model", scsi_model, sizeof(scsi_model)); if (!status) { info("%s: cannot get SCSI model attribute\n", dev_scsi->kernel); < at >< at > -307,7 +308,7 < at >< at > static int usb_id(const char *devpath) } set_str(model_str, scsi_model, sizeof(model_str)-1); -status = sysfs_attr_get_value(dev_scsi->devpath, "type", +status = sysfs_attr_get_value(NULL, dev_scsi->devpath, "type", scsi_type, sizeof(scsi_type)); if (!status) { info("%s: cannot get SCSI type attribute\n", dev_scsi->kernel); < at >< at > -315,7 +316,7 < at >< at > static int usb_id(const char *devpath) } set_scsi_type(type_str, scsi_type, sizeof(type_str)-1); -status = sysfs_attr_get_value(dev_scsi->devpath, "rev", +status = sysfs_attr_get_value(NULL, dev_scsi->devpath, "rev", scsi_rev, sizeof(scsi_rev)); if (!status) { info("%s: cannot get SCSI revision attribute\n", dev_scsi->kernel); < at >< at > -337,16 +338,16 < at >< at > fallback: status = 0; if (!use_num_info) -status = sysfs_attr_get_value(dev_usb->devpath, "manufacturer", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "manufacturer", usb_vendor, sizeof(usb_vendor)); if (!status) -status = sysfs_attr_get_value(dev_usb->devpath, "idVendor", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "idVendor", usb_vendor, sizeof(usb_vendor)); if (!status) { info("No USB vendor information available\n"); -return 1; +goto out; } set_str(vendor_str, usb_vendor, sizeof(vendor_str)-1); } < at >< at > -356,16 +357,16 < at >< at > fallback: status = 0; if (!use_num_info) -status = sysfs_attr_get_value(dev_usb->devpath, "product", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "product", usb_model, sizeof(usb_model)); if (!status) -status = sysfs_attr_get_value(dev_usb->devpath, "idProduct", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "idProduct", usb_model, sizeof(usb_model)); if (!status) { dbg("No USB model information available\n"); -return 1; +goto out; } set_str(model_str, usb_model, sizeof(model_str)-1); } < at >< at > -373,7 +374,7 < at >< at > fallback: if (revision_str[0] == '\0') { char usb_rev[NAME_SIZE]; -status = sysfs_attr_get_value(dev_usb->devpath, "bcdDevice", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "bcdDevice", usb_rev, sizeof(usb_rev)); if (status) set_str(revision_str, usb_rev, sizeof(revision_str)-1); < at >< at > -382,12 +383,20 < at >< at > fallback: if (serial_str[0] == '\0') { char usb_serial[NAME_SIZE]; -status = sysfs_attr_get_value(dev_usb->devpath, "serial", +status = sysfs_attr_get_value(NULL, dev_usb->devpath, "serial", usb_serial, sizeof(usb_serial)); if (status) set_str(serial_str, usb_serial, sizeof(serial_str)-1); } -return 0; + +/* success */ +retval = 0; + +out: +sysfs_device_cleanup(dev); +sysfs_device_cleanup(dev_interface); +sysfs_device_cleanup(dev_usb); +return retval; } int main(int argc, char **argv) diff --git a/udev/test-udev.c b/udev/test-udev.c index 07628f7..878c477 100644 --- a/udev/test-udev.c +++ b/udev/test-udev.c < at >< at > -62,7 +62,6 < at >< at > static void asmlinkage sig_handler(int signum) int main(int argc, char *argv[]) { -struct sysfs_device *dev; struct udevice *udev; const char *maj, *min; struct udev_rules rules; < at >< at > -133,18 +132,23 < at >< at > int main(int argc, char *argv[]) sysfs_init(); udev_rules_init(&rules, 0); -dev = sysfs_device_get(devpath); -if (dev == NULL) { -info("unable to open '%s'\n", devpath); -goto fail; -} - udev = udev_device_init(); if (udev == NULL) goto fail; +/* valgrind can only verify sysfs_device_cleanup() calls if cache is disabled */ +#if 0 +udev->cache = sysfs_cache_init(); +if (udev->cache == NULL) +goto fail; +#endif + /* override built-in sysfs device */ -udev->dev = dev; +udev->dev = sysfs_device_get(udev->cache, devpath); +if (udev->dev == NULL) { +info("unable to open '%s'\n", devpath); +goto fail; +} strlcpy(udev->action, action, sizeof(udev->action)); /* get dev_t from environment, which is needed for "remove" to work, "add" works also from sysfs */ < at >< at > -164,8 +168,8 < at >< at > int main(int argc, char *argv[]) if (retval == 0 && !udev->ignore_device && udev_run) udev_rules_run(udev); -udev_device_cleanup(udev); fail: +udev_device_cleanup(udev); udev_rules_cleanup(&rules); sysfs_cleanup(); selinux_exit(); diff --git a/udev/udev.h b/udev/udev.h index 28b59db..1037734 100644 --- a/udev/udev.h +++ b/udev/udev.h < at >< at > -48,9 +48,13 < at >< at > struct udev_rules; +struct sysfs_cache { +struct list_head dev_list;/* device cache */ +struct list_head attr_list;/* attribute value cache */ +}; + struct sysfs_device { struct list_head node;/* for device cache */ -struct sysfs_device *parent;/* already cached parent*/ char devpath[PATH_SIZE]; char subsystem[NAME_SIZE];/* $class, $bus, drivers, module */ char kernel[NAME_SIZE];/* device instance name */ < at >< at > -63,6 +67,7 < at >< at > struct udevice { struct sysfs_device *dev;/* points to dev_local by default */ struct sysfs_device dev_local; struct sysfs_device *dev_parent;/* current parent device used for matching */ +struct sysfs_cache *cache; char action[NAME_SIZE]; char *devpath_old; < at >< at > -103,6 +108,9 < at >< at > extern void udev_config_init(void); /* udev_device.c */ extern struct udevice *udev_device_init(void); extern void udev_device_cleanup(struct udevice *udev); +extern void udev_device_parents_iter_init(struct udevice *udev, struct sysfs_device **iter); +extern void udev_device_parents_iter_next(struct udevice *udev, struct sysfs_device **iter); +extern void udev_device_parents_iter_cleanup(struct udevice *udev, struct sysfs_device **iter); extern dev_t udev_device_get_devt(struct udevice *udev); /* udev_device_event.c */ < at >< at > -112,12 +120,17 < at >< at > extern int udev_device_event(struct udev_rules *rules, struct udevice *udev); extern char sysfs_path[PATH_SIZE]; extern int sysfs_init(void); extern void sysfs_cleanup(void); +extern struct sysfs_cache *sysfs_cache_init(void); +extern void sysfs_cache_cleanup(struct sysfs_cache *cache); extern void sysfs_device_set_values(struct sysfs_device *dev, const char *devpath, const char *subsystem, const char *driver); -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 int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value, size_t len); +extern struct sysfs_device *sysfs_device_get(struct sysfs_cache *cache, const char *devpath); +extern struct sysfs_device *sysfs_device_get_parent(struct sysfs_cache *cache, struct sysfs_device *dev); +extern struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_cache *cache, + struct sysfs_device *dev, const char *subsystem); +extern void sysfs_device_cleanup(struct sysfs_device *dev); +int sysfs_attr_get_value(struct sysfs_cache *cache, 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 13000b0..a81d80e 100644 --- a/udev/udev_device.c +++ b/udev/udev_device.c < at >< at > -63,19 +63,46 < at >< at > void udev_device_cleanup(struct udevice *udev) { if (udev == NULL) return; +if (udev->dev != &udev->dev_local) +sysfs_device_cleanup(udev->dev); +udev_device_parents_iter_cleanup(udev, &udev->dev_parent); +sysfs_cache_cleanup(udev->cache); name_list_cleanup(&udev->symlink_list); name_list_cleanup(&udev->run_list); name_list_cleanup(&udev->env_list); free(udev); } +void udev_device_parents_iter_init(struct udevice *udev, struct sysfs_device **iter) +{ +*iter = udev->dev; +} + +void udev_device_parents_iter_next(struct udevice *udev, struct sysfs_device **iter) +{ +struct sysfs_device *dev; + +dev = *iter; +*iter = sysfs_device_get_parent(udev->cache, dev); + +if (dev != udev->dev) +sysfs_device_cleanup(dev); +} + +void udev_device_parents_iter_cleanup(struct udevice *udev, struct sysfs_device **iter) +{ +if (*iter != udev->dev) +sysfs_device_cleanup(*iter); +*iter = NULL; +} + dev_t udev_device_get_devt(struct udevice *udev) { char attr[NAME_SIZE]; unsigned int maj, min; /* read it from sysfs */ -if (sysfs_attr_get_value(udev->dev->devpath, "dev", attr, sizeof(attr))) { +if (sysfs_attr_get_value(udev->cache, 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 a29c4b7..f489b67 100644 --- a/udev/udev_node.c +++ b/udev/udev_node.c < at >< at > -380,7 +380,7 < at >< at > int udev_node_add(struct udevice *udev) int range; /* take the maximum registered minor range */ -if (sysfs_attr_get_value(udev->dev->devpath, "range", attr, sizeof(attr))) { +if (sysfs_attr_get_value(udev->cache, 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 53c483a..da7a62a 100644 --- a/udev/udev_rules.c +++ b/udev/udev_rules.c < at >< at > -416,43 +416,45 < at >< at > static int import_program_into_env(struct udevice *udev, const char *program) static int import_parent_into_env(struct udevice *udev, const char *filter) { struct sysfs_device *dev_parent; +struct udevice *udev_parent; +struct name_entry *name_loop; int rc = -1; -dev_parent = sysfs_device_get_parent(udev->dev); -if (dev_parent != NULL) { -struct udevice *udev_parent; -struct name_entry *name_loop; +dev_parent = sysfs_device_get_parent(udev->cache, udev->dev); +if (dev_parent == NULL) +goto out; -dbg("found parent '%s', get the node name\n", dev_parent->devpath); -udev_parent = udev_device_init(); -if (udev_parent == NULL) -return -1; -/* import the udev_db of the parent */ -if (udev_db_get_device(udev_parent, dev_parent->devpath) == 0) { -dbg("import stored parent env '%s'\n", udev_parent->name); -list_for_each_entry(name_loop, &udev_parent->env_list, node) { -char name[NAME_SIZE]; -char *pos; - -strlcpy(name, name_loop->name, sizeof(name)); -pos = strchr(name, '='); -if (pos) { -pos[0] = '\0'; -pos++; -if (fnmatch(filter, name, 0) == 0) { -dbg("import key '%s'\n", name_loop->name); -name_list_add(&udev->env_list, name_loop->name, 0); -setenv(name, pos, 1); -} else -dbg("skip key '%s'\n", name_loop->name); -} +dbg("found parent '%s', get the node name\n", dev_parent->devpath); +udev_parent = udev_device_init(); +if (udev_parent == NULL) +goto out; +/* import the udev_db of the parent */ +if (udev_db_get_device(udev_parent, dev_parent->devpath) == 0) { +dbg("import stored parent env '%s'\n", udev_parent->name); +list_for_each_entry(name_loop, &udev_parent->env_list, node) { +char name[NAME_SIZE]; +char *pos; + +strlcpy(name, name_loop->name, sizeof(name)); +pos = strchr(name, '='); +if (pos) { +pos[0] = '\0'; +pos++; +if (fnmatch(filter, name, 0) == 0) { +dbg("import key '%s'\n", name_loop->name); +name_list_add(&udev->env_list, name_loop->name, 0); +setenv(name, pos, 1); +} else +dbg("skip key '%s'\n", name_loop->name); } -rc = 0; -} else -dbg("parent not found in database\n"); -udev_device_cleanup(udev_parent); -} +} +rc = 0; +} else +dbg("parent not found in database\n"); +udev_device_cleanup(udev_parent); +out: +sysfs_device_cleanup(dev_parent); return rc; } < at >< at > -834,7 +836,7 < at >< at > found: if (attr_get_by_subsys_id(attr, devpath, sizeof(devpath), &attrib)) { if (attrib != NULL) -found = sysfs_attr_get_value(devpath, attrib, +found = sysfs_attr_get_value(udev->cache, devpath, attrib, temp2, sizeof(temp2)); else break; < at >< at > -842,20 +844,23 < at >< at > found: /* try the current device, other matches may have selected */ if (!found && udev->dev_parent != NULL && udev->dev_parent != udev->dev) -found = sysfs_attr_get_value(udev->dev_parent->devpath, attr, +found = sysfs_attr_get_value(udev->cache, udev->dev_parent->devpath, attr, temp2, sizeof(temp2)); /* look at all devices along the chain of parents */ if (!found) { -struct sysfs_device *dev_parent = udev->dev; +struct sysfs_device *dev_parent; +udev_device_parents_iter_init(udev, &dev_parent); do { dbg("looking at '%s'\n", dev_parent->devpath); -found = sysfs_attr_get_value(dev_parent->devpath, attr, +found = sysfs_attr_get_value(udev->cache, dev_parent->devpath, attr, temp2, sizeof(temp2)); if (found) break; -dev_parent = sysfs_device_get_parent(dev_parent); +udev_device_parents_iter_next(udev, &dev_parent); } while (dev_parent != NULL); + +udev_device_parents_iter_cleanup(udev, &dev_parent); } if (!found) < at >< at > -876,7 +881,7 < at >< at > found: { struct sysfs_device *dev_parent; -dev_parent = sysfs_device_get_parent(udev->dev); +dev_parent = sysfs_device_get_parent(udev->cache, udev->dev); if (dev_parent != NULL) { struct udevice *udev_parent; < at >< at > -891,6 +896,7 < at >< at > found: dbg("parent not found in database\n"); udev_device_cleanup(udev_parent); } +sysfs_device_cleanup(dev_parent); } } break; < at >< at > -1142,13 +1148,13 < at >< at > static int match_rule(struct udevice *udev, struct udev_rule *rule) if (attr_get_by_subsys_id(key_name, devpath, sizeof(devpath), &attrib)) { if (attrib != NULL) -found = sysfs_attr_get_value(devpath, attrib, +found = sysfs_attr_get_value(udev->cache, devpath, attrib, val, sizeof(val)); else goto nomatch; } if (!found) -found = sysfs_attr_get_value(udev->dev->devpath, key_name, +found = sysfs_attr_get_value(udev->cache, udev->dev->devpath, key_name, val, sizeof(val)); if (!found) goto nomatch; < at >< at > -1168,7 +1174,8 < at >< at > static int match_rule(struct udevice *udev, struct udev_rule *rule) } /* walk up the chain of parent devices and find a match */ -udev->dev_parent = udev->dev; +udev_device_parents_iter_cleanup(udev, &udev->dev_parent); +udev_device_parents_iter_init(udev, &udev->dev_parent); while (1) { /* check for matching kernel device name */ if (match_key("KERNELS", rule, &rule->kernels, udev->dev_parent->kernel)) < at >< at > -1194,10 +1201,10 < at >< at > static int match_rule(struct udevice *udev, struct udev_rule *rule) size_t len; int found; -found = sysfs_attr_get_value(udev->dev_parent->devpath, key_name, +found = sysfs_attr_get_value(udev->cache, udev->dev_parent->devpath, key_name, val, sizeof(val)); if (!found) -found = sysfs_attr_get_value(udev->dev->devpath, key_name, +found = sysfs_attr_get_value(udev->cache, udev->dev->devpath, key_name, val, sizeof(val)); if (!found) goto try_parent; < at >< at > -1221,7 +1228,8 < at >< at > static int match_rule(struct udevice *udev, struct udev_rule *rule) try_parent: /* move to parent device */ dbg("try parent sysfs device\n"); -udev->dev_parent = sysfs_device_get_parent(udev->dev_parent); +udev_device_parents_iter_next(udev, &udev->dev_parent); + if (udev->dev_parent == NULL) goto nomatch; dbg("looking at dev_parent->devpath='%s'\n", udev->dev_parent->devpath); diff --git a/udev/udev_sysfs.c b/udev/udev_sysfs.c index 2d2457f..4028f7e 100644 --- a/udev/udev_sysfs.c +++ b/udev/udev_sysfs.c < at >< at > -31,11 +31,6 < at >< at > 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]; < at >< at > -55,27 +50,48 < at >< at > int sysfs_init(void) strlcpy(sysfs_path, "/sys", sizeof(sysfs_path)); dbg("sysfs_path='%s'\n", sysfs_path); -INIT_LIST_HEAD(&dev_list); -INIT_LIST_HEAD(&attr_list); return 0; } void sysfs_cleanup(void) { +} + +struct sysfs_cache *sysfs_cache_init() +{ +struct sysfs_cache *cache; + +cache = malloc(sizeof(struct sysfs_cache)); +if (cache == NULL) +return NULL; + +INIT_LIST_HEAD(&cache->dev_list); +INIT_LIST_HEAD(&cache->attr_list); + +return cache; +} + +void sysfs_cache_cleanup(struct sysfs_cache *cache) +{ 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) { +if (cache == NULL) +return; + +list_for_each_entry_safe(attr_loop, attr_temp, &cache->attr_list, node) { list_del(&attr_loop->node); free(attr_loop); } -list_for_each_entry_safe(dev_loop, dev_temp, &dev_list, node) { +list_for_each_entry_safe(dev_loop, dev_temp, &cache->dev_list, node) { list_del(&dev_loop->node); free(dev_loop); } + +free(cache); } void sysfs_device_set_values(struct sysfs_device *dev, const char *devpath, < at >< at > -144,7 +160,7 < at >< at > int sysfs_resolve_link(char *devpath, size_t size) return 0; } -struct sysfs_device *sysfs_device_get(const char *devpath) +struct sysfs_device *sysfs_device_get(struct sysfs_cache *cache, const char *devpath) { char path[PATH_SIZE]; char devpath_real[PATH_SIZE]; < at >< at > -173,12 +189,14 < at >< at > struct sysfs_device *sysfs_device_get(const char *devpath) return NULL; /* 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 (cache != NULL) { +list_for_each_entry(dev_loop, &cache->dev_list, node) { 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)); < at >< at > -192,13 +210,15 < at >< at > struct sysfs_device *sysfs_device_get(const char *devpath) return NULL; /* now look for device in cache after path translation */ -list_for_each_entry(dev_loop, &dev_list, node) { +if (cache != NULL) { +list_for_each_entry(dev_loop, &cache->dev_list, node) { 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); < at >< at > -252,23 +272,22 < at >< at > struct sysfs_device *sysfs_device_get(const char *devpath) strlcpy(dev->driver, &pos[1], sizeof(dev->driver)); } -dbg("add to cache 'devpath=%s', subsystem='%s', driver='%s'\n", dev->devpath, dev->subsystem, dev->driver); -list_add(&dev->node, &dev_list); +dbg("cache 'devpath=%s', subsystem='%s', driver='%s'\n", dev->devpath, dev->subsystem, dev->driver); +if (cache != NULL) +list_add(&dev->node, &cache->dev_list); +else +INIT_LIST_HEAD(&dev->node); return dev; } -struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev) +struct sysfs_device *sysfs_device_get_parent(struct sysfs_cache *cache, struct sysfs_device *dev) { char parent_devpath[PATH_SIZE]; char *pos; dbg("open '%s'\n", dev->devpath); -/* look if we already know the parent */ -if (dev->parent != NULL) -return dev->parent; - strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); dbg("'%s'\n", parent_devpath); < at >< at > -295,9 +314,8 < at >< at > struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev) if (pos == NULL || pos == parent_devpath) return NULL; -/* get parent and remember it */ -dev->parent = sysfs_device_get(parent_devpath); -return dev->parent; +/* get parent */ +return sysfs_device_get(cache, parent_devpath); device_link: strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); < at >< at > -305,25 +323,38 < at >< at > device_link: if (sysfs_resolve_link(parent_devpath, sizeof(parent_devpath)) != 0) return NULL; -/* get parent and remember it */ -dev->parent = sysfs_device_get(parent_devpath); -return dev->parent; +/* get parent */ +return sysfs_device_get(cache, parent_devpath); } -struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem) +struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_cache *cache, + struct sysfs_device *dev, const char *subsystem) { struct sysfs_device *dev_parent; -dev_parent = sysfs_device_get_parent(dev); +dev_parent = sysfs_device_get_parent(cache, dev); while (dev_parent != NULL) { if (strcmp(dev_parent->subsystem, subsystem) == 0) return dev_parent; -dev_parent = sysfs_device_get_parent(dev_parent); +dev_parent = sysfs_device_get_parent(cache, dev_parent); } return NULL; } -int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value, size_t len) +void sysfs_device_cleanup(struct sysfs_device *dev) +{ +if (dev == NULL) +return; + +/* device is on a dev_list - will be freed by sysfs_cache_cleanup() */ +if (!list_empty(&dev->node)) +return; + +free(dev); +} + +int sysfs_attr_get_value(struct sysfs_cache *cache, const char *devpath, const char *attr_name, + char *value, size_t len) { char path_full[PATH_SIZE]; const char *path; < at >< at > -345,7 +376,8 < at >< at > int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value strlcat(path_full, attr_name, sizeof(path_full)); /* look for attribute in cache */ -list_for_each_entry(attr_loop, &attr_list, node) { +if (cache != NULL) { +list_for_each_entry(attr_loop, &cache->attr_list, node) { if (strcmp(attr_loop->path, path) == 0) { dbg("found in cache '%s'\n", attr_loop->path); if (!attr_loop->value) < at >< at > -364,7 +396,8 < at >< at > int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value 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); +list_add(&attr->node, &cache->attr_list); +} if (lstat(path_full, &statbuf) != 0) { dbg("stat '%s' failed: %s\n", path_full, strerror(errno)); < at >< at > -417,9 +450,11 < at >< at > int sysfs_attr_get_value(const char *devpath, const char *attr_name, char *value remove_trailing_chars(val, '\n'); out: -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 (cache != NULL) { +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; diff --git a/udev/udevinfo.c b/udev/udevinfo.c index b76d11d..50d1bef 100644 --- a/udev/udevinfo.c +++ b/udev/udevinfo.c < at >< at > -66,7 +66,7 < at >< at > static void print_all_attributes(const char *devpath, const char *key) if (S_ISLNK(statbuf.st_mode)) continue; -if (!sysfs_attr_get_value(devpath, dent->d_name, value, sizeof(value))) +if (!sysfs_attr_get_value(NULL, devpath, dent->d_name, value, sizeof(value))) continue; len = strlen(value); dbg("attr '%s'='%s'(%zi)\n", dent->d_name, value, len); < at >< at > -89,7 +89,7 < at >< at > static int print_device_chain(const char *devpath) { struct sysfs_device *dev; -dev = sysfs_device_get(devpath); +dev = sysfs_device_get(NULL, devpath); if (dev == NULL) return -1; < at >< at > -109,9 +109,14 < at >< at > static int print_device_chain(const char *devpath) /* walk up the chain of devices */ while (1) { -dev = sysfs_device_get_parent(dev); -if (dev == NULL) +struct sysfs_device *dev_parent; + +dev_parent = sysfs_device_get_parent(NULL, dev); +if (dev_parent == NULL) break; +sysfs_device_cleanup(dev); +dev = dev_parent; + printf(" looking at parent device '%s':\n", dev->devpath); printf(" KERNELS==\"%s\"\n", dev->kernel); printf(" SUBSYSTEMS==\"%s\"\n", dev->subsystem); < at >< at > -120,6 +125,7 < at >< at > static int print_device_chain(const char *devpath) print_all_attributes(dev->devpath, "ATTRS"); } +sysfs_device_cleanup(dev); return 0; } diff --git a/udev/udevtest.c b/udev/udevtest.c index 63603aa..cf96019 100644 --- a/udev/udevtest.c +++ b/udev/udevtest.c < at >< at > -78,7 +78,6 < at >< at > int udevtest(int argc, char *argv[]) const char *subsystem = NULL; const char *devpath = NULL; struct udevice *udev; -struct sysfs_device *dev; struct udev_rules rules = {}; int retval; int rc = 0; < at >< at > -150,25 +149,30 < at >< at > int udevtest(int argc, char *argv[]) if (strncmp(devpath, sysfs_path, strlen(sysfs_path)) == 0) devpath = &devpath[strlen(sysfs_path)]; -dev = sysfs_device_get(devpath); -if (dev == NULL) { -fprintf(stderr, "unable to open device '%s'\n", devpath); +udev = udev_device_init(); +if (udev == NULL) { +fprintf(stderr, "error initializing device\n"); rc = 2; goto exit; } -udev = udev_device_init(); -if (udev == NULL) { -fprintf(stderr, "error initializing device\n"); +udev->cache = sysfs_cache_init(); +if (udev->cache == NULL) { +fprintf(stderr, "error initializing sysfs cache\n"); rc = 3; goto exit; } +/* override built-in sysfs device */ +udev->dev = sysfs_device_get(udev->cache, devpath); +if (udev->dev == NULL) { +fprintf(stderr, "unable to open device '%s'\n", devpath); +rc = 4; +goto exit; +} if (subsystem != NULL) -strlcpy(dev->subsystem, subsystem, sizeof(dev->subsystem)); +strlcpy(udev->dev->subsystem, subsystem, sizeof(udev->dev->subsystem)); -/* override built-in sysfs device */ -udev->dev = dev; strlcpy(udev->action, action, sizeof(udev->action)); udev->devt = udev_device_get_devt(udev); < at >< at > -198,9 +202,9 < at >< at > int udevtest(int argc, char *argv[]) info("run: '%s'\n", program); } } -udev_device_cleanup(udev); exit: +udev_device_cleanup(udev); udev_rules_cleanup(&rules); sysfs_cleanup(); return rc; -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] sysfs: sysfs_attr_get_value() writes to a buffer instead of returning a string

Alan Jenkins — 2008-09-04T14:21:40

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 < at >< at > -224,9 +224,11 < at >< at > 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); < at >< at > -244,15 +246,17 < at >< at > 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); < at >< at > -269,7 +273,10 < at >< at > 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 */ < at >< at > -284,29 +291,33 < at >< at > 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; } < at >< at > -322,15 +333,18 < at >< at > 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; } < at >< at > -338,15 +352,18 < at >< at > 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; } < at >< at > -354,18 +371,20 < at >< at > 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 < at >< at > -117,7 +117,7 < at >< at > 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 < at >< at > -71,12 +71,11 < at >< at > 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 < at >< at > -376,12 +376,11 < at >< at > 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 < at >< at > -829,40 +829,40 < at >< at > 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); < at >< at > -1136,21 +1136,22 < at >< at > 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); < at >< at > -1189,16 +1190,17 < at >< at > 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 < at >< at > -174,11 +174,11 < at >< at > 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)); < at >< at > -193,12 +193,12 < at >< at > 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); < at >< at > -323,11 +323,11 < at >< at > 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; < at >< at > -346,76 +346,85 < at >< at > 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 < at >< at > -47,7 +47,6 < at >< at > 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; < at >< at > -67,12 +66,9 < at >< at > 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< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH 1/3] cleanup: remove redundant code from callers of sysfs_get_attr_value()

Alan Jenkins — 2008-09-04T14:20:35

* duplicate strlcpy in udev_rules_apply_format() * "remove trailing newlines" in udevinfo, when sysfs_attr_get_value() does that already. diff --git a/udev/udev_rules.c b/udev/udev_rules.c index 4719cab..f7acd9c 100644 --- a/udev/udev_rules.c +++ b/udev/udev_rules.c < at >< at > -850,10 +850,8 < at >< at > found: do { dbg("looking at '%s'\n", dev_parent->devpath); value = sysfs_attr_get_value(dev_parent->devpath, attr); -if (value != NULL) { -strlcpy(temp2, value, sizeof(temp2)); +if (value != NULL) break; -} dev_parent = sysfs_device_get_parent(dev_parent); } while (dev_parent != NULL); } diff --git a/udev/udevinfo.c b/udev/udevinfo.c index 738ffc6..9b75364 100644 --- a/udev/udevinfo.c +++ b/udev/udevinfo.c < at >< at > -75,10 +75,6 < at >< at > static void print_all_attributes(const char *devpath, const char *key) len = sizeof(value) - 1; dbg("attr '%s'='%s'(%zi)\n", dent->d_name, value, len); -/* remove trailing newlines */ -while (len && value[len-1] == '\n') -value[--len] = '\0'; - /* skip nonprintable attributes */ while (len && isprint(value[len-1])) len--; -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: sync udev with sysfs

Kay Sievers — 2008-09-03T20:08:40

We fixed almost all known timing issues in the kernel, and the former WAIT_FOR_SYSFS rules file we used to ship is empty now. We try to avoid to work around such issues in userspace if possible. Maybe the bluetooth uevent timing in the kernel needs some attention? Marcel? Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] The commented line in udev-test.pl to use valgrind needs fixing

Kay Sievers — 2008-09-03T20:02:49

Removed. Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

sync udev with sysfs

Ron Rindjunsky — 2008-09-03T14:40:01

Hi folks i am a newbie to a the udev uses, so i'll thank any piece of information you can give. i am trying to use udev in the next fashion: every time my sdio bluetooth device is up, echo a value to sysfs entry of the driver from a script i am running from /lib/udev the problem seems to be that the sysfs entry i am trying to echo to is being created _after_ i do the echo. is there any way to sync between my echo command and the sysfs entry creation of the driver? i tried using WAIT_FOR_SYSFS but got nothing in this direction Thanks --Ron -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] The commented line in udev-test.pl to use valgrind needs fixing

Alan Jenkins — 2008-09-03T12:50:38

Ah, you forgot to remove this line though: # uncomment following line to run udev with valgrind. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Split udev_sysfs.c to leave out caches from libudev

Alan Jenkins — 2008-09-03T11:20:16

Oops. sysfs_cache can't be made optional ("just use NULL") without a bit more effort. At the moment we rely on sysfs_cleanup() freeing everything in the cache. If the cache isn't used, we need a new API to free individual results. So we also need sysfs_device_cleanup(), to free the device if it's not on a cache list. And sysfs_attr_get_value() can copy the result to a buffer instead of returning a string. I think that's reasonable. Alan -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Split udev_sysfs.c to leave out caches from libudev

Alan Jenkins — 2008-09-03T10:33:30

Good point. That helps make the cache usage more explicit, and it's much nicer for my purposes as well. (I had blindly converted the global cache variables to per-thread variables, without touching the callers). Ok. To keep the existing behaviour where udev_rules_get_run() reuses the cache from udev_rules_get_name(), they both need a sysfs_cache parameter. Then I have to change test-udev and udevtest as well as udevd - no point having test programs if they use different code paths. I'll see what that looks like then and resend. Thanks Alan -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] cleanup: remove deprecated envp (third argument to main)

Kay Sievers — 2008-09-02T21:24:05

Applied. Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [RFC PATCH] Split udev_sysfs.c to leave out caches from libudev

Kay Sievers — 2008-09-02T21:12:51

Sure, the cache was never coded to be used in threaded programs. For good reason, yeah. And it will not, at least not in its current state. Wouldn't it be nicer to create a sysfs_cache object, which is passed to all these functions? The "cheap" tools can just use NULL there, but any possible user could have more than one instance of the cache. The cache is really needed if you do thousand of rule compares against a sysfs value, over and over. I guess we should start changing the stuff to work with local vars, instead of moving the current global stuff to a different file? In the long run libudev will take over a lot of the functionality and replace the old code, but depending on the actual need, it might take a while ... Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[PATCH] cleanup: remove deprecated envp (third argument to main)

Alan Jenkins — 2008-09-02T18:26:08

envp is not standardized, and may become invalid when environment variables are modified. Since udev never actually uses it, we can simply remove it. Should anyone miss it in future, they can use the standardized environ variable - like udev_rules.c does already. https://www.securecoding.cert.org/confluence/display/seccode/ENV31-C.+Do+not+rely+on+an+environment+pointer+following+an+operation+that+may+invalidate+it --- Ulterior motive: threaded udevd can't use environment variables directly. This patch explains why I would import variables from environ, and not envp. diff --git a/udev/test-udev.c b/udev/test-udev.c index 484c3c0..53fc0ce 100644 --- a/udev/test-udev.c +++ b/udev/test-udev.c < at >< at > -59,7 +59,7 < at >< at > static void asmlinkage sig_handler(int signum) } } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { struct sysfs_device *dev; struct udevice *udev; diff --git a/udev/udev.h b/udev/udev.h index 455b68e..990ef01 100644 --- a/udev/udev.h +++ b/udev/udev.h < at >< at > -172,11 +172,11 < at >< at > extern int unlink_secure(const char *filename); extern size_t buf_get_line(const char *buf, size_t buflen, size_t cur); /* udev commands */ -extern int udevmonitor(int argc, char *argv[], char *envp[]); -extern int udevinfo(int argc, char *argv[], char *envp[]); -extern int udevcontrol(int argc, char *argv[], char *envp[]); -extern int udevtrigger(int argc, char *argv[], char *envp[]); -extern int udevsettle(int argc, char *argv[], char *envp[]); -extern int udevtest(int argc, char *argv[], char *envp[]); +extern int udevmonitor(int argc, char *argv[]); +extern int udevinfo(int argc, char *argv[]); +extern int udevcontrol(int argc, char *argv[]); +extern int udevtrigger(int argc, char *argv[]); +extern int udevsettle(int argc, char *argv[]); +extern int udevtest(int argc, char *argv[]); #endif diff --git a/udev/udevadm.c b/udev/udevadm.c index 7adb7c5..cb356c5 100644 --- a/udev/udevadm.c +++ b/udev/udevadm.c < at >< at > -49,20 +49,20 < at >< at > void log_message(int priority, const char *format, ...) struct command { const char *name; -int (*cmd)(int argc, char *argv[], char *envp[]); +int (*cmd)(int argc, char *argv[]); const char *help; int debug; }; static const struct command cmds[]; -static int version(int argc, char *argv[], char *envp[]) +static int version(int argc, char *argv[]) { printf("%s\n", VERSION); return 0; } -static int help(int argc, char *argv[], char *envp[]) +static int help(int argc, char *argv[]) { const struct command *cmd; < at >< at > -118,7 +118,7 < at >< at > static const struct command cmds[] = { {} }; -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { const char *command; const char *pos; < at >< at > -157,7 +157,7 < at >< at > int main(int argc, char *argv[], char *envp[]) for (cmd = cmds; cmd->name != NULL; cmd++) { if (strcmp(cmd->name, command) == 0) { debug = cmd->debug; -rc = cmd->cmd(argc, argv, envp); +rc = cmd->cmd(argc, argv); goto out; } } diff --git a/udev/udevcontrol.c b/udev/udevcontrol.c index a02bfc4..f4830d8 100644 --- a/udev/udevcontrol.c +++ b/udev/udevcontrol.c < at >< at > -36,7 +36,7 < at >< at > static int sock = -1; static int udev_log = 0; -int udevcontrol(int argc, char *argv[], char *envp[]) +int udevcontrol(int argc, char *argv[]) { static struct udevd_ctrl_msg ctrl_msg; struct sockaddr_un saddr; diff --git a/udev/udevd.c b/udev/udevd.c index bbe0b6d..5e41ee5 100644 --- a/udev/udevd.c +++ b/udev/udevd.c < at >< at > -953,7 +953,7 < at >< at > static void export_initial_seqnum(void) } } -int main(int argc, char *argv[], char *envp[]) +int main(int argc, char *argv[]) { int retval; int fd; diff --git a/udev/udevinfo.c b/udev/udevinfo.c index a97f09c..7e6c94e 100644 --- a/udev/udevinfo.c +++ b/udev/udevinfo.c < at >< at > -228,7 +228,7 < at >< at > static int stat_device(const char *name, int export, const char *prefix) return 0; } -int udevinfo(int argc, char *argv[], char *envp[]) +int udevinfo(int argc, char *argv[]) { int option; struct udevice *udev; diff --git a/udev/udevmonitor.c b/udev/udevmonitor.c index 2430dd3..3b65bca 100644 --- a/udev/udevmonitor.c +++ b/udev/udevmonitor.c < at >< at > -123,7 +123,7 < at >< at > static const char *search_key(const char *searchkey, const char *buf, size_t buf return NULL; } -int udevmonitor(int argc, char *argv[], char *envp[]) +int udevmonitor(int argc, char *argv[]) { struct sigaction act; int option; diff --git a/udev/udevsettle.c b/udev/udevsettle.c index f00a38b..11277f5 100644 --- a/udev/udevsettle.c +++ b/udev/udevsettle.c < at >< at > -67,7 +67,7 < at >< at > static void print_queue(const char *dir) printf("\n\n"); } -int udevsettle(int argc, char *argv[], char *envp[]) +int udevsettle(int argc, char *argv[]) { char queuename[PATH_SIZE]; char filename[PATH_SIZE]; diff --git a/udev/udevtest.c b/udev/udevtest.c index 2b43691..019b482 100644 --- a/udev/udevtest.c +++ b/udev/udevtest.c < at >< at > -71,7 +71,7 < at >< at > out: return rc; } -int udevtest(int argc, char *argv[], char *envp[]) +int udevtest(int argc, char *argv[]) { int force = 0; const char *action = "add"; diff --git a/udev/udevtrigger.c b/udev/udevtrigger.c index 19a3dbb..c0bd845 100644 --- a/udev/udevtrigger.c +++ b/udev/udevtrigger.c < at >< at > -560,7 +560,7 < at >< at > static void scan_failed(void) } } -int udevtrigger(int argc, char *argv[], char *envp[]) +int udevtrigger(int argc, char *argv[]) { int failed = 0; const char *sockpath = NULL; -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

[RFC PATCH] Split udev_sysfs.c to leave out caches from libudev

Alan Jenkins — 2008-09-02T11:09:08

udev_sysfs.c device and attribute value caches are only appropriate for short-lived processes, like udev-event and usbid. In long lived processes they would fill up with stale data. Fortunately libudev doesn't use these caches. But this should be made explicit to guard against future changes. Let's move the caches into a separate file, with its own initialization and cleanup functions. Signed-off-by: Alan Jenkins tuffmail.co.uk> diff --git a/extras/usb_id/Makefile.am b/extras/usb_id/Makefile.am index 0de004a..bf3aa1c 100644 --- a/extras/usb_id/Makefile.am +++ b/extras/usb_id/Makefile.am < at >< at > -9,6 +9,7 < at >< at > AM_CPPFLAGS = \ usb_id_SOURCES = \ usb_id.c \ ../../udev/udev_sysfs.c \ +../../udev/udev_sysfs_cache.c \ ../../udev/udev_sysdeps.c \ ../../udev/udev_utils.c \ ../../udev/udev_utils_string.c diff --git a/extras/usb_id/usb_id.c b/extras/usb_id/usb_id.c index 92492c8..cd0705d 100644 --- a/extras/usb_id/usb_id.c +++ b/extras/usb_id/usb_id.c < at >< at > -387,6 +387,7 < at >< at > int main(int argc, char **argv) logging_init("usb_id"); sysfs_init(); +sysfs_cache_init(); while (1) { int option; < at >< at > -463,6 +464,7 < at >< at > int main(int argc, char **argv) } exit: +sysfs_cache_cleanup(); sysfs_cleanup(); logging_close(); return retval; diff --git a/udev/Makefile.am b/udev/Makefile.am index 4a7c785..2d25518 100644 --- a/udev/Makefile.am +++ b/udev/Makefile.am < at >< at > -30,6 +30,7 < at >< at > common_files = \ udev_rules_parse.c \ udev_sysdeps.c \ udev_sysfs.c \ +udev_sysfs_cache.c \ udev_utils.c \ udev_utils_file.c \ udev_utils_string.c diff --git a/udev/test-udev.c b/udev/test-udev.c index 591e930..484c3c0 100644 --- a/udev/test-udev.c +++ b/udev/test-udev.c < at >< at > -130,6 +130,7 < at >< at > int main(int argc, char *argv[], char *envp[]) } sysfs_init(); +sysfs_cache_init(); udev_rules_init(&rules, 0); dev = sysfs_device_get(devpath); < at >< at > -166,6 +167,7 < at >< at > int main(int argc, char *argv[], char *envp[]) udev_device_cleanup(udev); fail: udev_rules_cleanup(&rules); +sysfs_cache_cleanup(); sysfs_cleanup(); selinux_exit(); diff --git a/udev/udev.h b/udev/udev.h index b633807..455b68e 100644 --- a/udev/udev.h +++ b/udev/udev.h < at >< at > -114,12 +114,16 < at >< at > extern int sysfs_init(void); extern void sysfs_cleanup(void); extern void sysfs_device_set_values(struct sysfs_device *dev, const char *devpath, const char *subsystem, const char *driver); +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); + +/* udev_sysfs_cache.c */ +extern int sysfs_cache_init(void); +extern void sysfs_cache_cleanup(void); 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_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); /* udev_node.c */ extern int udev_node_mknod(struct udevice *udev, const char *file, dev_t devt, mode_t mode, uid_t uid, gid_t gid); diff --git a/udev/udev_device.c b/udev/udev_device.c index 9888676..a2f225a 100644 --- a/udev/udev_device.c +++ b/udev/udev_device.c < at >< at > -66,17 +66,3 < at >< at > void udev_device_cleanup(struct udevice *udev) name_list_cleanup(&udev->env_list); free(udev); } - -dev_t udev_device_get_devt(struct udevice *udev) -{ -const char *attr; -unsigned int maj, min; - -/* read it from sysfs */ -attr = sysfs_attr_get_value(udev->dev->devpath, "dev"); -if (attr != NULL) { -if (sscanf(attr, "%u:%u", &maj, &min) == 2) -return makedev(maj, min); -} -return makedev(0, 0); -} diff --git a/udev/udev_device_event.c b/udev/udev_device_event.c index 045035d..d031226 100644 --- a/udev/udev_device_event.c +++ b/udev/udev_device_event.c < at >< at > -257,3 +257,17 < at >< at > int udev_device_event(struct udev_rules *rules, struct udevice *udev) exit: return retval; } + +dev_t udev_device_get_devt(struct udevice *udev) +{ +const char *attr; +unsigned int maj, min; + +/* read it from sysfs */ +attr = sysfs_attr_get_value(udev->dev->devpath, "dev"); +if (attr != NULL) { +if (sscanf(attr, "%u:%u", &maj, &min) == 2) +return makedev(maj, min); +} +return makedev(0, 0); +} diff --git a/udev/udev_sysfs.c b/udev/udev_sysfs.c index 91f5997..c4247fa 100644 --- a/udev/udev_sysfs.c +++ b/udev/udev_sysfs.c < at >< at > -31,18 +31,6 < at >< at > 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; < at >< at > -55,27 +43,11 < at >< at > int sysfs_init(void) strlcpy(sysfs_path, "/sys", sizeof(sysfs_path)); dbg("sysfs_path='%s'\n", 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(&attr_loop->node); -free(attr_loop); -} - -list_for_each_entry_safe(dev_loop, dev_temp, &dev_list, node) { -list_del(&dev_loop->node); -free(dev_loop); -} } void sysfs_device_set_values(struct sysfs_device *dev, const char *devpath, < at >< at > -144,280 +116,6 < at >< at > int sysfs_resolve_link(char *devpath, size_t size) return 0; } -struct sysfs_device *sysfs_device_get(const char *devpath) -{ -char path[PATH_SIZE]; -char devpath_real[PATH_SIZE]; -struct sysfs_device *dev; -struct sysfs_device *dev_loop; -struct stat statbuf; -char link_path[PATH_SIZE]; -char link_target[PATH_SIZE]; -int len; -char *pos; - -/* we handle only these devpathes */ -if (devpath != NULL && - strncmp(devpath, "/devices/", 9) != 0 && - strncmp(devpath, "/subsystem/", 11) != 0 && - strncmp(devpath, "/module/", 8) != 0 && - strncmp(devpath, "/bus/", 5) != 0 && - strncmp(devpath, "/class/", 7) != 0 && - strncmp(devpath, "/block/", 7) != 0) -return NULL; - -dbg("open '%s'\n", devpath); -strlcpy(devpath_real, devpath, sizeof(devpath_real)); -remove_trailing_chars(devpath_real, '/'); -if (devpath[0] == '\0' ) -return NULL; - -/* 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 we got a link, resolve it to the real device */ -strlcpy(path, sysfs_path, sizeof(path)); -strlcat(path, devpath_real, sizeof(path)); -if (lstat(path, &statbuf) != 0) { -dbg("stat '%s' failed: %s\n", path, strerror(errno)); -return NULL; -} -if (S_ISLNK(statbuf.st_mode)) { -if (sysfs_resolve_link(devpath_real, sizeof(devpath_real)) != 0) -return NULL; - -/* 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; -} -} -} - -/* it is a new device */ -dbg("new uncached device '%s'\n", devpath_real); -dev = malloc(sizeof(struct sysfs_device)); -if (dev == NULL) -return NULL; -memset(dev, 0x00, sizeof(struct sysfs_device)); - -sysfs_device_set_values(dev, devpath_real, NULL, NULL); - -/* get subsystem name */ -strlcpy(link_path, sysfs_path, sizeof(link_path)); -strlcat(link_path, dev->devpath, sizeof(link_path)); -strlcat(link_path, "/subsystem", sizeof(link_path)); -len = readlink(link_path, link_target, sizeof(link_target)); -if (len > 0) { -/* get subsystem from "subsystem" link */ -link_target[len] = '\0'; -dbg("subsystem link '%s' points to '%s'\n", link_path, link_target); -pos = strrchr(link_target, '/'); -if (pos != NULL) -strlcpy(dev->subsystem, &pos[1], sizeof(dev->subsystem)); -} else if (strstr(dev->devpath, "/drivers/") != NULL) { -strlcpy(dev->subsystem, "drivers", sizeof(dev->subsystem)); -} else if (strncmp(dev->devpath, "/module/", 8) == 0) { -strlcpy(dev->subsystem, "module", sizeof(dev->subsystem)); -} else if (strncmp(dev->devpath, "/subsystem/", 11) == 0) { -pos = strrchr(dev->devpath, '/'); -if (pos == &dev->devpath[10]) -strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); -} else if (strncmp(dev->devpath, "/class/", 7) == 0) { -pos = strrchr(dev->devpath, '/'); -if (pos == &dev->devpath[6]) -strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); -} else if (strncmp(dev->devpath, "/bus/", 5) == 0) { -pos = strrchr(dev->devpath, '/'); -if (pos == &dev->devpath[4]) -strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); -} - -/* get driver name */ -strlcpy(link_path, sysfs_path, sizeof(link_path)); -strlcat(link_path, dev->devpath, sizeof(link_path)); -strlcat(link_path, "/driver", sizeof(link_path)); -len = readlink(link_path, link_target, sizeof(link_target)); -if (len > 0) { -link_target[len] = '\0'; -dbg("driver link '%s' points to '%s'\n", link_path, link_target); -pos = strrchr(link_target, '/'); -if (pos != NULL) -strlcpy(dev->driver, &pos[1], sizeof(dev->driver)); -} - -dbg("add to cache 'devpath=%s', subsystem='%s', driver='%s'\n", dev->devpath, dev->subsystem, dev->driver); -list_add(&dev->node, &dev_list); - -return dev; -} - -struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev) -{ -char parent_devpath[PATH_SIZE]; -char *pos; - -dbg("open '%s'\n", dev->devpath); - -/* look if we already know the parent */ -if (dev->parent != NULL) -return dev->parent; - -strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); -dbg("'%s'\n", parent_devpath); - -/* strip last element */ -pos = strrchr(parent_devpath, '/'); -if (pos == NULL || pos == parent_devpath) -return NULL; -pos[0] = '\0'; - -if (strncmp(parent_devpath, "/class", 6) == 0) { -pos = strrchr(parent_devpath, '/'); -if (pos == &parent_devpath[6] || pos == parent_devpath) { -dbg("/class top level, look for device link\n"); -goto device_link; -} -} -if (strcmp(parent_devpath, "/block") == 0) { -dbg("/block top level, look for device link\n"); -goto device_link; -} - -/* are we at the top level? */ -pos = strrchr(parent_devpath, '/'); -if (pos == NULL || pos == parent_devpath) -return NULL; - -/* get parent and remember it */ -dev->parent = sysfs_device_get(parent_devpath); -return dev->parent; - -device_link: -strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); -strlcat(parent_devpath, "/device", sizeof(parent_devpath)); -if (sysfs_resolve_link(parent_devpath, sizeof(parent_devpath)) != 0) -return NULL; - -/* get parent and remember it */ -dev->parent = sysfs_device_get(parent_devpath); -return dev->parent; -} - -struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem) -{ -struct sysfs_device *dev_parent; - -dev_parent = sysfs_device_get_parent(dev); -while (dev_parent != NULL) { -if (strcmp(dev_parent->subsystem, subsystem) == 0) -return dev_parent; -dev_parent = sysfs_device_get_parent(dev_parent); -} -return NULL; -} - -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; -size_t sysfs_len; - -dbg("open '%s'/'%s'\n", devpath, 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'\n", attr_loop->path); -return attr_loop->value; -} -} - -/* 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); -list_add(&attr->node, &attr_list); - -if (lstat(path_full, &statbuf) != 0) { -dbg("stat '%s' failed: %s\n", path_full, strerror(errno)); -goto out; -} - -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; -} -} -goto out; -} - -/* skip directories */ -if (S_ISDIR(statbuf.st_mode)) -goto out; - -/* skip non-readable files */ -if ((statbuf.st_mode & S_IRUSR) == 0) -goto out; - -/* read attribute value */ -fd = open(path_full, O_RDONLY); -if (fd < 0) { -dbg("attribute '%s' can not be opened\n", path_full); -goto out; -} -size = read(fd, value, sizeof(value)); -close(fd); -if (size < 0) -goto out; -if (size == sizeof(value)) -goto out; - -/* 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; - -out: -return attr->value; -} - int sysfs_lookup_devpath_by_subsys_id(char *devpath_full, size_t len, const char *subsystem, const char *id) { size_t sysfs_len; diff --git a/udev/udev_sysfs_cache.c b/udev/udev_sysfs_cache.c new file mode 100644 index 0000000..2f33ed5 --- /dev/null +++ b/udev/udev_sysfs_cache.c < at >< at > -0,0 +1,341 < at >< at > +/* + * Copyright (C) 2005-2006 Kay Sievers vrfy.org> + * + *This program is free software; you can redistribute it and/or modify it + *under the terms of the GNU General Public License as published by the + *Free Software Foundation version 2 of the License. + * + *This program is distributed in the hope that it will be useful, but + *WITHOUT ANY WARRANTY; without even the implied warranty of + *MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + *General Public License for more details. + * + *You should have received a copy of the GNU General Public License along + *with this program; if not, write to the Free Software Foundation, Inc., + *51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + * + */ + + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "udev.h" + +/* 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_cache_init(void) +{ +INIT_LIST_HEAD(&dev_list); +INIT_LIST_HEAD(&attr_list); +return 0; +} + +void sysfs_cache_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(&attr_loop->node); +free(attr_loop); +} + +list_for_each_entry_safe(dev_loop, dev_temp, &dev_list, node) { +list_del(&dev_loop->node); +free(dev_loop); +} +} + +struct sysfs_device *sysfs_device_get(const char *devpath) +{ +char path[PATH_SIZE]; +char devpath_real[PATH_SIZE]; +struct sysfs_device *dev; +struct sysfs_device *dev_loop; +struct stat statbuf; +char link_path[PATH_SIZE]; +char link_target[PATH_SIZE]; +int len; +char *pos; + +/* we handle only these devpathes */ +if (devpath != NULL && + strncmp(devpath, "/devices/", 9) != 0 && + strncmp(devpath, "/subsystem/", 11) != 0 && + strncmp(devpath, "/module/", 8) != 0 && + strncmp(devpath, "/bus/", 5) != 0 && + strncmp(devpath, "/class/", 7) != 0 && + strncmp(devpath, "/block/", 7) != 0) +return NULL; + +dbg("open '%s'\n", devpath); +strlcpy(devpath_real, devpath, sizeof(devpath_real)); +remove_trailing_chars(devpath_real, '/'); +if (devpath[0] == '\0' ) +return NULL; + +/* 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 we got a link, resolve it to the real device */ +strlcpy(path, sysfs_path, sizeof(path)); +strlcat(path, devpath_real, sizeof(path)); +if (lstat(path, &statbuf) != 0) { +dbg("stat '%s' failed: %s\n", path, strerror(errno)); +return NULL; +} +if (S_ISLNK(statbuf.st_mode)) { +if (sysfs_resolve_link(devpath_real, sizeof(devpath_real)) != 0) +return NULL; + +/* 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; +} +} +} + +/* it is a new device */ +dbg("new uncached device '%s'\n", devpath_real); +dev = malloc(sizeof(struct sysfs_device)); +if (dev == NULL) +return NULL; +memset(dev, 0x00, sizeof(struct sysfs_device)); + +sysfs_device_set_values(dev, devpath_real, NULL, NULL); + +/* get subsystem name */ +strlcpy(link_path, sysfs_path, sizeof(link_path)); +strlcat(link_path, dev->devpath, sizeof(link_path)); +strlcat(link_path, "/subsystem", sizeof(link_path)); +len = readlink(link_path, link_target, sizeof(link_target)); +if (len > 0) { +/* get subsystem from "subsystem" link */ +link_target[len] = '\0'; +dbg("subsystem link '%s' points to '%s'\n", link_path, link_target); +pos = strrchr(link_target, '/'); +if (pos != NULL) +strlcpy(dev->subsystem, &pos[1], sizeof(dev->subsystem)); +} else if (strstr(dev->devpath, "/drivers/") != NULL) { +strlcpy(dev->subsystem, "drivers", sizeof(dev->subsystem)); +} else if (strncmp(dev->devpath, "/module/", 8) == 0) { +strlcpy(dev->subsystem, "module", sizeof(dev->subsystem)); +} else if (strncmp(dev->devpath, "/subsystem/", 11) == 0) { +pos = strrchr(dev->devpath, '/'); +if (pos == &dev->devpath[10]) +strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); +} else if (strncmp(dev->devpath, "/class/", 7) == 0) { +pos = strrchr(dev->devpath, '/'); +if (pos == &dev->devpath[6]) +strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); +} else if (strncmp(dev->devpath, "/bus/", 5) == 0) { +pos = strrchr(dev->devpath, '/'); +if (pos == &dev->devpath[4]) +strlcpy(dev->subsystem, "subsystem", sizeof(dev->subsystem)); +} + +/* get driver name */ +strlcpy(link_path, sysfs_path, sizeof(link_path)); +strlcat(link_path, dev->devpath, sizeof(link_path)); +strlcat(link_path, "/driver", sizeof(link_path)); +len = readlink(link_path, link_target, sizeof(link_target)); +if (len > 0) { +link_target[len] = '\0'; +dbg("driver link '%s' points to '%s'\n", link_path, link_target); +pos = strrchr(link_target, '/'); +if (pos != NULL) +strlcpy(dev->driver, &pos[1], sizeof(dev->driver)); +} + +dbg("add to cache 'devpath=%s', subsystem='%s', driver='%s'\n", dev->devpath, dev->subsystem, dev->driver); +list_add(&dev->node, &dev_list); + +return dev; +} + +struct sysfs_device *sysfs_device_get_parent(struct sysfs_device *dev) +{ +char parent_devpath[PATH_SIZE]; +char *pos; + +dbg("open '%s'\n", dev->devpath); + +/* look if we already know the parent */ +if (dev->parent != NULL) +return dev->parent; + +strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); +dbg("'%s'\n", parent_devpath); + +/* strip last element */ +pos = strrchr(parent_devpath, '/'); +if (pos == NULL || pos == parent_devpath) +return NULL; +pos[0] = '\0'; + +if (strncmp(parent_devpath, "/class", 6) == 0) { +pos = strrchr(parent_devpath, '/'); +if (pos == &parent_devpath[6] || pos == parent_devpath) { +dbg("/class top level, look for device link\n"); +goto device_link; +} +} +if (strcmp(parent_devpath, "/block") == 0) { +dbg("/block top level, look for device link\n"); +goto device_link; +} + +/* are we at the top level? */ +pos = strrchr(parent_devpath, '/'); +if (pos == NULL || pos == parent_devpath) +return NULL; + +/* get parent and remember it */ +dev->parent = sysfs_device_get(parent_devpath); +return dev->parent; + +device_link: +strlcpy(parent_devpath, dev->devpath, sizeof(parent_devpath)); +strlcat(parent_devpath, "/device", sizeof(parent_devpath)); +if (sysfs_resolve_link(parent_devpath, sizeof(parent_devpath)) != 0) +return NULL; + +/* get parent and remember it */ +dev->parent = sysfs_device_get(parent_devpath); +return dev->parent; +} + +struct sysfs_device *sysfs_device_get_parent_with_subsystem(struct sysfs_device *dev, const char *subsystem) +{ +struct sysfs_device *dev_parent; + +dev_parent = sysfs_device_get_parent(dev); +while (dev_parent != NULL) { +if (strcmp(dev_parent->subsystem, subsystem) == 0) +return dev_parent; +dev_parent = sysfs_device_get_parent(dev_parent); +} +return NULL; +} + +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; +size_t sysfs_len; + +dbg("open '%s'/'%s'\n", devpath, 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'\n", attr_loop->path); +return attr_loop->value; +} +} + +/* 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); +list_add(&attr->node, &attr_list); + +if (lstat(path_full, &statbuf) != 0) { +dbg("stat '%s' failed: %s\n", path_full, strerror(errno)); +goto out; +} + +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; +} +} +goto out; +} + +/* skip directories */ +if (S_ISDIR(statbuf.st_mode)) +goto out; + +/* skip non-readable files */ +if ((statbuf.st_mode & S_IRUSR) == 0) +goto out; + +/* read attribute value */ +fd = open(path_full, O_RDONLY); +if (fd < 0) { +dbg("attribute '%s' can not be opened\n", path_full); +goto out; +} +size = read(fd, value, sizeof(value)); +close(fd); +if (size < 0) +goto out; +if (size == sizeof(value)) +goto out; + +/* 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; + +out: +return attr->value; +} diff --git a/udev/udevd.c b/udev/udevd.c index 22d261f..bbe0b6d 100644 --- a/udev/udevd.c +++ b/udev/udevd.c < at >< at > -1077,6 +1077,7 < at >< at > int main(int argc, char *argv[], char *envp[]) /* parse the rules and keep them in memory */ sysfs_init(); +sysfs_cache_init(); udev_rules_init(&rules, 1); export_initial_seqnum(); < at >< at > -1289,6 +1290,7 < at >< at > int main(int argc, char *argv[], char *envp[]) exit: udev_rules_cleanup(&rules); sysfs_cleanup(); +sysfs_cache_cleanup(); selinux_exit(); if (signal_pipe[READ_END] >= 0) -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: How to create /dev/pts

Aras Vaichas — 2008-09-01T23:28:56

If it will help, this is the top section of my init script. I create the pts directory. # essential system init mount -t tmpfs mdev /dev mkdir /dev/pts mount -t devpts devpts /dev/pts mount proc mount sys # copy over some necessary devices cp -a /lib/udev/devices/* /dev where: # ll /lib/udev/devices/ drwxr-xr-x 3 root root 4096 Sep 1 2008 ./ drwxr-xr-x 4 root root 4096 Aug 29 2008 ../ lrwxrwxrwx 1 root root 11 Jul 7 2008 core -> /proc/kcore< at > lrwxrwxrwx 1 root root 13 Jul 7 2008 fd -> /proc/self/fd/ crw-rw-rw- 1 root root 1, 3 Aug 12 2008 null crw-rw-r-- 1 root root 108, 0 Sep 1 2008 ppp drwxrwxrwt 2 root root 4096 Jan 9 2006 shm/ lrwxrwxrwx 1 root root 4 Jul 7 2008 stderr -> fd/2 lrwxrwxrwx 1 root root 4 Jul 7 2008 stdin -> fd/0 lrwxrwxrwx 1 root root 4 Jul 7 2008 stdout -> fd/1 crw-r--r-- 1 root root 4, 64 Aug 27 2008 ttyS0 Some of those might be unnecessary, I'm currently in the middle of an overhaul and update of our whole distribution. Aras ______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email ______________________________________________________________________ -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Some function locals in udev_rules_parse.c wereneedlessly declared as static

Kay Sievers — 2008-09-01T17:50:16

Ah, they should return a pointer to the data passed in to the function, which should be fine. Applied. Thanks, Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: [PATCH] Some function locals in udev_rules_parse.c were needlessly declared as static

Alan Jenkins — 2008-09-01T17:37:11

Thanks for looking at these patches. The functions do "return rule", but they always write to it before reading it, so there's no persistent state here. And they don't "return &rule", so it's fine for the variable to be on the stack. Did I miss something? I belatedly noticed the other parsing stuff a few hours after posting the patch :-). I hacked it up and (with unpublished patches) finally got a threaded udevd which appeared to work. (I also did per-thread environment variable emulation, and fixed the caches in udev_sysfs.c for thread-safety). So empirically I had judged this patch correct. And my unpublished patches wouldn't conflict with or obsolete this one. Regards Alan -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Re: Udevstart & udevtrigger

Kay Sievers — 2008-09-01T15:26:34

Udevstart is obsolete and does not exist anymore. It was replaced, and is not installed since udev 082: http://git.kernel.org/?p=linux/hotplug/udev.git;a=commit;h=c233b59baaabe55a8f99dc37c4ee9011b85f8917 Kay -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html

Udevstart & udevtrigger

Francesco RUNDO — 2008-09-01T14:44:13

Hi, My name is Francesco Rundo and I work porting Linux (kernel, device drivers, etc..) for embedded system. I've a question about udev, in particular way, I need to get some info about udevstart and udevtrigger. In order to improve the boot speed, I've removed in my udev init scripts the tool udevstart. Basically, in my udev init scripts (which start firstly in the user space boot sequence) there are in order: 1) some operation on /dev with manually creation of some devices; 2) udevstart call; 3) starting of udevd; 4) udevtrigger call; 5) udevsettle call; After that, the common services will be initialized: ssh, D-bus, HAL, Lirc, etc...; I've noted that this sequence is used in different Linux distribution for embedded system. As far as I know, the tool "udevstart" has the target to populate initial device directoty (/dev). It walks trough the sysfs device tree and calls udev (daemon) to create the nodes for every valid device found. It is used to fill the initial empty device directory with nodes for all devices currently available. Is it right ? Now, I know that "udevtrigger" replies the uevents generated by system coldplug of the devices (passing the info to the udev daemon) while udevsettle waits watching the udev event queue and exits if all current events are handled. Is it right ? By taking into account what above mentioned, I've removed from udev starting script the tool "udevstart" which needs a lot of time to populate the device (In my kernel config, I've a lot of device included in the kernel ) and I've managed udev only with udevtrigger and udevsettle. In this way, the device creation under /dev will be made by udevtrigger. I've saved 3/4 sec of boot time!! Finally, I've to say that in some web site I've noted that some Linux distribution were using this approach. Also the initNG project suggests to remove udevstart using only udevtrigger & udevsettle. Please, see http://www.initng.org/wiki/udevstart for more detail. Now the question is: Is it right to remove udevstart using only udevtrigger ? Are there some funcionality-restrictions in the udev if I don't use udevstart ? Is it true that starting from udev-117 udevstart will not be delivered ? In my distribution, we are aligned to udev-116 Thanks in advance for your support. Best Regards, -- Francesco Rundo -- To unsubscribe from this list: send the line "unsubscribe linux-hotplug" in the body of a message to majordomo< at >vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html