| Message ID | 1621244810-38832-1-git-send-email-yang.lee@linux.alibaba.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] btrfs: Remove redundant initialization of 'to_add' | expand |
On 2021/5/17 下午5:46, Yang Li wrote: > Variable 'to_add' is being initialized however this value is never > read as 'to_add' is assigned a new value in if statement. Remove the > redundant assignment. At the same time, move its declaration into the > if statement, because the variable is not used elsewhere. > > Clean up clang warning: > > fs/btrfs/extent-tree.c:2774:8: warning: Value stored to 'to_add' during > its initialization is never read [clang-analyzer-deadcode.DeadStores] Personally speaking, compiler should be able to optimize out such problem, nothing really worthy bothering. Especially considering these "fixes" just randomly pop up, distracting the reviewers' time. If you really believe these "fixes" are really worthy (not to just fulfill the stupid KPI), please at least pack them into a larger patchset (but keep the separate patches), not just sending one when you find one. > > Reported-by: Abaci Robot <abaci@linux.alibaba.com> I know some maintainers are already very upset about the bot, although in your case it reduces a lifespan of a variable, thus it's marginally acceptable, but under other cases, it doesn't really help much. If such fixes come from indie developers, I'm pretty fine or even happy to help them to start more contribution. But a sponsored bot just repeating clang static analyzer Trust me, no maintainer will be happy with that, and you're destroying the reputation of your company (if the reputation hasn't been destoryed already). > Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> > --- > > Change in v2: > --According to Lukas's suggestion, combine the declaration and assignment of > variable 'to_add' into one line, just as "u64 to_add = min(len, ...);" > https://lore.kernel.org/patchwork/patch/1428697/ > > fs/btrfs/extent-tree.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) > > diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c > index f1d15b6..13ac978 100644 > --- a/fs/btrfs/extent-tree.c > +++ b/fs/btrfs/extent-tree.c > @@ -2774,11 +2774,9 @@ static int unpin_extent_range(struct btrfs_fs_info *fs_info, > spin_unlock(&cache->lock); > if (!readonly && return_free_space && > global_rsv->space_info == space_info) { > - u64 to_add = len; > - > spin_lock(&global_rsv->lock); > if (!global_rsv->full) { > - to_add = min(len, global_rsv->size - > + u64 to_add = min(len, global_rsv->size - > global_rsv->reserved); Have you ever wondered why "global_rsv" is not indented by tab only, but with extra spaces? It's supposed to be aligned with "len". Thanks, Qu > global_rsv->reserved += to_add; > btrfs_space_info_update_bytes_may_use(fs_info, >
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c index f1d15b6..13ac978 100644 --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -2774,11 +2774,9 @@ static int unpin_extent_range(struct btrfs_fs_info *fs_info, spin_unlock(&cache->lock); if (!readonly && return_free_space && global_rsv->space_info == space_info) { - u64 to_add = len; - spin_lock(&global_rsv->lock); if (!global_rsv->full) { - to_add = min(len, global_rsv->size - + u64 to_add = min(len, global_rsv->size - global_rsv->reserved); global_rsv->reserved += to_add; btrfs_space_info_update_bytes_may_use(fs_info,
Variable 'to_add' is being initialized however this value is never read as 'to_add' is assigned a new value in if statement. Remove the redundant assignment. At the same time, move its declaration into the if statement, because the variable is not used elsewhere. Clean up clang warning: fs/btrfs/extent-tree.c:2774:8: warning: Value stored to 'to_add' during its initialization is never read [clang-analyzer-deadcode.DeadStores] Reported-by: Abaci Robot <abaci@linux.alibaba.com> Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> --- Change in v2: --According to Lukas's suggestion, combine the declaration and assignment of variable 'to_add' into one line, just as "u64 to_add = min(len, ...);" https://lore.kernel.org/patchwork/patch/1428697/ fs/btrfs/extent-tree.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-)