Re: [patch] nfs: fix some issues in nfs41_proc_reclaim_complete()

[Benny Halevy <bhalevy@panasas.com>]

On Apr. 22, 2010, 12:28 +0300, Dan Carpenter <error27@gmail.com> wrote:
> The original code passed an ERR_PTR() to rpc_put_task() and instead of
> returning zero on success it returned -ENOMEM.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> This was found by smatch and I've only compile tested it.  Sorry.  :/
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 6380670..071fced 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5218,9 +5218,12 @@ static int nfs41_proc_reclaim_complete(struct nfs_client *clp)
>  	msg.rpc_resp = &calldata->res;
>  	task_setup_data.callback_data = calldata;
>  	task = rpc_run_task(&task_setup_data);
> -	if (IS_ERR(task))
> +	if (IS_ERR(task)) {
>  		status = PTR_ERR(task);
> +		goto out;
> +	}
>  	rpc_put_task(task);
> +	return 0;
>  out:
>  	dprintk("<-- %s status=%d\n", __func__, status);
>  	return status;

Dan, thanks for sending the fix.
For the sake of dprintk, how about doing this instead:

git diff --stat -p -M
 fs/nfs/nfs4proc.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 6380670..8671f7a 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5204,12 +5204,14 @@ static int nfs41_proc_reclaim_complete(struct nfs_client *clp)
 		.callback_ops = &nfs4_reclaim_complete_call_ops,
 		.flags = RPC_TASK_ASYNC,
 	};
-	int status = -ENOMEM;
+	int status;
 
 	dprintk("--> %s\n", __func__);
 	calldata = kzalloc(sizeof(*calldata), GFP_KERNEL);
-	if (calldata == NULL)
+	if (calldata == NULL) {
+		status = -ENOMEM;
 		goto out;
+	}
 	calldata->clp = clp;
 	calldata->arg.one_fs = 0;
 	calldata->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
@@ -5218,9 +5220,12 @@ static int nfs41_proc_reclaim_complete(struct nfs_client *clp)
 	msg.rpc_resp = &calldata->res;
 	task_setup_data.callback_data = calldata;
 	task = rpc_run_task(&task_setup_data);
-	if (IS_ERR(task))
+	if (IS_ERR(task)) {
 		status = PTR_ERR(task);
+		goto out;
+	}
 	rpc_put_task(task);
+	status = 0;
 out:
 	dprintk("<-- %s status=%d\n", __func__, status);
 	return status;