diff --git a/src/backend/lib/ilist.c b/src/backend/lib/ilist.c index 957a1181227..eb23fa1c687 100644 --- a/src/backend/lib/ilist.c +++ b/src/backend/lib/ilist.c @@ -28,7 +28,7 @@ * * It is not allowed to delete a 'node' which is is not in the list 'head' * - * Caution: this is O(n) + * Caution: this is O(n); consider using slist_delete_current() instead. */ void slist_delete(slist_head *head, slist_node *node) diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c index ac20dffd988..dac5bca78aa 100644 --- a/src/backend/postmaster/pgstat.c +++ b/src/backend/postmaster/pgstat.c @@ -3598,6 +3598,13 @@ pgstat_write_statsfiles(bool permanent, bool allDbs) { slist_mutable_iter iter; + /* + * Strictly speaking we should do slist_delete_current() before + * freeing each request struct. We skip that and instead + * re-initialize the list header at the end. Nonetheless, we must use + * slist_foreach_modify, not just slist_foreach, since we will free + * the node's storage before advancing. + */ slist_foreach_modify(iter, &last_statrequests) { DBWriteRequest *req; diff --git a/src/include/lib/ilist.h b/src/include/lib/ilist.h index 73f41159a6c..734ef70aa1c 100644 --- a/src/include/lib/ilist.h +++ b/src/include/lib/ilist.h @@ -211,9 +211,14 @@ typedef struct slist_head * Used as state in slist_foreach(). To get the current element of the * iteration use the 'cur' member. * - * Do *not* manipulate the list while iterating! + * It's allowed to modify the list while iterating, with the exception of + * deleting the iterator's current node; deletion of that node requires + * care if the iteration is to be continued afterward. (Doing so and also + * deleting or inserting adjacent list elements might misbehave; also, if + * the user frees the current node's storage, continuing the iteration is + * not safe.) * - * NB: this wouldn't really need to be an extra struct, we could use a + * NB: this wouldn't really need to be an extra struct, we could use an * slist_node * directly. We prefer a separate type for consistency. */ typedef struct slist_iter @@ -224,15 +229,18 @@ typedef struct slist_iter /* * Singly linked list iterator allowing some modifications while iterating. * - * Used as state in slist_foreach_modify(). + * Used as state in slist_foreach_modify(). To get the current element of the + * iteration use the 'cur' member. * - * Iterations using this are allowed to remove the current node and to add - * more nodes ahead of the current node. + * The only list modification allowed while iterating is to remove the current + * node via slist_delete_current() (*not* slist_delete()). Insertion or + * deletion of nodes adjacent to the current node would misbehave. */ typedef struct slist_mutable_iter { slist_node *cur; /* current element */ slist_node *next; /* next node we'll iterate to */ + slist_node *prev; /* prev node, for deletions */ } slist_mutable_iter; @@ -243,7 +251,7 @@ typedef struct slist_mutable_iter /* Prototypes for functions too big to be inline */ -/* Caution: this is O(n) */ +/* Caution: this is O(n); consider using slist_delete_current() instead */ extern void slist_delete(slist_head *head, slist_node *node); #ifdef ILIST_DEBUG @@ -578,6 +586,7 @@ extern slist_node *slist_pop_head_node(slist_head *head); extern bool slist_has_next(slist_head *head, slist_node *node); extern slist_node *slist_next_node(slist_head *head, slist_node *node); extern slist_node *slist_head_node(slist_head *head); +extern void slist_delete_current(slist_mutable_iter *iter); /* slist macro support function */ extern void *slist_head_element_off(slist_head *head, size_t off); @@ -679,6 +688,29 @@ slist_head_node(slist_head *head) { return (slist_node *) slist_head_element_off(head, 0); } + +/* + * Delete the list element the iterator currently points to. + * + * Caution: this modifies iter->cur, so don't use that again in the current + * loop iteration. + */ +STATIC_IF_INLINE void +slist_delete_current(slist_mutable_iter *iter) +{ + /* + * Update previous element's forward link. If the iteration is at the + * first list element, iter->prev will point to the list header's "head" + * field, so we don't need a special case for that. + */ + iter->prev->next = iter->next; + + /* + * Reset cur to prev, so that prev will continue to point to the prior + * valid list element after slist_foreach_modify() advances to the next. + */ + iter->cur = iter->prev; +} #endif /* PG_USE_INLINE || ILIST_INCLUDE_DEFINITIONS */ /* @@ -706,7 +738,12 @@ slist_head_node(slist_head *head) * * Access the current element with iter.cur. * - * It is *not* allowed to manipulate the list during iteration. + * It's allowed to modify the list while iterating, with the exception of + * deleting the iterator's current node; deletion of that node requires + * care if the iteration is to be continued afterward. (Doing so and also + * deleting or inserting adjacent list elements might misbehave; also, if + * the user frees the current node's storage, continuing the iteration is + * not safe.) */ #define slist_foreach(iter, lhead) \ for (AssertVariableIsOfTypeMacro(iter, slist_iter), \ @@ -720,15 +757,18 @@ slist_head_node(slist_head *head) * * Access the current element with iter.cur. * - * Iterations using this are allowed to remove the current node and to add - * more nodes ahead of the current node. + * The only list modification allowed while iterating is to remove the current + * node via slist_delete_current() (*not* slist_delete()). Insertion or + * deletion of nodes adjacent to the current node would misbehave. */ #define slist_foreach_modify(iter, lhead) \ for (AssertVariableIsOfTypeMacro(iter, slist_mutable_iter), \ AssertVariableIsOfTypeMacro(lhead, slist_head *), \ - (iter).cur = (lhead)->head.next, \ + (iter).prev = &(lhead)->head, \ + (iter).cur = (iter).prev->next, \ (iter).next = (iter).cur ? (iter).cur->next : NULL; \ (iter).cur != NULL; \ + (iter).prev = (iter).cur, \ (iter).cur = (iter).next, \ (iter).next = (iter).next ? (iter).next->next : NULL)