[ccan] [PATCH] list: add list_for_each_rev_safe

[Rusty Russell]

David Gibson <david at gibson.dropbear.id.au> writes:
> On Sun, Oct 05, 2014 at 12:20:45PM +0000, Eric Wong wrote:
>> Deleting while iterating backwards will be needed in the
>> Ruby st_foreach_reverse_check implementation if we decide
>> to port Ruby's st.c to use ccan/list.
>> 
>> ref: http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/65408
>> 
>> Signed-off-by: Eric Wong <normalperson at yhbt.net>
>> ---
>>  ccan/list/list.h     | 27 ++++++++++++++++++++++++++-
>>  ccan/list/test/run.c | 31 ++++++++++++++++++++++++++++++-
>>  2 files changed, 56 insertions(+), 2 deletions(-)
>> 
>> diff --git a/ccan/list/list.h b/ccan/list/list.h
>> index 983675b..b4a6cdf 100644
>> --- a/ccan/list/list.h
>> +++ b/ccan/list/list.h
>> @@ -527,6 +527,32 @@ static inline const void *list_tail_(const struct list_head *h, size_t off)
>>  	     i = container_of_var(i->member.prev, i, member))
>>  
>>  /**
>> + * list_for_each_rev_safe - iterate through a list backwards,
>> + * maybe during deletion
>> + * @h: the list_head
>> + * @i: the structure containing the list_node
>> + * @nxt: the structure containing the list_node
>> + * @member: the list_node member of the structure
>> + *
>> + * This is a convenient wrapper to iterate @i over the entire list backwards.
>> + * It's a for loop, so you can break and continue as normal.  The extra
>> + * variable * @nxt is used to hold the next element, so you can delete @i
>> + * from the list.
>> + *
>> + * Example:
>> + *	struct child *next;
>> + *	list_for_each_rev_safe(&parent->children, child, next, list) {
>> + *		printf("Name: %s\n", child->name);
>> + *	}
>> + */
>> +#define list_for_each_rev_safe(h, i, nxt, member)			\
>> +	for (i = container_of_var(list_debug(h,	LIST_LOC)->n.prev, i, member), \
>> +	     nxt = container_of_var(i->member.prev, i, member);		\
>> +	     &i->member != &(h)->n;					\
>> +	     i = nxt,							\
>> +	     nxt = container_of_var(i->member.prev, i, member))
>
>
> For proper orthogonality, I think rather than implementing it this
> way, you should implement list_for_each_rev_safe_off() and implement
> list_for_each_rev_safe() in terms of it, like the forwards
> list_for_each_safe().
>
> Also, I think the way that's implemented is to avoid double-evaluation
> of macro arguments, so it's possible there's such a double evaluation
> in your version.

Well, it matches list_for_each_rev().

I don't use the offset versions, so I'm happy to leave it to someone who
does to fix it.

And both variants will multi-evaluate h, so there's no win there.

Thanks!
Rusty.