| Message ID | 20240424033916.2748488-3-libaokun@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | cachefiles: some bugfixes and cleanups for ondemand requests | expand |
在 2024/4/24 11:39, libaokun@huaweicloud.com 写道: > From: Baokun Li <libaokun1@huawei.com> > > The err_put_fd tag is only used once, so remove it to make the code more > readable. > > Signed-off-by: Baokun Li <libaokun1@huawei.com> Reviewed-by: Jia Zhu <zhujia.zj@bytedance.com> > --- > fs/cachefiles/ondemand.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c > index 4ba42f1fa3b4..fd49728d8bae 100644 > --- a/fs/cachefiles/ondemand.c > +++ b/fs/cachefiles/ondemand.c > @@ -347,7 +347,9 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, > > if (copy_to_user(_buffer, msg, n) != 0) { > ret = -EFAULT; > - goto err_put_fd; > + if (msg->opcode == CACHEFILES_OP_OPEN) > + close_fd(((struct cachefiles_open *)msg->data)->fd); > + goto error; > } > > /* CLOSE request has no reply */ > @@ -358,9 +360,6 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, > > return n; > > -err_put_fd: > - if (msg->opcode == CACHEFILES_OP_OPEN) > - close_fd(((struct cachefiles_open *)msg->data)->fd); > error: > xa_erase(&cache->reqs, id); > req->error = ret;
On 4/24/24 11:39 AM, libaokun@huaweicloud.com wrote: > From: Baokun Li <libaokun1@huawei.com> > > The err_put_fd tag is only used once, so remove it to make the code more > readable. I think it's a conventional style to put error handling in the bottom of the function so that it could be reused. Indeed currently err_put_fd has only one caller but IMHO it's only styling issues. By the way it seems that this is not needed anymore if patch 9 is applied. > --- > fs/cachefiles/ondemand.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c > index 4ba42f1fa3b4..fd49728d8bae 100644 > --- a/fs/cachefiles/ondemand.c > +++ b/fs/cachefiles/ondemand.c > @@ -347,7 +347,9 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, > > if (copy_to_user(_buffer, msg, n) != 0) { > ret = -EFAULT; > - goto err_put_fd; > + if (msg->opcode == CACHEFILES_OP_OPEN) > + close_fd(((struct cachefiles_open *)msg->data)->fd); > + goto error; > } > > /* CLOSE request has no reply */ > @@ -358,9 +360,6 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, > > return n; > > -err_put_fd: > - if (msg->opcode == CACHEFILES_OP_OPEN) > - close_fd(((struct cachefiles_open *)msg->data)->fd); > error: > xa_erase(&cache->reqs, id); > req->error = ret;
On 2024/5/6 11:55, Jingbo Xu wrote: > > On 4/24/24 11:39 AM, libaokun@huaweicloud.com wrote: >> From: Baokun Li <libaokun1@huawei.com> >> >> The err_put_fd tag is only used once, so remove it to make the code more >> readable. > I think it's a conventional style to put error handling in the bottom of > the function so that it could be reused. Indeed currently err_put_fd > has only one caller but IMHO it's only styling issues. > > By the way it seems that this is not needed anymore if patch 9 is applied. This is just to make patch 3 look clearer, if you insist on dropping it I will drop it in the next revision. Cheers, Baokun >> --- >> fs/cachefiles/ondemand.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c >> index 4ba42f1fa3b4..fd49728d8bae 100644 >> --- a/fs/cachefiles/ondemand.c >> +++ b/fs/cachefiles/ondemand.c >> @@ -347,7 +347,9 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, >> >> if (copy_to_user(_buffer, msg, n) != 0) { >> ret = -EFAULT; >> - goto err_put_fd; >> + if (msg->opcode == CACHEFILES_OP_OPEN) >> + close_fd(((struct cachefiles_open *)msg->data)->fd); >> + goto error; >> } >> >> /* CLOSE request has no reply */ >> @@ -358,9 +360,6 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, >> >> return n; >> >> -err_put_fd: >> - if (msg->opcode == CACHEFILES_OP_OPEN) >> - close_fd(((struct cachefiles_open *)msg->data)->fd); >> error: >> xa_erase(&cache->reqs, id); >> req->error = ret;
diff --git a/fs/cachefiles/ondemand.c b/fs/cachefiles/ondemand.c index 4ba42f1fa3b4..fd49728d8bae 100644 --- a/fs/cachefiles/ondemand.c +++ b/fs/cachefiles/ondemand.c @@ -347,7 +347,9 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, if (copy_to_user(_buffer, msg, n) != 0) { ret = -EFAULT; - goto err_put_fd; + if (msg->opcode == CACHEFILES_OP_OPEN) + close_fd(((struct cachefiles_open *)msg->data)->fd); + goto error; } /* CLOSE request has no reply */ @@ -358,9 +360,6 @@ ssize_t cachefiles_ondemand_daemon_read(struct cachefiles_cache *cache, return n; -err_put_fd: - if (msg->opcode == CACHEFILES_OP_OPEN) - close_fd(((struct cachefiles_open *)msg->data)->fd); error: xa_erase(&cache->reqs, id); req->error = ret;