From: Yu Watanabe Date: Wed, 12 Jun 2019 04:03:19 +0000 (+0900) Subject: libudev: hide definition of struct udev_list from other libudev components X-Git-Tag: v243-rc1~293^2~3 X-Git-Url: http://git.ipfire.org/?p=thirdparty%2Fsystemd.git;a=commitdiff_plain;h=dcf557f7b04a3c5202ddbafca06311a12442fd4c libudev: hide definition of struct udev_list from other libudev components In the later commit, udev_list will be just a wrapper of hashmap or LIST. So, allocating udev_list does not increase much cost. --- diff --git a/src/libudev/libudev-device-internal.h b/src/libudev/libudev-device-internal.h index 8a6e5a48f65..2be49c98927 100644 --- a/src/libudev/libudev-device-internal.h +++ b/src/libudev/libudev-device-internal.h @@ -23,16 +23,16 @@ struct udev_device { struct udev_device *parent; bool parent_set; - struct udev_list properties; + struct udev_list *properties; uint64_t properties_generation; - struct udev_list tags; + struct udev_list *tags; uint64_t tags_generation; - struct udev_list devlinks; + struct udev_list *devlinks; uint64_t devlinks_generation; bool properties_read:1; bool tags_read:1; bool devlinks_read:1; - struct udev_list sysattrs; + struct udev_list *sysattrs; bool sysattrs_read; }; diff --git a/src/libudev/libudev-device.c b/src/libudev/libudev-device.c index 357adf69644..d3a3c539697 100644 --- a/src/libudev/libudev-device.c +++ b/src/libudev/libudev-device.c @@ -168,10 +168,24 @@ _public_ const char *udev_device_get_property_value(struct udev_device *udev_dev } struct udev_device *udev_device_new(struct udev *udev, sd_device *device) { + _cleanup_(udev_list_freep) struct udev_list *properties = NULL, *tags = NULL, *sysattrs = NULL, *devlinks = NULL; struct udev_device *udev_device; assert(device); + properties = udev_list_new(true); + if (!properties) + return_with_errno(NULL, ENOMEM); + tags = udev_list_new(true); + if (!tags) + return_with_errno(NULL, ENOMEM); + sysattrs = udev_list_new(true); + if (!sysattrs) + return_with_errno(NULL, ENOMEM); + devlinks = udev_list_new(true); + if (!devlinks) + return_with_errno(NULL, ENOMEM); + udev_device = new(struct udev_device, 1); if (!udev_device) return_with_errno(NULL, ENOMEM); @@ -180,13 +194,12 @@ struct udev_device *udev_device_new(struct udev *udev, sd_device *device) { .n_ref = 1, .udev = udev, .device = sd_device_ref(device), + .properties = TAKE_PTR(properties), + .tags = TAKE_PTR(tags), + .sysattrs = TAKE_PTR(sysattrs), + .devlinks = TAKE_PTR(devlinks), }; - udev_list_init(&udev_device->properties, true); - udev_list_init(&udev_device->tags, true); - udev_list_init(&udev_device->sysattrs, true); - udev_list_init(&udev_device->devlinks, true); - return udev_device; } @@ -429,10 +442,10 @@ static struct udev_device *udev_device_free(struct udev_device *udev_device) { sd_device_unref(udev_device->device); udev_device_unref(udev_device->parent); - udev_list_cleanup(&udev_device->properties); - udev_list_cleanup(&udev_device->sysattrs); - udev_list_cleanup(&udev_device->tags); - udev_list_cleanup(&udev_device->devlinks); + udev_list_free(udev_device->properties); + udev_list_free(udev_device->sysattrs); + udev_list_free(udev_device->tags); + udev_list_free(udev_device->devlinks); return mfree(udev_device); } @@ -587,17 +600,17 @@ _public_ struct udev_list_entry *udev_device_get_devlinks_list_entry(struct udev !udev_device->devlinks_read) { const char *devlink; - udev_list_cleanup(&udev_device->devlinks); + udev_list_cleanup(udev_device->devlinks); FOREACH_DEVICE_DEVLINK(udev_device->device, devlink) - if (!udev_list_entry_add(&udev_device->devlinks, devlink, NULL)) + if (!udev_list_entry_add(udev_device->devlinks, devlink, NULL)) return_with_errno(NULL, ENOMEM); udev_device->devlinks_read = true; udev_device->devlinks_generation = device_get_devlinks_generation(udev_device->device); } - return udev_list_get_entry(&udev_device->devlinks); + return udev_list_get_entry(udev_device->devlinks); } /** @@ -619,17 +632,17 @@ _public_ struct udev_list_entry *udev_device_get_properties_list_entry(struct ud !udev_device->properties_read) { const char *key, *value; - udev_list_cleanup(&udev_device->properties); + udev_list_cleanup(udev_device->properties); FOREACH_DEVICE_PROPERTY(udev_device->device, key, value) - if (!udev_list_entry_add(&udev_device->properties, key, value)) + if (!udev_list_entry_add(udev_device->properties, key, value)) return_with_errno(NULL, ENOMEM); udev_device->properties_read = true; udev_device->properties_generation = device_get_properties_generation(udev_device->device); } - return udev_list_get_entry(&udev_device->properties); + return udev_list_get_entry(udev_device->properties); } /** @@ -739,16 +752,16 @@ _public_ struct udev_list_entry *udev_device_get_sysattr_list_entry(struct udev_ if (!udev_device->sysattrs_read) { const char *sysattr; - udev_list_cleanup(&udev_device->sysattrs); + udev_list_cleanup(udev_device->sysattrs); FOREACH_DEVICE_SYSATTR(udev_device->device, sysattr) - if (!udev_list_entry_add(&udev_device->sysattrs, sysattr, NULL)) + if (!udev_list_entry_add(udev_device->sysattrs, sysattr, NULL)) return_with_errno(NULL, ENOMEM); udev_device->sysattrs_read = true; } - return udev_list_get_entry(&udev_device->sysattrs); + return udev_list_get_entry(udev_device->sysattrs); } /** @@ -794,17 +807,17 @@ _public_ struct udev_list_entry *udev_device_get_tags_list_entry(struct udev_dev !udev_device->tags_read) { const char *tag; - udev_list_cleanup(&udev_device->tags); + udev_list_cleanup(udev_device->tags); FOREACH_DEVICE_TAG(udev_device->device, tag) - if (!udev_list_entry_add(&udev_device->tags, tag, NULL)) + if (!udev_list_entry_add(udev_device->tags, tag, NULL)) return_with_errno(NULL, ENOMEM); udev_device->tags_read = true; udev_device->tags_generation = device_get_tags_generation(udev_device->device); } - return udev_list_get_entry(&udev_device->tags); + return udev_list_get_entry(udev_device->tags); } /** diff --git a/src/libudev/libudev-enumerate.c b/src/libudev/libudev-enumerate.c index 80d5bafdf7d..481c90df105 100644 --- a/src/libudev/libudev-enumerate.c +++ b/src/libudev/libudev-enumerate.c @@ -34,7 +34,7 @@ struct udev_enumerate { struct udev *udev; unsigned n_ref; - struct udev_list devices_list; + struct udev_list *devices_list; bool devices_uptodate:1; sd_device_enumerator *enumerator; @@ -50,6 +50,7 @@ struct udev_enumerate { **/ _public_ struct udev_enumerate *udev_enumerate_new(struct udev *udev) { _cleanup_(sd_device_enumerator_unrefp) sd_device_enumerator *e = NULL; + _cleanup_(udev_list_freep) struct udev_list *list = NULL; struct udev_enumerate *udev_enumerate; int r; @@ -61,6 +62,10 @@ _public_ struct udev_enumerate *udev_enumerate_new(struct udev *udev) { if (r < 0) return_with_errno(NULL, r); + list = udev_list_new(false); + if (!list) + return_with_errno(NULL, ENOMEM); + udev_enumerate = new(struct udev_enumerate, 1); if (!udev_enumerate) return_with_errno(NULL, ENOMEM); @@ -69,17 +74,16 @@ _public_ struct udev_enumerate *udev_enumerate_new(struct udev *udev) { .udev = udev, .n_ref = 1, .enumerator = TAKE_PTR(e), + .devices_list = TAKE_PTR(list), }; - udev_list_init(&udev_enumerate->devices_list, false); - return udev_enumerate; } static struct udev_enumerate *udev_enumerate_free(struct udev_enumerate *udev_enumerate) { assert(udev_enumerate); - udev_list_cleanup(&udev_enumerate->devices_list); + udev_list_free(udev_enumerate->devices_list); sd_device_enumerator_unref(udev_enumerate->enumerator); return mfree(udev_enumerate); } @@ -134,7 +138,7 @@ _public_ struct udev_list_entry *udev_enumerate_get_list_entry(struct udev_enume if (!udev_enumerate->devices_uptodate) { sd_device *device; - udev_list_cleanup(&udev_enumerate->devices_list); + udev_list_cleanup(udev_enumerate->devices_list); FOREACH_DEVICE_AND_SUBSYSTEM(udev_enumerate->enumerator, device) { const char *syspath; @@ -144,14 +148,14 @@ _public_ struct udev_list_entry *udev_enumerate_get_list_entry(struct udev_enume if (r < 0) return_with_errno(NULL, r); - if (!udev_list_entry_add(&udev_enumerate->devices_list, syspath, NULL)) + if (!udev_list_entry_add(udev_enumerate->devices_list, syspath, NULL)) return_with_errno(NULL, ENOMEM); } udev_enumerate->devices_uptodate = true; } - e = udev_list_get_entry(&udev_enumerate->devices_list); + e = udev_list_get_entry(udev_enumerate->devices_list); if (!e) return_with_errno(NULL, ENODATA); diff --git a/src/libudev/libudev-hwdb.c b/src/libudev/libudev-hwdb.c index ed755e5d3cc..5299e0a16f5 100644 --- a/src/libudev/libudev-hwdb.c +++ b/src/libudev/libudev-hwdb.c @@ -23,7 +23,7 @@ struct udev_hwdb { unsigned n_ref; sd_hwdb *hwdb; - struct udev_list properties_list; + struct udev_list *properties_list; }; /** @@ -35,6 +35,7 @@ struct udev_hwdb { * Returns: a hwdb context. **/ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) { + _cleanup_(udev_list_freep) struct udev_list *list = NULL; _cleanup_(sd_hwdb_unrefp) sd_hwdb *hwdb_internal = NULL; struct udev_hwdb *hwdb; int r; @@ -43,6 +44,10 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) { if (r < 0) return_with_errno(NULL, r); + list = udev_list_new(true); + if (!list) + return_with_errno(NULL, ENOMEM); + hwdb = new(struct udev_hwdb, 1); if (!hwdb) return_with_errno(NULL, ENOMEM); @@ -50,10 +55,9 @@ _public_ struct udev_hwdb *udev_hwdb_new(struct udev *udev) { *hwdb = (struct udev_hwdb) { .n_ref = 1, .hwdb = TAKE_PTR(hwdb_internal), + .properties_list = TAKE_PTR(list), }; - udev_list_init(&hwdb->properties_list, true); - return hwdb; } @@ -61,7 +65,7 @@ static struct udev_hwdb *udev_hwdb_free(struct udev_hwdb *hwdb) { assert(hwdb); sd_hwdb_unref(hwdb->hwdb); - udev_list_cleanup(&hwdb->properties_list); + udev_list_free(hwdb->properties_list); return mfree(hwdb); } @@ -105,13 +109,13 @@ _public_ struct udev_list_entry *udev_hwdb_get_properties_list_entry(struct udev assert_return_errno(hwdb, NULL, EINVAL); assert_return_errno(modalias, NULL, EINVAL); - udev_list_cleanup(&hwdb->properties_list); + udev_list_cleanup(hwdb->properties_list); SD_HWDB_FOREACH_PROPERTY(hwdb->hwdb, modalias, key, value) - if (!udev_list_entry_add(&hwdb->properties_list, key, value)) + if (!udev_list_entry_add(hwdb->properties_list, key, value)) return_with_errno(NULL, ENOMEM); - e = udev_list_get_entry(&hwdb->properties_list); + e = udev_list_get_entry(hwdb->properties_list); if (!e) return_with_errno(NULL, ENODATA); diff --git a/src/libudev/libudev-list-internal.h b/src/libudev/libudev-list-internal.h index 4e1632c78d6..a15b3853439 100644 --- a/src/libudev/libudev-list-internal.h +++ b/src/libudev/libudev-list-internal.h @@ -3,19 +3,14 @@ #include "libudev.h" -struct udev_list_node { - struct udev_list_node *next, *prev; -}; +#include "macro.h" -struct udev_list { - struct udev_list_node node; - struct udev_list_entry **entries; - unsigned entries_cur; - unsigned entries_max; - bool unique; -}; +struct udev_list; -void udev_list_init(struct udev_list *list, bool unique); +struct udev_list *udev_list_new(bool unique); void udev_list_cleanup(struct udev_list *list); +struct udev_list *udev_list_free(struct udev_list *list); +DEFINE_TRIVIAL_CLEANUP_FUNC(struct udev_list *, udev_list_free); + struct udev_list_entry *udev_list_get_entry(struct udev_list *list); struct udev_list_entry *udev_list_entry_add(struct udev_list *list, const char *name, const char *value); diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c index e051f1a80c5..f8cd51f3c72 100644 --- a/src/libudev/libudev-list.c +++ b/src/libudev/libudev-list.c @@ -16,6 +16,10 @@ * Libudev list operations. */ +struct udev_list_node { + struct udev_list_node *next, *prev; +}; + /** * udev_list_entry: * @@ -29,6 +33,14 @@ struct udev_list_entry { char *value; }; +struct udev_list { + struct udev_list_node node; + struct udev_list_entry **entries; + unsigned entries_cur; + unsigned entries_max; + bool unique; +}; + /* the list's head points to itself if empty */ static void udev_list_node_init(struct udev_list_node *list) { list->next = list; @@ -64,10 +76,20 @@ static struct udev_list_entry *list_node_to_entry(struct udev_list_node *node) { return container_of(node, struct udev_list_entry, node); } -void udev_list_init(struct udev_list *list, bool unique) { - memzero(list, sizeof(struct udev_list)); - list->unique = unique; +struct udev_list *udev_list_new(bool unique) { + struct udev_list *list; + + list = new(struct udev_list, 1); + if (!list) + return NULL; + + *list = (struct udev_list) { + .unique = unique, + }; + udev_list_node_init(&list->node); + + return list; } /* insert entry into a list as the last element */ @@ -211,8 +233,7 @@ static void udev_list_entry_delete(struct udev_list_entry *entry) { entry = tmp, tmp = udev_list_entry_get_next(tmp)) void udev_list_cleanup(struct udev_list *list) { - struct udev_list_entry *entry_loop; - struct udev_list_entry *entry_tmp; + struct udev_list_entry *entry_loop, *entry_tmp; list->entries = mfree(list->entries); list->entries_cur = 0; @@ -221,6 +242,15 @@ void udev_list_cleanup(struct udev_list *list) { udev_list_entry_delete(entry_loop); } +struct udev_list *udev_list_free(struct udev_list *list) { + if (!list) + return NULL; + + udev_list_cleanup(list); + + return mfree(list); +} + struct udev_list_entry *udev_list_get_entry(struct udev_list *list) { if (udev_list_node_is_empty(&list->node)) return NULL; diff --git a/src/test/test-libudev.c b/src/test/test-libudev.c index 7878512465e..dcb5bcc2544 100644 --- a/src/test/test-libudev.c +++ b/src/test/test-libudev.c @@ -433,19 +433,20 @@ static void test_util_resolve_subsys_kernel(void) { } static void test_list(void) { - struct udev_list list = {}; + _cleanup_(udev_list_freep) struct udev_list *list = NULL; struct udev_list_entry *e; /* empty list */ - udev_list_init(&list, false); - assert_se(!udev_list_get_entry(&list)); + assert_se(list = udev_list_new(false)); + assert_se(!udev_list_get_entry(list)); + list = udev_list_free(list); /* unique == false */ - udev_list_init(&list, false); - assert_se(udev_list_entry_add(&list, "aaa", "hoge")); - assert_se(udev_list_entry_add(&list, "aaa", "hogehoge")); - assert_se(udev_list_entry_add(&list, "bbb", "foo")); - e = udev_list_get_entry(&list); + assert_se(list = udev_list_new(false)); + assert_se(udev_list_entry_add(list, "aaa", "hoge")); + assert_se(udev_list_entry_add(list, "aaa", "hogehoge")); + assert_se(udev_list_entry_add(list, "bbb", "foo")); + e = udev_list_get_entry(list); assert_se(e); assert_se(streq_ptr(udev_list_entry_get_name(e), "aaa")); assert_se(streq_ptr(udev_list_entry_get_value(e), "hoge")); @@ -462,14 +463,14 @@ static void test_list(void) { assert_se(!udev_list_entry_get_by_name(e, "aaa")); assert_se(!udev_list_entry_get_by_name(e, "bbb")); assert_se(!udev_list_entry_get_by_name(e, "ccc")); - udev_list_cleanup(&list); + list = udev_list_free(list); /* unique == true */ - udev_list_init(&list, true); - assert_se(udev_list_entry_add(&list, "aaa", "hoge")); - assert_se(udev_list_entry_add(&list, "aaa", "hogehoge")); - assert_se(udev_list_entry_add(&list, "bbb", "foo")); - e = udev_list_get_entry(&list); + assert_se(list = udev_list_new(true)); + assert_se(udev_list_entry_add(list, "aaa", "hoge")); + assert_se(udev_list_entry_add(list, "aaa", "hogehoge")); + assert_se(udev_list_entry_add(list, "bbb", "foo")); + e = udev_list_get_entry(list); assert_se(e); assert_se(streq_ptr(udev_list_entry_get_name(e), "aaa")); assert_se(streq_ptr(udev_list_entry_get_value(e), "hogehoge")); @@ -487,7 +488,6 @@ static void test_list(void) { assert_se(streq_ptr(udev_list_entry_get_name(e), "aaa")); assert_se(streq_ptr(udev_list_entry_get_value(e), "hogehoge")); assert_se(!udev_list_entry_get_by_name(e, "ccc")); - udev_list_cleanup(&list); } static int parse_args(int argc, char *argv[], const char **syspath, const char **subsystem) {