bcachefs: remove redundant initialization of variable level

[PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by Colin Ian King 2 years, 1 month ago

Variable level is being initialized a value that is never read, the
variable is being re-assigned another value several statements later
on. The initialization is redundant and can be removed. Cleans up
clang scan build warning:

fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
during its initialization is never read [deadcode.DeadStores]

Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
---
 fs/bcachefs/btree_iter.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index c2adf3fbb0b3..bd04aeda37bb 100644
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -1214,7 +1214,7 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
 		   struct btree_path *path, struct bpos new_pos,
 		   bool intent, unsigned long ip, int cmp)
 {
-	unsigned level = path->level;
+	unsigned level;
 
 	bch2_trans_verify_not_in_restart(trans);
 	EBUG_ON(!path->ref);
-- 
2.39.2

Re: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by Kent Overstreet 2 years, 1 month ago

On Sat, Nov 11, 2023 at 08:45:28PM +0000, Colin Ian King wrote:
> Variable level is being initialized a value that is never read, the
> variable is being re-assigned another value several statements later
> on. The initialization is redundant and can be removed. Cleans up
> clang scan build warning:
> 
> fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
> during its initialization is never read [deadcode.DeadStores]
> 
> Signed-off-by: Colin Ian King <colin.i.king@gmail.com>

since we're no longer gnu89, we can simply declare the variable when
it's first used, like so:

diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
index 96bdf0c6051c..104172f6822b 100644
--- a/fs/bcachefs/btree_iter.c
+++ b/fs/bcachefs/btree_iter.c
@@ -1214,8 +1214,6 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
 		   struct btree_path *path, struct bpos new_pos,
 		   bool intent, unsigned long ip, int cmp)
 {
-	unsigned level = path->level;
-
 	bch2_trans_verify_not_in_restart(trans);
 	EBUG_ON(!path->ref);
 
@@ -1231,7 +1229,7 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
 		goto out;
 	}
 
-	level = btree_path_up_until_good_node(trans, path, cmp);
+	unsigned level = btree_path_up_until_good_node(trans, path, cmp);
 
 	if (btree_path_node(path, level)) {
 		struct btree_path_level *l = &path->l[level];

RE: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by David Laight 2 years, 1 month ago

From: Kent Overstreet <kent.overstreet@linux.dev>
> Sent: 11 November 2023 21:02
> > Variable level is being initialized a value that is never read, the
> > variable is being re-assigned another value several statements later
> > on. The initialization is redundant and can be removed. Cleans up
> > clang scan build warning:
> >
> > fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
> > during its initialization is never read [deadcode.DeadStores]
> >
> > Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> 
> since we're no longer gnu89, we can simply declare the variable when
> it's first used, like so:

ugg... I think that is still frowned upon.
It makes it very difficult for the average human to find
the variable declaration.

	David

> 
> diff --git a/fs/bcachefs/btree_iter.c b/fs/bcachefs/btree_iter.c
> index 96bdf0c6051c..104172f6822b 100644
> --- a/fs/bcachefs/btree_iter.c
> +++ b/fs/bcachefs/btree_iter.c
> @@ -1214,8 +1214,6 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
>  		   struct btree_path *path, struct bpos new_pos,
>  		   bool intent, unsigned long ip, int cmp)
>  {
> -	unsigned level = path->level;
> -
>  	bch2_trans_verify_not_in_restart(trans);
>  	EBUG_ON(!path->ref);
> 
> @@ -1231,7 +1229,7 @@ __bch2_btree_path_set_pos(struct btree_trans *trans,
>  		goto out;
>  	}
> 
> -	level = btree_path_up_until_good_node(trans, path, cmp);
> +	unsigned level = btree_path_up_until_good_node(trans, path, cmp);
> 
>  	if (btree_path_node(path, level)) {
>  		struct btree_path_level *l = &path->l[level];

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by Kent Overstreet 2 years, 1 month ago

On Sat, Nov 11, 2023 at 09:19:40PM +0000, David Laight wrote:
> From: Kent Overstreet <kent.overstreet@linux.dev>
> > Sent: 11 November 2023 21:02
> > > Variable level is being initialized a value that is never read, the
> > > variable is being re-assigned another value several statements later
> > > on. The initialization is redundant and can be removed. Cleans up
> > > clang scan build warning:
> > >
> > > fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
> > > during its initialization is never read [deadcode.DeadStores]
> > >
> > > Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> > 
> > since we're no longer gnu89, we can simply declare the variable when
> > it's first used, like so:
> 
> ugg... I think that is still frowned upon.
> It makes it very difficult for the average human to find
> the variable declaration.

No, it's 2023, there's no good reason to be declaring variables before
giving them values.

RE: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by David Laight 2 years, 1 month ago

From: Kent Overstreet
> Sent: 11 November 2023 23:39
> 
> On Sat, Nov 11, 2023 at 09:19:40PM +0000, David Laight wrote:
> > From: Kent Overstreet <kent.overstreet@linux.dev>
> > > Sent: 11 November 2023 21:02
> > > > Variable level is being initialized a value that is never read, the
> > > > variable is being re-assigned another value several statements later
> > > > on. The initialization is redundant and can be removed. Cleans up
> > > > clang scan build warning:
> > > >
> > > > fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
> > > > during its initialization is never read [deadcode.DeadStores]
> > > >
> > > > Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> > >
> > > since we're no longer gnu89, we can simply declare the variable when
> > > it's first used, like so:
> >
> > ugg... I think that is still frowned upon.
> > It makes it very difficult for the average human to find
> > the variable declaration.
> 
> No, it's 2023, there's no good reason to be declaring variables before
> giving them values.

The year has nothing to do with whether it is a good idea.
It is epically bad without -Wshadow.
(Have you ever played 'stop the declaration' in C++, it isn't fun.)

Finding declarations is bad enough when they are at the top
of a big block, never mind in the middle of a load of assignments.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

Re: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by Kent Overstreet 2 years, 1 month ago

On Sun, Nov 12, 2023 at 06:34:59PM +0000, David Laight wrote:
> From: Kent Overstreet
> > Sent: 11 November 2023 23:39
> > 
> > On Sat, Nov 11, 2023 at 09:19:40PM +0000, David Laight wrote:
> > > From: Kent Overstreet <kent.overstreet@linux.dev>
> > > > Sent: 11 November 2023 21:02
> > > > > Variable level is being initialized a value that is never read, the
> > > > > variable is being re-assigned another value several statements later
> > > > > on. The initialization is redundant and can be removed. Cleans up
> > > > > clang scan build warning:
> > > > >
> > > > > fs/bcachefs/btree_iter.c:1217:11: warning: Value stored to 'level'
> > > > > during its initialization is never read [deadcode.DeadStores]
> > > > >
> > > > > Signed-off-by: Colin Ian King <colin.i.king@gmail.com>
> > > >
> > > > since we're no longer gnu89, we can simply declare the variable when
> > > > it's first used, like so:
> > >
> > > ugg... I think that is still frowned upon.
> > > It makes it very difficult for the average human to find
> > > the variable declaration.
> > 
> > No, it's 2023, there's no good reason to be declaring variables before
> > giving them values.
> 
> The year has nothing to do with whether it is a good idea.
> It is epically bad without -Wshadow.
> (Have you ever played 'stop the declaration' in C++, it isn't fun.)
> 
> Finding declarations is bad enough when they are at the top
> of a big block, never mind in the middle of a load of assignments.

David, I don't want you giving this kind of advice here, and if finding
declarations is something you have trouble with - perhaps find something
easier to do.

Re: [PATCH][next] bcachefs: remove redundant initialization of variable level

Posted by Dan Carpenter 2 years, 1 month ago

On Sun, Nov 12, 2023 at 02:12:49PM -0500, Kent Overstreet wrote:
> David, I don't want you giving this kind of advice here, and if finding
> declarations is something you have trouble with - perhaps find something
> easier to do.

David is correct.  Putting declarations in the middle of code is still
frowned on.  It's necessary for the __cleanup work and it's okay in for
loop iterators but it's generally frowned on.

Please don't force people to redo patches in non-standard style.

regards,
dan carpenter