2023-03-08 07:13:35

by Jiapeng Chong

[permalink] [raw]
Subject: [PATCH] fs/locks: Remove redundant assignment to cmd

Variable 'cmd' set but not used.

fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.

Reported-by: Abaci Robot <[email protected]>
Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4439
Signed-off-by: Jiapeng Chong <[email protected]>
---
fs/locks.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index 66b4eef09db5..d82c4cacdfb9 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -2425,7 +2425,6 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
if (flock->l_pid != 0)
goto out;

- cmd = F_GETLK64;
fl->fl_flags |= FL_OFDLCK;
fl->fl_owner = filp;
}
--
2.20.1.7.g153144c



2023-03-09 01:01:46

by Chaitanya Kulkarni

[permalink] [raw]
Subject: Re: [PATCH] fs/locks: Remove redundant assignment to cmd

On 3/7/23 23:13, Jiapeng Chong wrote:
> Variable 'cmd' set but not used.
>
> fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.
>
> Reported-by: Abaci Robot <[email protected]>
> Link: https://bugzilla.openanolis.cn/show_bug.cgi?id=4439
> Signed-off-by: Jiapeng Chong <[email protected]>
> ---
> fs/locks.c | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/fs/locks.c b/fs/locks.c
> index 66b4eef09db5..d82c4cacdfb9 100644
> --- a/fs/locks.c
> +++ b/fs/locks.c
> @@ -2425,7 +2425,6 @@ int fcntl_getlk64(struct file *filp, unsigned int cmd, struct flock64 *flock)
> if (flock->l_pid != 0)
> goto out;
>
> - cmd = F_GETLK64;
> fl->fl_flags |= FL_OFDLCK;
> fl->fl_owner = filp;
> }
Indeed cmd is not used after this assignment in above function.

Looks good.

Reviewed-by: Chaitanya Kulkarni <[email protected]>

-ck


2023-03-09 09:26:12

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] fs/locks: Remove redundant assignment to cmd

From: Christian Brauner (Microsoft) <[email protected]>


On Wed, 08 Mar 2023 15:13:16 +0800, Jiapeng Chong wrote:
> Variable 'cmd' set but not used.
>
> fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.
>
>

Seems unused for quite a while. I've picked this up since there's a few other
trivial fixes I have pending. But I'm happy to drop this if you prefer this
goes via the lock tree, Jeff.

[1/1] fs/locks: Remove redundant assignment to cmd
commit: dc592190a5543c559010e09e8130a1af3f9068d3

2023-03-09 11:50:43

by Jeffrey Layton

[permalink] [raw]
Subject: Re: [PATCH] fs/locks: Remove redundant assignment to cmd

On Thu, 2023-03-09 at 10:25 +0100, Christian Brauner wrote:
> From: Christian Brauner (Microsoft) <[email protected]>
>
>
> On Wed, 08 Mar 2023 15:13:16 +0800, Jiapeng Chong wrote:
> > Variable 'cmd' set but not used.
> >
> > fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.
> >
> >
>
> Seems unused for quite a while. I've picked this up since there's a few other
> trivial fixes I have pending. But I'm happy to drop this if you prefer this
> goes via the lock tree, Jeff.
>
> [1/1] fs/locks: Remove redundant assignment to cmd
> commit: dc592190a5543c559010e09e8130a1af3f9068d3

Thanks Christian,

I had already picked it into the locks-next branch (though I didn't get
a chance to reply and mention that), but I'll drop it and plan to let
you carry it.

Thanks,
--
Jeff Layton <[email protected]>

2023-03-10 03:40:49

by Al Viro

[permalink] [raw]
Subject: Re: [PATCH] fs/locks: Remove redundant assignment to cmd

On Thu, Mar 09, 2023 at 06:50:15AM -0500, Jeff Layton wrote:
> On Thu, 2023-03-09 at 10:25 +0100, Christian Brauner wrote:
> > From: Christian Brauner (Microsoft) <[email protected]>
> >
> >
> > On Wed, 08 Mar 2023 15:13:16 +0800, Jiapeng Chong wrote:
> > > Variable 'cmd' set but not used.
> > >
> > > fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.
> > >
> > >
> >
> > Seems unused for quite a while. I've picked this up since there's a few other
> > trivial fixes I have pending. But I'm happy to drop this if you prefer this
> > goes via the lock tree, Jeff.
> >
> > [1/1] fs/locks: Remove redundant assignment to cmd
> > commit: dc592190a5543c559010e09e8130a1af3f9068d3
>
> Thanks Christian,
>
> I had already picked it into the locks-next branch (though I didn't get
> a chance to reply and mention that), but I'll drop it and plan to let
> you carry it.

IMO that's better off in locks tree; unless Christian is planning some
work in the area this cycle (I definitely do not), it's better to have
it in the branch in your git - we can always pull a branch from it if
needed and it's less likely to cause conflicts that way.

2023-03-10 09:00:22

by Christian Brauner

[permalink] [raw]
Subject: Re: [PATCH] fs/locks: Remove redundant assignment to cmd

On Fri, Mar 10, 2023 at 03:40:16AM +0000, Al Viro wrote:
> On Thu, Mar 09, 2023 at 06:50:15AM -0500, Jeff Layton wrote:
> > On Thu, 2023-03-09 at 10:25 +0100, Christian Brauner wrote:
> > > From: Christian Brauner (Microsoft) <[email protected]>
> > >
> > >
> > > On Wed, 08 Mar 2023 15:13:16 +0800, Jiapeng Chong wrote:
> > > > Variable 'cmd' set but not used.
> > > >
> > > > fs/locks.c:2428:3: warning: Value stored to 'cmd' is never read.
> > > >
> > > >
> > >
> > > Seems unused for quite a while. I've picked this up since there's a few other
> > > trivial fixes I have pending. But I'm happy to drop this if you prefer this
> > > goes via the lock tree, Jeff.
> > >
> > > [1/1] fs/locks: Remove redundant assignment to cmd
> > > commit: dc592190a5543c559010e09e8130a1af3f9068d3
> >
> > Thanks Christian,
> >
> > I had already picked it into the locks-next branch (though I didn't get
> > a chance to reply and mention that), but I'll drop it and plan to let
> > you carry it.
>
> IMO that's better off in locks tree; unless Christian is planning some
> work in the area this cycle (I definitely do not), it's better to have
> it in the branch in your git - we can always pull a branch from it if
> needed and it's less likely to cause conflicts that way.

I have picked up another fix for fs/locks.c for idmapped mounts so I
plan on sending that with some other smaller fixes soon. (And that
idmapped thing needs to be backported as well.)

So if there's non-trivial changes then I fully agree that the locking
tree is absolutely the right place. Especially since Jeff will also be
able to describe the changes properly and as you said we can always just
pull a branch to minimize conflicts.

However, when we have simple stuff like this let's just collect it in
our for *.misc trees and send it to Linus if we think enough stuff has
piled up (2+?). There's no point in holding fixes like this back for a
long time.

The likelihood of getting merge conflicts for smallish things that we
send weekly is rather low judging from my personal experience. Stephen
will tell us that quickly anyway. And even if we do get them Linus has
said repeatedly that he doesn't mind fixing them so we shouldn't be
fuzzed about it. Worst(/best?) case, everyone gets to watch my head
being torn off...