* [bug report] nfs: make sillyrename an async operation
@ 2018-12-19 10:33 Dan Carpenter
2018-12-19 23:31 ` NeilBrown
0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2018-12-19 10:33 UTC (permalink / raw)
To: jlayton, NeilBrown; +Cc: linux-nfs
[ Ancient code. But NeilB changed get_cred() recently so Smatch thinks
it understands it now. - dan ]
Hello Jeff Layton,
The patch d3d4152a5d59: "nfs: make sillyrename an async operation"
from Sep 17, 2010, leads to the following static checker warning:
fs/nfs/unlink.c:351 nfs_async_rename()
warn: 'data->cred' isn't an ERR_PTR
fs/nfs/unlink.c
330 struct rpc_task *
331 nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
332 struct dentry *old_dentry, struct dentry *new_dentry,
333 void (*complete)(struct rpc_task *, struct nfs_renamedata *))
334 {
335 struct nfs_renamedata *data;
336 struct rpc_message msg = { };
337 struct rpc_task_setup task_setup_data = {
338 .rpc_message = &msg,
339 .callback_ops = &nfs_rename_ops,
340 .workqueue = nfsiod_workqueue,
341 .rpc_client = NFS_CLIENT(old_dir),
342 .flags = RPC_TASK_ASYNC,
343 };
344
345 data = kzalloc(sizeof(*data), GFP_KERNEL);
346 if (data == NULL)
347 return ERR_PTR(-ENOMEM);
348 task_setup_data.callback_data = data;
349
350 data->cred = get_current_cred();
351 if (IS_ERR(data->cred)) {
get_current_cred() always returns a valid pointer, I think? Can we
just remove this condition?
352 struct rpc_task *task = ERR_CAST(data->cred);
353 kfree(data);
354 return task;
355 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [bug report] nfs: make sillyrename an async operation
2018-12-19 10:33 [bug report] nfs: make sillyrename an async operation Dan Carpenter
@ 2018-12-19 23:31 ` NeilBrown
0 siblings, 0 replies; 2+ messages in thread
From: NeilBrown @ 2018-12-19 23:31 UTC (permalink / raw)
To: Dan Carpenter, jlayton; +Cc: linux-nfs
[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]
On Wed, Dec 19 2018, Dan Carpenter wrote:
> [ Ancient code. But NeilB changed get_cred() recently so Smatch thinks
> it understands it now. - dan ]
>
> Hello Jeff Layton,
>
> The patch d3d4152a5d59: "nfs: make sillyrename an async operation"
> from Sep 17, 2010, leads to the following static checker warning:
>
> fs/nfs/unlink.c:351 nfs_async_rename()
> warn: 'data->cred' isn't an ERR_PTR
It would have been impressive to determine that at the time, though I
think it was true.
At the time it was rpc_lookup_cred(), that provided data->cred.
That was
return rpcauth_lookupcred(&generic_auth, 0);
of which the key line is :
ret = auth->au_ops->lookup_cred(auth, &acred, flags);
and generic_auth.au_ops->lookup_cred was
return rpcauth_lookup_credcache(&generic_auth, acred, flags);
which can only return an error if
cred->cr_ops->cr_init != NULL &&
and
int res = cred->cr_ops->cr_init(auth, cred);
if (res < 0) {
There is no generic_auth.au_ops->cr_init, so no error can be return.
It is much easier to determine that now.
Thanks,
NeilBrown
From: NeilBrown <neilb@suse.com>
Date: Thu, 20 Dec 2018 10:29:55 +1100
Subject: [PATCH] NFS: remove unnecessary test for IS_ERR(cred)
As gte_current_cred() cannot return an error,
this test is not necessary.
It hasn't been necessary for years, but it wasn't so obvious
before.
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: NeilBrown <neilb@suse.com>
---
fs/nfs/unlink.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index a227ab7d6891..79b97b3c4427 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -348,11 +348,6 @@ nfs_async_rename(struct inode *old_dir, struct inode *new_dir,
task_setup_data.callback_data = data;
data->cred = get_current_cred();
- if (IS_ERR(data->cred)) {
- struct rpc_task *task = ERR_CAST(data->cred);
- kfree(data);
- return task;
- }
msg.rpc_argp = &data->args;
msg.rpc_resp = &data->res;
--
2.14.0.rc0.dirty
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-12-19 23:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-19 10:33 [bug report] nfs: make sillyrename an async operation Dan Carpenter
2018-12-19 23:31 ` NeilBrown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).