list: make LIST_FOREACH() and LIST_FOREACH_BACKWARDS() safer

yuwata · yuwata · commit 80a226b26b5e · 2022-03-19T08:10:29.000+09:00
diff --git a/src/basic/list.h b/src/basic/list.h
@@ -139,14 +139,17 @@
 /* The type of the iterator 'i' is automatically determined by the type of 'head', and declared in the
  * loop. Hence, do not declare the same variable in the outer scope. Sometimes, we set 'head' through
  * hashmap_get(). In that case, you need to explicitly cast the result. */
+#define LIST_FOREACH_WITH_NEXT(name,i,n,head)                           \
+        for (typeof(*(head)) *n, *i = (head); i && (n = i->name##_next, true); i = n)
+
 #define LIST_FOREACH(name,i,head)                                       \
-        for (typeof(*(head)) *i = (head); i; i = i->name##_next)
+        LIST_FOREACH_WITH_NEXT(name, i, UNIQ_T(n, UNIQ), head)
 
-#define LIST_FOREACH_SAFE(name,i,n,head)                                \
-        for (typeof(*(head)) *n, *i = (head); i && ((n = i->name##_next), 1); i = n)
+#define _LIST_FOREACH_WITH_PREV(name,i,p,start)                         \
+        for (typeof(*(start)) *p, *i = (start); i && (p = i->name##_prev, true); i = p)
 
-#define LIST_FOREACH_BACKWARDS(name,i,p)                                \
-        for (typeof(*(p)) *i = (p); i; i = i->name##_prev)
+#define LIST_FOREACH_BACKWARDS(name,i,start)                            \
+        _LIST_FOREACH_WITH_PREV(name, i, UNIQ_T(p, UNIQ), start)
 
 /* Iterate through all the members of the list p is included in, but skip over p */
 #define LIST_FOREACH_OTHERS(name,i,p)                                   \
diff --git a/src/core/device.c b/src/core/device.c
@@ -719,7 +719,7 @@ static void device_update_found_by_sysfs(Manager *m, const char *sysfs, DeviceFo
                 return;
 
         l = hashmap_get(m->devices_by_sysfs, sysfs);
-        LIST_FOREACH_SAFE(same_sysfs, d, n, l)
+        LIST_FOREACH(same_sysfs, d, l)
                 device_update_found_one(d, found, mask);
 }
 
@@ -905,7 +905,7 @@ static void device_propagate_reload_by_sysfs(Manager *m, const char *sysfs) {
         assert(sysfs);
 
         l = hashmap_get(m->devices_by_sysfs, sysfs);
-        LIST_FOREACH_SAFE(same_sysfs, d, n, l) {
+        LIST_FOREACH(same_sysfs, d, l) {
                 if (d->state == DEVICE_DEAD)
                         continue;
 
diff --git a/src/core/service.c b/src/core/service.c
@@ -507,7 +507,7 @@ static void service_remove_fd_store(Service *s, const char *name) {
         assert(s);
         assert(name);
 
-        LIST_FOREACH_SAFE(fd_store, fs, n, s->fd_store) {
+        LIST_FOREACH(fd_store, fs, s->fd_store) {
                 if (!streq(fs->fdname, name))
                         continue;
 
diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c
@@ -108,7 +108,7 @@ int sd_dhcp6_lease_get_server_address(sd_dhcp6_lease *lease, struct in6_addr *re
 void dhcp6_ia_clear_addresses(DHCP6IA *ia) {
         assert(ia);
 
-        LIST_FOREACH_SAFE(addresses, a, n, ia->addresses)
+        LIST_FOREACH(addresses, a, ia->addresses)
                 free(a);
 
         ia->addresses = NULL;
diff --git a/src/libudev/libudev-list.c b/src/libudev/libudev-list.c
@@ -117,7 +117,7 @@ void udev_list_cleanup(struct udev_list *list) {
                 list->uptodate = false;
                 hashmap_clear_with_destructor(list->unique_entries, udev_list_entry_free);
         } else
-                LIST_FOREACH_SAFE(entries, i, n, list->entries)
+                LIST_FOREACH(entries, i, list->entries)
                         udev_list_entry_free(i);
 }
 
diff --git a/src/network/netdev/wireguard.c b/src/network/netdev/wireguard.c
@@ -1146,7 +1146,7 @@ static int wireguard_verify(NetDev *netdev, const char *filename) {
                                               "%s: Missing PrivateKey= or PrivateKeyFile=, "
                                               "Ignoring network device.", filename);
 
-        LIST_FOREACH_SAFE(peers, peer, peer_next, w->peers) {
+        LIST_FOREACH(peers, peer, w->peers) {
                 if (wireguard_peer_verify(peer) < 0) {
                         wireguard_peer_free(peer);
                         continue;
diff --git a/src/partition/repart.c b/src/partition/repart.c
@@ -1902,7 +1902,7 @@ static int context_load_partition_table(
 static void context_unload_partition_table(Context *context) {
         assert(context);
 
-        LIST_FOREACH_SAFE(partitions, p, next, context->partitions) {
+        LIST_FOREACH(partitions, p, context->partitions) {
 
                 /* Entirely remove partitions that have no configuration */
                 if (PARTITION_IS_FOREIGN(p)) {
diff --git a/src/resolve/resolved-dns-cache.c b/src/resolve/resolved-dns-cache.c
@@ -144,7 +144,7 @@ static bool dns_cache_remove_by_key(DnsCache *c, DnsResourceKey *key) {
         if (!first)
                 return false;
 
-        LIST_FOREACH_SAFE(by_key, i, n, first) {
+        LIST_FOREACH(by_key, i, first) {
                 prioq_remove(c->by_expiry, i, &i->prioq_idx);
                 dns_cache_item_free(i);
         }
diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
@@ -635,7 +635,7 @@ static int on_stream_complete(DnsStream *s, int error) {
         }
 
         if (error != 0)
-                LIST_FOREACH_SAFE(transactions_by_stream, t, n, s->transactions)
+                LIST_FOREACH(transactions_by_stream, t, s->transactions)
                         on_transaction_stream_error(t, error);
 
         return 0;
diff --git a/src/rfkill/rfkill.c b/src/rfkill/rfkill.c
@@ -190,7 +190,7 @@ static int load_state(Context *c, const struct rfkill_event *event) {
 static void save_state_queue_remove(Context *c, int idx, const char *state_file) {
         assert(c);
 
-        LIST_FOREACH_SAFE(queue, item, tmp, c->write_queue)
+        LIST_FOREACH(queue, item, c->write_queue)
                 if ((state_file && streq(item->file, state_file)) || idx == item->rfkill_idx) {
                         log_debug("Canceled previous save state of '%s' to %s.", one_zero(item->state), item->file);
                         LIST_REMOVE(queue, c->write_queue, item);
diff --git a/src/shared/condition.c b/src/shared/condition.c
@@ -89,7 +89,7 @@ Condition* condition_free(Condition *c) {
 }
 
 Condition* condition_free_list_type(Condition *head, ConditionType type) {
-        LIST_FOREACH_SAFE(conditions, c, n, head)
+        LIST_FOREACH(conditions, c, head)
                 if (type < 0 || c->type == type) {
                         LIST_REMOVE(conditions, head, c);
                         condition_free(c);
diff --git a/src/shutdown/umount.c b/src/shutdown/umount.c
@@ -639,7 +639,7 @@ static int swap_points_list_off(MountPoint **head, bool *changed) {
         assert(head);
         assert(changed);
 
-        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+        LIST_FOREACH(mount_point, m, *head) {
                 log_info("Deactivating swap %s.", m->path);
                 if (swapoff(m->path) < 0) {
                         log_warning_errno(errno, "Could not deactivate swap %s: %m", m->path);
@@ -663,7 +663,7 @@ static int loopback_points_list_detach(MountPoint **head, bool *changed, int umo
 
         (void) get_block_device("/", &rootdev);
 
-        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+        LIST_FOREACH(mount_point, m, *head) {
                 if (major(rootdev) != 0 && rootdev == m->devnum) {
                         n_failed++;
                         continue;
@@ -694,7 +694,7 @@ static int dm_points_list_detach(MountPoint **head, bool *changed, int umount_lo
 
         (void) get_block_device("/", &rootdev);
 
-        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+        LIST_FOREACH(mount_point, m, *head) {
                 if (major(rootdev) != 0 && rootdev == m->devnum) {
                         n_failed ++;
                         continue;
@@ -724,7 +724,7 @@ static int md_points_list_detach(MountPoint **head, bool *changed, int umount_lo
 
         (void) get_block_device("/", &rootdev);
 
-        LIST_FOREACH_SAFE(mount_point, m, n, *head) {
+        LIST_FOREACH(mount_point, m, *head) {
                 if (major(rootdev) != 0 && rootdev == m->devnum) {
                         n_failed ++;
                         continue;
diff --git a/src/timesync/timesyncd-manager.c b/src/timesync/timesyncd-manager.c
@@ -1008,7 +1008,7 @@ static int manager_network_read_link_servers(Manager *m) {
                 }
         }
 
-        LIST_FOREACH_SAFE(names, n, nx, m->link_servers)
+        LIST_FOREACH(names, n, m->link_servers)
                 if (n->marked) {
                         server_name_free(n);
                         changed = true;
diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c
@@ -72,7 +72,7 @@ static void link_configs_free(LinkConfigContext *ctx) {
         if (!ctx)
                 return;
 
-        LIST_FOREACH_SAFE(configs, config, config_next, ctx->configs)
+        LIST_FOREACH(configs, config, ctx->configs)
                 link_config_free(config);
 }
 
diff --git a/src/udev/udev-rules.c b/src/udev/udev-rules.c
@@ -270,7 +270,7 @@ static void udev_rule_token_free(UdevRuleToken *token) {
 static void udev_rule_line_clear_tokens(UdevRuleLine *rule_line) {
         assert(rule_line);
 
-        LIST_FOREACH_SAFE(tokens, i, next, rule_line->tokens)
+        LIST_FOREACH(tokens, i, rule_line->tokens)
                 udev_rule_token_free(i);
 
         rule_line->tokens = NULL;
@@ -299,7 +299,7 @@ static void udev_rule_file_free(UdevRuleFile *rule_file) {
         if (!rule_file)
                 return;
 
-        LIST_FOREACH_SAFE(rule_lines, i, next, rule_file->rule_lines)
+        LIST_FOREACH(rule_lines, i, rule_file->rule_lines)
                 udev_rule_line_free(i);
 
         free(rule_file->filename);
@@ -310,7 +310,7 @@ UdevRules *udev_rules_free(UdevRules *rules) {
         if (!rules)
                 return NULL;
 
-        LIST_FOREACH_SAFE(rule_files, i, next, rules->rule_files)
+        LIST_FOREACH(rule_files, i, rules->rule_files)
                 udev_rule_file_free(i);
 
         hashmap_free_free_key(rules->known_users);
@@ -1143,7 +1143,7 @@ static void rule_resolve_goto(UdevRuleFile *rule_file) {
         assert(rule_file);
 
         /* link GOTOs to LABEL rules in this file to be able to fast-forward */
-        LIST_FOREACH_SAFE(rule_lines, line, line_next, rule_file->rule_lines) {
+        LIST_FOREACH(rule_lines, line, rule_file->rule_lines) {
                 if (!FLAGS_SET(line->type, LINE_HAS_GOTO))
                         continue;
 
@@ -2486,7 +2486,7 @@ static int udev_rule_apply_line_to_event(
 
         DEVICE_TRACE_POINT(rules_apply_line, event->dev, line->rule_file->filename, line->line_number);
 
-        LIST_FOREACH_SAFE(tokens, token, next_token, line->tokens) {
+        LIST_FOREACH(tokens, token, line->tokens) {
                 line->current_token = token;
 
                 if (token_is_for_parents(token)) {
@@ -2526,7 +2526,7 @@ int udev_rules_apply_to_event(
 
         LIST_FOREACH(rule_files, file, rules->rule_files) {
                 rules->current_file = file;
-                LIST_FOREACH_SAFE(rule_lines, line, next_line, file->rule_lines) {
+                LIST_FOREACH_WITH_NEXT(rule_lines, line, next_line, file->rule_lines) {
                         file->current_line = line;
                         r = udev_rule_apply_line_to_event(rules, event, timeout_usec, timeout_signal, properties_list, &next_line);
                         if (r < 0)
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
@@ -181,7 +181,7 @@ static Event *event_free(Event *event) {
 }
 
 static void event_queue_cleanup(Manager *manager, EventState match_state) {
-        LIST_FOREACH_SAFE(event, event, tmp, manager->events) {
+        LIST_FOREACH(event, event, manager->events) {
                 if (match_state != EVENT_UNDEF && match_state != event->state)
                         continue;
 
@@ -949,7 +949,7 @@ static int event_queue_start(Manager *manager) {
                         return log_warning_errno(r, "Failed to read udev rules: %m");
         }
 
-        LIST_FOREACH_SAFE(event, event, event_next, manager->events) {
+        LIST_FOREACH(event, event, manager->events) {
                 if (event->state != EVENT_QUEUED)
                         continue;
 

Original file line number	Diff line number	Diff line change
`@@ -117,7 +117,7 @@ void udev_list_cleanup(struct udev_list *list) {`
`117`	`117`	`list->uptodate = false;`
`118`	`118`	`hashmap_clear_with_destructor(list->unique_entries, udev_list_entry_free);`
`119`	`119`	`} else`
`120`		`- LIST_FOREACH_SAFE(entries, i, n, list->entries)`
	`120`	`+ LIST_FOREACH(entries, i, list->entries)`
`121`	`121`	`udev_list_entry_free(i);`
`122`	`122`	`}`
`123`	`123`
Original file line number	Diff line number	Diff line change
`@@ -144,7 +144,7 @@ static bool dns_cache_remove_by_key(DnsCache c, DnsResourceKey key) {`
`144`	`144`	`if (!first)`
`145`	`145`	`return false;`
`146`	`146`
`147`		`- LIST_FOREACH_SAFE(by_key, i, n, first) {`
	`147`	`+ LIST_FOREACH(by_key, i, first) {`
`148`	`148`	`prioq_remove(c->by_expiry, i, &i->prioq_idx);`
`149`	`149`	`dns_cache_item_free(i);`
`150`	`150`	`}`
Original file line number	Diff line number	Diff line change
`@@ -635,7 +635,7 @@ static int on_stream_complete(DnsStream *s, int error) {`
`635`	`635`	`}`
`636`	`636`
`637`	`637`	`if (error != 0)`
`638`		`- LIST_FOREACH_SAFE(transactions_by_stream, t, n, s->transactions)`
	`638`	`+ LIST_FOREACH(transactions_by_stream, t, s->transactions)`
`639`	`639`	`on_transaction_stream_error(t, error);`
`640`	`640`
`641`	`641`	`return 0;`