* [PATCH] debugfs: remove unused variable
@ 2016-09-20 16:17 Eric Engestrom
2016-09-21 8:01 ` Greg Kroah-Hartman
0 siblings, 1 reply; 8+ messages in thread
From: Eric Engestrom @ 2016-09-20 16:17 UTC (permalink / raw)
To: linux-kernel; +Cc: Eric Engestrom, Greg Kroah-Hartman
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
fs/debugfs/file.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 592059f..04eca0b 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -195,7 +195,6 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
const struct dentry *dentry = F_DENTRY(filp);
const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
const struct file_operations *proxy_fops = filp->f_op;
- int r = 0;
/*
* We must not protect this against removal races here: the
@@ -204,7 +203,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
* ->i_private is still being meaningful here.
*/
if (real_fops->release)
- r = real_fops->release(inode, filp);
+ real_fops->release(inode, filp);
replace_fops(filp, d_inode(dentry)->i_fop);
kfree((void *)proxy_fops);
--
Cheers,
Eric
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] debugfs: remove unused variable
2016-09-20 16:17 [PATCH] debugfs: remove unused variable Eric Engestrom
@ 2016-09-21 8:01 ` Greg Kroah-Hartman
2016-09-21 8:36 ` [PATCH] debugfs: propagate release() call result Eric Engestrom
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Greg Kroah-Hartman @ 2016-09-21 8:01 UTC (permalink / raw)
To: Eric Engestrom; +Cc: linux-kernel
On Tue, Sep 20, 2016 at 05:17:15PM +0100, Eric Engestrom wrote:
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> ---
> fs/debugfs/file.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> index 592059f..04eca0b 100644
> --- a/fs/debugfs/file.c
> +++ b/fs/debugfs/file.c
> @@ -195,7 +195,6 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> const struct dentry *dentry = F_DENTRY(filp);
> const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
> const struct file_operations *proxy_fops = filp->f_op;
> - int r = 0;
>
> /*
> * We must not protect this against removal races here: the
> @@ -204,7 +203,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> * ->i_private is still being meaningful here.
> */
> if (real_fops->release)
> - r = real_fops->release(inode, filp);
> + real_fops->release(inode, filp);
Hm, shouldn't we be propagating the result back up the call chain?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] debugfs: propagate release() call result
2016-09-21 8:01 ` Greg Kroah-Hartman
@ 2016-09-21 8:36 ` Eric Engestrom
2016-09-21 9:15 ` Greg Kroah-Hartman
2016-09-21 8:39 ` [PATCH] debugfs: remove unused variable Eric Engestrom
2016-09-21 8:50 ` Nicolai Stange
2 siblings, 1 reply; 8+ messages in thread
From: Eric Engestrom @ 2016-09-21 8:36 UTC (permalink / raw)
To: linux-kernel; +Cc: Eric Engestrom, Greg Kroah-Hartman
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
fs/debugfs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 592059f..40e586f 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -209,7 +209,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
replace_fops(filp, d_inode(dentry)->i_fop);
kfree((void *)proxy_fops);
fops_put(real_fops);
- return 0;
+ return r;
}
static void __full_proxy_fops_init(struct file_operations *proxy_fops,
--
Cheers,
Eric
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] debugfs: remove unused variable
2016-09-21 8:01 ` Greg Kroah-Hartman
2016-09-21 8:36 ` [PATCH] debugfs: propagate release() call result Eric Engestrom
@ 2016-09-21 8:39 ` Eric Engestrom
2016-09-21 8:50 ` Nicolai Stange
2 siblings, 0 replies; 8+ messages in thread
From: Eric Engestrom @ 2016-09-21 8:39 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel
On Wed, Sep 21, 2016 at 10:01:11AM +0200, Greg Kroah-Hartman wrote:
> On Tue, Sep 20, 2016 at 05:17:15PM +0100, Eric Engestrom wrote:
> > Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> > ---
> > fs/debugfs/file.c | 3 +--
> > 1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> > index 592059f..04eca0b 100644
> > --- a/fs/debugfs/file.c
> > +++ b/fs/debugfs/file.c
> > @@ -195,7 +195,6 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> > const struct dentry *dentry = F_DENTRY(filp);
> > const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
> > const struct file_operations *proxy_fops = filp->f_op;
> > - int r = 0;
> >
> > /*
> > * We must not protect this against removal races here: the
> > @@ -204,7 +203,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> > * ->i_private is still being meaningful here.
> > */
> > if (real_fops->release)
> > - r = real_fops->release(inode, filp);
> > + real_fops->release(inode, filp);
>
> Hm, shouldn't we be propagating the result back up the call chain?
You're right, sorry, I wasn't thinking. Correct fix incoming :)
Cheers,
Eric
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] debugfs: remove unused variable
2016-09-21 8:01 ` Greg Kroah-Hartman
2016-09-21 8:36 ` [PATCH] debugfs: propagate release() call result Eric Engestrom
2016-09-21 8:39 ` [PATCH] debugfs: remove unused variable Eric Engestrom
@ 2016-09-21 8:50 ` Nicolai Stange
2016-09-21 9:23 ` Eric Engestrom
2 siblings, 1 reply; 8+ messages in thread
From: Nicolai Stange @ 2016-09-21 8:50 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: Nicolai Stange, Eric Engestrom, linux-kernel
Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
> On Tue, Sep 20, 2016 at 05:17:15PM +0100, Eric Engestrom wrote:
>> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
>> ---
>> fs/debugfs/file.c | 3 +--
>> 1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
>> index 592059f..04eca0b 100644
>> --- a/fs/debugfs/file.c
>> +++ b/fs/debugfs/file.c
>> @@ -195,7 +195,6 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
>> const struct dentry *dentry = F_DENTRY(filp);
>> const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
>> const struct file_operations *proxy_fops = filp->f_op;
>> - int r = 0;
>>
>> /*
>> * We must not protect this against removal races here: the
>> @@ -204,7 +203,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
>> * ->i_private is still being meaningful here.
>> */
>> if (real_fops->release)
>> - r = real_fops->release(inode, filp);
>> + real_fops->release(inode, filp);
>
> Hm, shouldn't we be propagating the result back up the call chain?
AFAICS, the VFS layer doesn't ever evaluate the return value of
->release(), c.f. __fput() in fs/file_table.c .
OTOH, propagating that value back to caller also wouldn't hurt. But this
would be a matter of taste/coding style.
I can't remember whether I left this unused int r there on purpose. I
doubt not. Eric, did you run your patch through sparse and Coccinelle?
If so,
Reviewed-by: Nicolai Stange <nicstange@gmail.com>
for the diff. (This patch lacks a description though.)
Thanks,
Nicolai
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] debugfs: propagate release() call result
2016-09-21 8:36 ` [PATCH] debugfs: propagate release() call result Eric Engestrom
@ 2016-09-21 9:15 ` Greg Kroah-Hartman
2016-09-21 9:27 ` [PATCH v2] " Eric Engestrom
0 siblings, 1 reply; 8+ messages in thread
From: Greg Kroah-Hartman @ 2016-09-21 9:15 UTC (permalink / raw)
To: Eric Engestrom; +Cc: linux-kernel
On Wed, Sep 21, 2016 at 09:36:53AM +0100, Eric Engestrom wrote:
> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
Oops, I can't take patches without any changelog text at all.
Please fix up and resend.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] debugfs: remove unused variable
2016-09-21 8:50 ` Nicolai Stange
@ 2016-09-21 9:23 ` Eric Engestrom
0 siblings, 0 replies; 8+ messages in thread
From: Eric Engestrom @ 2016-09-21 9:23 UTC (permalink / raw)
To: Nicolai Stange; +Cc: Greg Kroah-Hartman, linux-kernel
On Wed, Sep 21, 2016 at 10:50:38AM +0200, Nicolai Stange wrote:
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> writes:
>
> > On Tue, Sep 20, 2016 at 05:17:15PM +0100, Eric Engestrom wrote:
> >> Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
> >> ---
> >> fs/debugfs/file.c | 3 +--
> >> 1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
> >> index 592059f..04eca0b 100644
> >> --- a/fs/debugfs/file.c
> >> +++ b/fs/debugfs/file.c
> >> @@ -195,7 +195,6 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> >> const struct dentry *dentry = F_DENTRY(filp);
> >> const struct file_operations *real_fops = REAL_FOPS_DEREF(dentry);
> >> const struct file_operations *proxy_fops = filp->f_op;
> >> - int r = 0;
> >>
> >> /*
> >> * We must not protect this against removal races here: the
> >> @@ -204,7 +203,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
> >> * ->i_private is still being meaningful here.
> >> */
> >> if (real_fops->release)
> >> - r = real_fops->release(inode, filp);
> >> + real_fops->release(inode, filp);
> >
> > Hm, shouldn't we be propagating the result back up the call chain?
>
> AFAICS, the VFS layer doesn't ever evaluate the return value of
> ->release(), c.f. __fput() in fs/file_table.c .
>
> OTOH, propagating that value back to caller also wouldn't hurt. But this
> would be a matter of taste/coding style.
I actually sent an updated fix [1] about an hour ago, which propagates
the result instead (which is better IMO, I don't know why I didn't do
that the first time around).
[1] http://marc.info/?m=147444718118891 (lkml.org is down?)
>
> I can't remember whether I left this unused int r there on purpose. I
> doubt not. Eric, did you run your patch through sparse and Coccinelle?
I didn't; how do I do that? I know these tools, but not how to use them
in this context.
Cheers,
Eric
>
> If so,
>
> Reviewed-by: Nicolai Stange <nicstange@gmail.com>
>
> for the diff. (This patch lacks a description though.)
>
>
> Thanks,
>
> Nicolai
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] debugfs: propagate release() call result
2016-09-21 9:15 ` Greg Kroah-Hartman
@ 2016-09-21 9:27 ` Eric Engestrom
0 siblings, 0 replies; 8+ messages in thread
From: Eric Engestrom @ 2016-09-21 9:27 UTC (permalink / raw)
To: linux-kernel; +Cc: Nicolai Stange, Eric Engestrom, Greg Kroah-Hartman
The result was being ignored and 0 was always returned.
Return the actual result instead.
Signed-off-by: Eric Engestrom <eric.engestrom@imgtec.com>
---
v2: add a commit message
---
fs/debugfs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c
index 592059f..40e586f 100644
--- a/fs/debugfs/file.c
+++ b/fs/debugfs/file.c
@@ -209,7 +209,7 @@ static int full_proxy_release(struct inode *inode, struct file *filp)
replace_fops(filp, d_inode(dentry)->i_fop);
kfree((void *)proxy_fops);
fops_put(real_fops);
- return 0;
+ return r;
}
static void __full_proxy_fops_init(struct file_operations *proxy_fops,
--
Cheers,
Eric
^ permalink raw reply related [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-21 9:28 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-20 16:17 [PATCH] debugfs: remove unused variable Eric Engestrom
2016-09-21 8:01 ` Greg Kroah-Hartman
2016-09-21 8:36 ` [PATCH] debugfs: propagate release() call result Eric Engestrom
2016-09-21 9:15 ` Greg Kroah-Hartman
2016-09-21 9:27 ` [PATCH v2] " Eric Engestrom
2016-09-21 8:39 ` [PATCH] debugfs: remove unused variable Eric Engestrom
2016-09-21 8:50 ` Nicolai Stange
2016-09-21 9:23 ` Eric Engestrom
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.