diff mbox series

[6/9] maple_tree: Remove a confusing check

Message ID 20230425110511.11680-7-zhangpeng.00@bytedance.com
State New
Headers
Series fix, rework and clean up for maple tree |

Commit Message

Peng Zhang April 25, 2023, 11:05 a.m. UTC 
  After this loop, we are at the last slot of a node. Our purpose is to
find an entry that is not NULL, but the pivot is checked here, delete
it, and change to mas_logical_pivot() to get the pivot. Finally, only
check whether the entry is NULL.

Why is this confusing? If the pivot is equal to 0, but if the entry is
not NULL at this time, it will return NULL because of the pivot, but it
should not do this, the entry is valid.

Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
---
 lib/maple_tree.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Liam R. Howlett April 25, 2023, 4:23 p.m. UTC | #1 
* Peng Zhang <zhangpeng.00@bytedance.com> [230425 07:05]:
> After this loop, we are at the last slot of a node. Our purpose is to
> find an entry that is not NULL, but the pivot is checked here, delete
> it, and change to mas_logical_pivot() to get the pivot. Finally, only
> check whether the entry is NULL.
> 
> Why is this confusing? If the pivot is equal to 0, but if the entry is
> not NULL at this time, it will return NULL because of the pivot, but it
> should not do this, the entry is valid.

Please avoid titles that don't say much at all.. and it's borderline
insulting. I could go into the reasons this was written this way, but
that would be a waste of time.. which is another reason to focus on what
you did in your commit message.

I've removed this function entirely in my patch set, so this isn't
needed.

> 
> Signed-off-by: Peng Zhang <zhangpeng.00@bytedance.com>
> ---
>  lib/maple_tree.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/lib/maple_tree.c b/lib/maple_tree.c
> index 7f4b2ce84ce61..83441ef2e1f57 100644
> --- a/lib/maple_tree.c
> +++ b/lib/maple_tree.c
> @@ -4742,17 +4742,13 @@ static inline void *mas_next_nentry(struct ma_state *mas,
>  		return NULL;
>  	}
>  
> -	pivot = mas_safe_pivot(mas, pivots, mas->offset, type);
> +	pivot = mas_logical_pivot(mas, pivots, mas->offset, type);
>  	entry = mas_slot(mas, slots, mas->offset);
>  	if (ma_dead_node(node))
>  		return NULL;
>  
> -	if (!pivot)
> -		return NULL;
> -
>  	if (!entry)
>  		return NULL;
> -
>  found:
>  	mas->last = pivot;
>  	return entry;
> -- 
> 2.20.1
>
diff mbox series

Patch

diff --git a/lib/maple_tree.c b/lib/maple_tree.c
index 7f4b2ce84ce61..83441ef2e1f57 100644
--- a/lib/maple_tree.c
+++ b/lib/maple_tree.c
@@ -4742,17 +4742,13 @@  static inline void *mas_next_nentry(struct ma_state *mas,
 		return NULL;
 	}
 
-	pivot = mas_safe_pivot(mas, pivots, mas->offset, type);
+	pivot = mas_logical_pivot(mas, pivots, mas->offset, type);
 	entry = mas_slot(mas, slots, mas->offset);
 	if (ma_dead_node(node))
 		return NULL;
 
-	if (!pivot)
-		return NULL;
-
 	if (!entry)
 		return NULL;
-
 found:
 	mas->last = pivot;
 	return entry;