2017-02-14 16:40 GMT+01:00 Jacek Caban jacek@codeweavers.com:
On 14.02.2017 16:21, Matteo Bruni wrote:
2017-02-13 21:12 GMT+01:00 Jacek Caban jacek@codeweavers.com:
Signed-off-by: Jacek Caban jacek@codeweavers.com
include/wine/rbtree.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)
These sound like the _SAFE variants of our list implementation. Any reason not to use the same naming here?
I thought about it. I think that there are different meanings of safety:
- safe to free current entry (but not necessarily safe to call
wine_rb_remove on it) 2. safe to cause tree mutation like call wine_rb_put or wine_rb_remove on any other element than current one 3. safe to remove current element (and possibly free it later)
The variant that I implemented is safe only as in (1), while current WINE_RB_FOR_EACH is safe as in (2).
By analogy, list _SAFE macros are safe as in (3) - you can remove current element, but generally shouldn't remove random other ones, because it may be the stored next entry. Also you need to remove element from the list before freeing it. I wanted to preserve that name if we ever find a need for macro safe as in (3) in rb tree. Its implementation for rb trees would look like the one I sent, except it would use regular order instead of postorder.
Those are valid points and I didn't appreciate all the nuance there. Yeah, I'd reserve _SAFE for (3) too and name this one differently (I don't have any better name to suggest either).
I guess my remaining question is whether it makes sense to just introduce the _SAFE variant right away, in place of this one. If I'm not mistaken that would be a bit slower since you'd need to wine_rb_remove() the elements one by one before freeing them but it might be more "natural" in a way, since it would match the semantics of the list. I'm not opposed to this patch in any case.