On Wed, Aug 13, 2008 at 06:40:17PM -0400, Chuck Lever wrote:
> Clean up: refactor the rpcb_register() call out of svc_register().
>
> The next patch will choose the correct registration subroutine to use
> based on whether IPv6 support is desired for RPC services.
>
> Signed-off-by: Chuck Lever <chuck.lever@xxxxxxxxxx>
> ---
>
> include/linux/sunrpc/svc.h | 4 +++-
> net/sunrpc/svc.c | 40 +++++++++++++++++++++++++++-------------
> 2 files changed, 30 insertions(+), 14 deletions(-)
>
> diff --git a/include/linux/sunrpc/svc.h b/include/linux/sunrpc/svc.h
> index a794d4a..2a41d29 100644
> --- a/include/linux/sunrpc/svc.h
> +++ b/include/linux/sunrpc/svc.h
> @@ -395,7 +395,9 @@ struct svc_serv * svc_create_pooled(struct svc_program *, unsigned int,
> int svc_set_num_threads(struct svc_serv *, struct svc_pool *, int);
> void svc_destroy(struct svc_serv *);
> int svc_process(struct svc_rqst *);
> -int svc_register(struct svc_serv *, int, unsigned short);
> +int svc_register(const struct svc_serv *, const unsigned short,
> + const unsigned short);
> +
> void svc_wake_up(struct svc_serv *);
> void svc_reserve(struct svc_rqst *rqstp, int space);
> struct svc_pool * svc_pool_for_cpu(struct svc_serv *serv, int cpu);
> diff --git a/net/sunrpc/svc.c b/net/sunrpc/svc.c
> index 58a8012..aa334c2 100644
> --- a/net/sunrpc/svc.c
> +++ b/net/sunrpc/svc.c
> @@ -720,17 +720,32 @@ svc_exit_thread(struct svc_rqst *rqstp)
> }
> EXPORT_SYMBOL(svc_exit_thread);
>
> -/*
> - * Register an RPC service with the local portmapper.
> - * To unregister a service, call this routine with
> - * proto and port == 0.
> +static int __svc_register(const u32 program, const u32 version,
> + sa_family_t family,
> + const unsigned short protocol,
> + const unsigned short port)
> +{
> + int error, result;
> +
> + error = rpcb_register(program, version, protocol, port, &result);
> + if (!result)
> + error = -EACCES;
> + return error;
Isn't "result" 0 whenever error is nonzero? In that case the above is
really equivalent to
rpcb_register(program, version, protocol, port, &result);
return result ? 0 : -EACCES;
Is that what's intended? I don't believe it's the behavior of the
original code.
I understand why they're there, but the separate "result" and "error"
returns are kinda confusing. If the distinction's not needed then I
wonder whether it should just be buried as deep in rpcb_register() as
possible so we don't have to think about it.
--b.
> +}
> +
> +/**
> + * svc_register - register an RPC service with the local portmapper
> + * @serv: svc_serv struct for the service to register
> + * @proto: transport protocol number to advertise
> + * @port: port to advertise
> + *
> */
> -int
> -svc_register(struct svc_serv *serv, int proto, unsigned short port)
> +int svc_register(const struct svc_serv *serv, const unsigned short proto,
> + const unsigned short port)
> {
> struct svc_program *progp;
> unsigned int i;
> - int error = 0, dummy;
> + int error = 0;
>
> BUG_ON(proto == 0 && port == 0);
>
> @@ -739,8 +754,9 @@ svc_register(struct svc_serv *serv, int proto, unsigned short port)
> if (progp->pg_vers[i] == NULL)
> continue;
>
> - dprintk("svc: svc_register(%s, %s, %d, %d)%s\n",
> + dprintk("svc: svc_register(%s, %u, %s, %u, %d)%s\n",
> progp->pg_name,
> + serv->sv_family,
> proto == IPPROTO_UDP? "udp" : "tcp",
> port,
> i,
> @@ -750,13 +766,11 @@ svc_register(struct svc_serv *serv, int proto, unsigned short port)
> if (progp->pg_vers[i]->vs_hidden)
> continue;
>
> - error = rpcb_register(progp->pg_prog, i, proto, port, &dummy);
> + error = __svc_register(progp->pg_prog, i,
> + serv->sv_family, proto,
> + port);
> if (error < 0)
> break;
> - if (!dummy) {
> - error = -EACCES;
> - break;
> - }
> }
> }
>
>
--
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
