On Mar. 11, 2009, 2:27 +0200, "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> From: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
>
> The errors returned aren't used. Just return 0 and make them available
Wouldn't it be better to change the function to return void
so justice would be seen to be done?
A couple more comments below...
> to a dprintk(). Also, consistently use -ERRNO errors instead of nfs
> errors.
>
> Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxxxxxx>
> ---
> fs/nfsd/nfs4callback.c | 15 +++++++--------
> 1 files changed, 7 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 5dcd38e..c979af7 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -389,12 +389,10 @@ static int do_probe_callback(void *data)
> .rpc_argp = clp,
> };
> struct rpc_clnt *client;
> - int status;
> + int status = -EINVAL;
>
> - if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5)) {
> - status = nfserr_cb_path_down;
> + if (!clp->cl_principal && (clp->cl_flavor >= RPC_AUTH_GSS_KRB5))
> goto out_err;
> - }
>
> /* Initialize address */
> memset(&addr, 0, sizeof(addr));
> @@ -405,7 +403,8 @@ static int do_probe_callback(void *data)
> /* Create RPC client */
> client = rpc_create(&args);
> if (IS_ERR(client)) {
> - dprintk("NFSD: couldn't create callback client\n");
> + dprintk("NFSD: couldn't create callback client: %d\n",
> + PTR_ERR(client));
> status = PTR_ERR(client);
How about reversing the order and dprintk status instead of PTR_ERR(client)?
> goto out_err;
> }
> @@ -422,10 +421,10 @@ static int do_probe_callback(void *data)
> out_release_client:
> rpc_shutdown_client(client);
> out_err:
> - dprintk("NFSD: warning: no callback path to client %.*s\n",
> - (int)clp->cl_name.len, clp->cl_name.data);
> + dprintk("NFSD: warning: no callback path to client %.*: error %ds\n",
seems like a typo. ": error %d" was pasted one char too soon...
should be:
+ dprintk("NFSD: warning: no callback path to client %.*s: error %d\n",
Benny
> + (int)clp->cl_name.len, clp->cl_name.data, status);
> put_nfs4_client(clp);
> - return status;
> + return 0;
> }
>
> /*
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
