* [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()
@ 2020-10-23 11:26 Dan Carpenter
2020-10-23 11:39 ` Qu Wenruo
2020-10-29 20:51 ` David Sterba
0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-10-23 11:26 UTC (permalink / raw)
To: Chris Mason, Qu Wenruo
Cc: Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()
2020-10-23 11:26 [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range() Dan Carpenter
@ 2020-10-23 11:39 ` Qu Wenruo
2020-10-29 20:51 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: Qu Wenruo @ 2020-10-23 11:39 UTC (permalink / raw)
To: Dan Carpenter, Chris Mason
Cc: Josef Bacik, David Sterba, linux-btrfs, kernel-janitors
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);
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range()
2020-10-23 11:26 [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range() Dan Carpenter
2020-10-23 11:39 ` Qu Wenruo
@ 2020-10-29 20:51 ` David Sterba
1 sibling, 0 replies; 3+ messages in thread
From: David Sterba @ 2020-10-29 20:51 UTC (permalink / raw)
To: Dan Carpenter
Cc: Chris Mason, Qu Wenruo, Josef Bacik, David Sterba, linux-btrfs,
kernel-janitors
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.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-10-29 20:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-23 11:26 [PATCH] btrfs: clean up NULL checks in qgroup_unreserve_range() Dan Carpenter
2020-10-23 11:39 ` Qu Wenruo
2020-10-29 20:51 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).