[PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()

[PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()

[Dan Carpenter]

Smatch complains that this code dereferences "entry" before checking
whether it's NULL on the next line.  Fortunately, rb_entry() will never
return NULL so it doesn't cause a problem.  We can clean up the NULL
checking a bit to silence the warning and make the code more clear.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
My patch does not change run-time, but it's possible that the original
code was buggy and I missed it.

 fs/btrfs/qgroup.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
index 580899bdb991..a7ae2f18f486 100644
--- a/fs/btrfs/qgroup.c
+++ b/fs/btrfs/qgroup.c
@@ -3417,24 +3417,20 @@ static int qgroup_unreserve_range(struct btrfs_inode *inode,
 {
 	struct rb_node *node;
 	struct rb_node *next;
-	struct ulist_node *entry = NULL;
+	struct ulist_node *entry;
 	int ret = 0;
 
 	node = reserved->range_changed.root.rb_node;
+	if (!node)
+		return 0;
 	while (node) {
 		entry = rb_entry(node, struct ulist_node, rb_node);
 		if (entry->val < start)
 			node = node->rb_right;
-		else if (entry)
-			node = node->rb_left;
 		else
-			break;
+			node = node->rb_left;
 	}
 
-	/* Empty changeset */
-	if (!entry)
-		return 0;
-
 	if (entry->val > start && rb_prev(&entry->rb_node))
 		entry = rb_entry(rb_prev(&entry->rb_node), struct ulist_node,
 				 rb_node);
-- 
2.28.0

Re: [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()

[Qu Wenruo]

On 2020/10/23 下午7:26, Dan Carpenter wrote:
> Smatch complains that this code dereferences "entry" before checking
> whether it's NULL on the next line.  Fortunately, rb_entry() will never
> return NULL so it doesn't cause a problem.  We can clean up the NULL
> checking a bit to silence the warning and make the code more clear.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> My patch does not change run-time, but it's possible that the original
> code was buggy and I missed it.
> 
>  fs/btrfs/qgroup.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/btrfs/qgroup.c b/fs/btrfs/qgroup.c
> index 580899bdb991..a7ae2f18f486 100644
> --- a/fs/btrfs/qgroup.c
> +++ b/fs/btrfs/qgroup.c
> @@ -3417,24 +3417,20 @@ static int qgroup_unreserve_range(struct btrfs_inode *inode,
>  {
>  	struct rb_node *node;
>  	struct rb_node *next;
> -	struct ulist_node *entry = NULL;
> +	struct ulist_node *entry;
>  	int ret = 0;
>  
>  	node = reserved->range_changed.root.rb_node;
> +	if (!node)
> +		return 0;
>  	while (node) {
>  		entry = rb_entry(node, struct ulist_node, rb_node);
>  		if (entry->val < start)
>  			node = node->rb_right;
> -		else if (entry)
> -			node = node->rb_left;
>  		else
> -			break;
> +			node = node->rb_left;
>  	}
Indeed, the new result does the same work, by going to the nearest node
of @start.

New code looks like:

while (node) {
	if (entry->val < start)
		node = node->rb_right;
	else
		node = node->rb_left;
}

is good enough and removes the dead break branch.

Looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu
>  
> -	/* Empty changeset */
> -	if (!entry)
> -		return 0;
> -
>  	if (entry->val > start && rb_prev(&entry->rb_node))
>  		entry = rb_entry(rb_prev(&entry->rb_node), struct ulist_node,
>  				 rb_node);
> 

Re: [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()

[David Sterba]

On Fri, Oct 23, 2020 at 02:26:33PM +0300, Dan Carpenter wrote:
> Smatch complains that this code dereferences "entry" before checking
> whether it's NULL on the next line.  Fortunately, rb_entry() will never
> return NULL so it doesn't cause a problem.  We can clean up the NULL
> checking a bit to silence the warning and make the code more clear.
> 
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Added to misc-next, thanks.