2018-12-17 16:26:17

by Vasily Averin

Subject: [PATCH 4/4] nfs: fix debug message in svc_create_xprt()

_svc_create_xprt() returns positive port number
so its non-zero return value is not an error

Signed-off-by: Vasily Averin <[email protected]>
---
net/sunrpc/svc_xprt.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
index 51d36230b6e3..130af22d5c66 100644
--- a/net/sunrpc/svc_xprt.c
+++ b/net/sunrpc/svc_xprt.c
@@ -296,9 +296,9 @@ int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
request_module("svc%s", xprt_name);
err = _svc_create_xprt(serv, xprt_name, net, family, port, flags);
}
- if (err)
+ if (err < 0)
dprintk("svc: transport %s not found, err %d\n",
- xprt_name, err);
+ xprt_name, -err);
return err;
}
EXPORT_SYMBOL_GPL(svc_create_xprt);
--
2.17.1

2018-12-17 17:56:49

by Jeff Layton

Subject: Re: [PATCH 4/4] nfs: fix debug message in svc_create_xprt()

On Mon, 2018-12-17 at 19:26 +0300, Vasily Averin wrote:
> _svc_create_xprt() returns positive port number
> so its non-zero return value is not an error
>
> Signed-off-by: Vasily Averin <[email protected]>
> ---
> net/sunrpc/svc_xprt.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> index 51d36230b6e3..130af22d5c66 100644
> --- a/net/sunrpc/svc_xprt.c
> +++ b/net/sunrpc/svc_xprt.c
> @@ -296,9 +296,9 @@ int svc_create_xprt(struct svc_serv *serv, const char *xprt_name,
> request_module("svc%s", xprt_name);
> err = _svc_create_xprt(serv, xprt_name, net, family, port, flags);
> }
> - if (err)
> + if (err < 0)
> dprintk("svc: transport %s not found, err %d\n",
> - xprt_name, err);
> + xprt_name, -err);
> return err;
> }
> EXPORT_SYMBOL_GPL(svc_create_xprt);

This patch is fine, but _svc_create_xprt does not make it clear that it
returns a positive port number.

Would you be amenable to spinning up a patch to add a kerneldoc comment
over that function (and maybe one over svc_create_xprt too), to make it
clear what it does?

Thanks,

Reviewed-by: Jeff Layton <[email protected]>