On Fri, Jan 20, 2012 at 10:54:55AM +0300, Dan Carpenter wrote:
> The intent here was to do a logical && instead of a bitwise &. The
> original condition tests whether they have the some of same bits set.
> I have fixed that and rewritten it to be more clear.
>
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> Warning: This is a static analysis bug and I'm not very familiar with
> the code. Please review carefully.
>
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 0b4e2af..0c54027 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2376,8 +2376,8 @@ static int should_balance_chunk(struct btrfs_root *root,
> u64 chunk_type = btrfs_chunk_type(leaf, chunk);
>
> /* type filter */
> - if (!((chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) &
> - (bctl->flags & BTRFS_BALANCE_TYPE_MASK))) {
> + if (!(chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) ||
> + !(bctl->flags & BTRFS_BALANCE_TYPE_MASK)) {
> return 0;
> }
>
The intent here is definitely bitwise &. The original code tests
whether at least one of the bits set in (chunk_type & MASK) is set in
(bctl_flags & MASK), and if not returns 0. IOW,
u64 mask = chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK;
if (((bctl->flags & BTRFS_BALANCE_TYPE_MASK) & mask) == 0)
return 0;
Your patch does something completely different. However, I think we can
strengthen that check (and make it more idiomatic). Can you try the
below diff with your checker ?
Thanks,
Ilya
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 7ffdb15..9c6a074 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2375,12 +2375,11 @@ static int should_balance_chunk(struct btrfs_root *root,
struct btrfs_balance_control *bctl = root->fs_info->balance_ctl;
struct btrfs_balance_args *bargs = NULL;
u64 chunk_type = btrfs_chunk_type(leaf, chunk);
+ u64 mask = chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK;
/* type filter */
- if (!((chunk_type & BTRFS_BLOCK_GROUP_TYPE_MASK) &
- (bctl->flags & BTRFS_BALANCE_TYPE_MASK))) {
+ if (((bctl->flags & BTRFS_BALANCE_TYPE_MASK) & mask) != mask)
return 0;
- }
if (chunk_type & BTRFS_BLOCK_GROUP_DATA)
bargs = &bctl->data;
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
