* [PATCH][next] bcachefs: remove redundant initialization of variable level
@ 2023-11-11 20:45 Colin Ian King
2023-11-11 21:02 ` Kent Overstreet
0 siblings, 1 reply; 7+ messages in thread
From: Colin Ian King @ 2023-11-11 20:45 UTC (permalink / raw)
To: Kent Overstreet, Brian Foster, linux-bcachefs
Cc: kernel-janitors, linux-kernel
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-11 20:45 [PATCH][next] bcachefs: remove redundant initialization of variable level Colin Ian King
@ 2023-11-11 21:02 ` Kent Overstreet
2023-11-11 21:19 ` David Laight
0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2023-11-11 21:02 UTC (permalink / raw)
To: Colin Ian King
Cc: Brian Foster, linux-bcachefs, kernel-janitors, linux-kernel
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];
^ permalink raw reply related [flat|nested] 7+ messages in thread
* RE: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-11 21:02 ` Kent Overstreet
@ 2023-11-11 21:19 ` David Laight
2023-11-11 23:39 ` Kent Overstreet
0 siblings, 1 reply; 7+ messages in thread
From: David Laight @ 2023-11-11 21:19 UTC (permalink / raw)
To: 'Kent Overstreet', Colin Ian King
Cc: Brian Foster, linux-bcachefs, kernel-janitors, linux-kernel
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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-11 21:19 ` David Laight
@ 2023-11-11 23:39 ` Kent Overstreet
2023-11-12 18:34 ` David Laight
0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2023-11-11 23:39 UTC (permalink / raw)
To: David Laight
Cc: Colin Ian King, Brian Foster, linux-bcachefs, kernel-janitors,
linux-kernel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-11 23:39 ` Kent Overstreet
@ 2023-11-12 18:34 ` David Laight
2023-11-12 19:12 ` Kent Overstreet
0 siblings, 1 reply; 7+ messages in thread
From: David Laight @ 2023-11-12 18:34 UTC (permalink / raw)
To: 'Kent Overstreet'
Cc: Colin Ian King, Brian Foster, linux-bcachefs, kernel-janitors,
linux-kernel
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)
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-12 18:34 ` David Laight
@ 2023-11-12 19:12 ` Kent Overstreet
2023-11-20 12:00 ` Dan Carpenter
0 siblings, 1 reply; 7+ messages in thread
From: Kent Overstreet @ 2023-11-12 19:12 UTC (permalink / raw)
To: David Laight
Cc: Colin Ian King, Brian Foster, linux-bcachefs, kernel-janitors,
linux-kernel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH][next] bcachefs: remove redundant initialization of variable level
2023-11-12 19:12 ` Kent Overstreet
@ 2023-11-20 12:00 ` Dan Carpenter
0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2023-11-20 12:00 UTC (permalink / raw)
To: Kent Overstreet
Cc: David Laight, Colin Ian King, Brian Foster, linux-bcachefs,
kernel-janitors, linux-kernel
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-11-20 12:00 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-11 20:45 [PATCH][next] bcachefs: remove redundant initialization of variable level Colin Ian King
2023-11-11 21:02 ` Kent Overstreet
2023-11-11 21:19 ` David Laight
2023-11-11 23:39 ` Kent Overstreet
2023-11-12 18:34 ` David Laight
2023-11-12 19:12 ` Kent Overstreet
2023-11-20 12:00 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.