2005-10-06 14:13:21

by Lever, Charles

[permalink] [raw]
Subject: RE: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups

actually, can we hold off on this change? the RPC transport switch will
eventually need most of those EXPORT_SYMBOLs.

the only harmless change i see below is removing xdr_decode_string().

> -----Original Message-----
> From: Adrian Bunk [mailto:[email protected]]
> Sent: Saturday, October 01, 2005 10:21 AM
> To: David Miller
> Cc: [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]
> Subject: [NFS] [RFC: 2.6 patch] net/sunrpc/: possible cleanups
>
> This patch contains the following possible cleanups:
> - make needlessly global code static
> - #if 0 the following unused global function:
> - xdr.c: xdr_decode_string
> - remove the following unneeded EXPORT_SYMBOL's:
> - auth_gss/gss_mech_switch.c: gss_mech_get
> - auth_gss/gss_mech_switch.c: gss_mech_get_by_name
> - auth_gss/gss_mech_switch.c: gss_mech_get_by_pseudoflavor
> - auth_gss/gss_mech_switch.c: gss_pseudoflavor_to_service
> - auth_gss/gss_mech_switch.c: gss_service_to_auth_domain_name
> - auth_gss/gss_mech_switch.c: gss_mech_put
> - sunrpc_syms.c: rpc_wake_up_next
> - sunrpc_syms.c: rpc_new_child
> - sunrpc_syms.c: rpc_run_child
> - sunrpc_syms.c: rpc_new_task
> - sunrpc_syms.c: rpc_release_task
> - sunrpc_syms.c: rpc_release_client
> - sunrpc_syms.c: xprt_udp_slot_table_entries
> - sunrpc_syms.c: xprt_tcp_slot_table_entries
> - sunrpc_syms.c: svc_drop
> - sunrpc_syms.c: svc_authenticate
> - sunrpc_syms.c: xdr_decode_string
>
> Please review which of these patches do make sense and which conflict
> with pending patches.
>
> Signed-off-by: Adrian Bunk <[email protected]>
>
> ---
>
> This patch was already sent on:
> - 30 May 2005
> - 7 May 2005
>
> include/linux/sunrpc/clnt.h | 1 -
> include/linux/sunrpc/gss_api.h | 3 ---
> include/linux/sunrpc/xdr.h | 2 --
> net/sunrpc/auth_gss/gss_mech_switch.c | 13 +------------
> net/sunrpc/clnt.c | 3 ++-
> net/sunrpc/sunrpc_syms.c | 11 -----------
> net/sunrpc/xdr.c | 4 +++-
> 7 files changed, 6 insertions(+), 31 deletions(-)
>
> ---
> linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/gss_api.h.old
> 2005-05-05 23:05:01.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/gss_api.h
> 2005-05-05 23:05:10.000000000 +0200
> @@ -110,9 +110,6 @@
> /* Similar, but get by pseudoflavor. */
> struct gss_api_mech *gss_mech_get_by_pseudoflavor(u32);
>
> -/* Just increments the mechanism's reference count and
> returns its input: */
> -struct gss_api_mech * gss_mech_get(struct gss_api_mech *);
> -
> /* For every succesful gss_mech_get or gss_mech_get_by_*
> call there must be a
> * corresponding call to gss_mech_put. */
> void gss_mech_put(struct gss_api_mech *);
> ---
> linux-2.6.12-rc3-mm3-full/net/sunrpc/auth_gss/gss_mech_switc
> h.c.old 2005-05-05 23:05:17.000000000 +0200
> +++
> linux-2.6.12-rc3-mm3-full/net/sunrpc/auth_gss/gss_mech_s
> witch.c 2005-05-05 23:19:33.000000000 +0200
> @@ -133,14 +133,13 @@
>
> EXPORT_SYMBOL(gss_mech_unregister);
>
> -struct gss_api_mech *
> +static struct gss_api_mech *
> gss_mech_get(struct gss_api_mech *gm)
> {
> __module_get(gm->gm_owner);
> return gm;
> }
>
> -EXPORT_SYMBOL(gss_mech_get);
>
> struct gss_api_mech *
> gss_mech_get_by_name(const char *name)
> @@ -160,8 +159,6 @@
>
> }
>
> -EXPORT_SYMBOL(gss_mech_get_by_name);
> -
> static inline int
> mech_supports_pseudoflavor(struct gss_api_mech *gm, u32 pseudoflavor)
> {
> @@ -193,8 +190,6 @@
> return gm;
> }
>
> -EXPORT_SYMBOL(gss_mech_get_by_pseudoflavor);
> -
> u32
> gss_pseudoflavor_to_service(struct gss_api_mech *gm, u32
> pseudoflavor)
> {
> @@ -207,8 +202,6 @@
> return 0;
> }
>
> -EXPORT_SYMBOL(gss_pseudoflavor_to_service);
> -
> char *
> gss_service_to_auth_domain_name(struct gss_api_mech *gm, u32 service)
> {
> @@ -221,16 +214,12 @@
> return NULL;
> }
>
> -EXPORT_SYMBOL(gss_service_to_auth_domain_name);
> -
> void
> gss_mech_put(struct gss_api_mech * gm)
> {
> module_put(gm->gm_owner);
> }
>
> -EXPORT_SYMBOL(gss_mech_put);
> -
> /* The mech could probably be determined from the token
> instead, but it's just
> * as easy for now to pass it in. */
> int
> --- linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/clnt.h.old
> 2005-05-05 23:05:45.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/clnt.h
> 2005-05-05 23:05:50.000000000 +0200
> @@ -134,7 +134,6 @@
> void rpc_clnt_sigunmask(struct rpc_clnt *clnt,
> sigset_t *oldset);
> void rpc_setbufsize(struct rpc_clnt *, unsigned int,
> unsigned int);
> size_t rpc_max_payload(struct rpc_clnt *);
> -int rpc_ping(struct rpc_clnt *clnt, int flags);
>
> static __inline__
> int rpc_call(struct rpc_clnt *clnt, u32 proc, void *argp,
> void *resp, int flags)
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/clnt.c.old
> 2005-05-05 23:05:58.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/clnt.c
> 2005-05-05 23:06:21.000000000 +0200
> @@ -63,6 +63,7 @@
> static u32 * call_header(struct rpc_task *task);
> static u32 * call_verify(struct rpc_task *task);
>
> +static int rpc_ping(struct rpc_clnt *clnt, int flags);
>
> static int
> rpc_setup_pipedir(struct rpc_clnt *clnt, char *dir_name)
> @@ -1178,7 +1179,7 @@
> .p_decode = rpcproc_decode_null,
> };
>
> -int rpc_ping(struct rpc_clnt *clnt, int flags)
> +static int rpc_ping(struct rpc_clnt *clnt, int flags)
> {
> struct rpc_message msg = {
> .rpc_proc = &rpcproc_null,
> --- linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/xdr.h.old
> 2005-05-05 23:06:40.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/include/linux/sunrpc/xdr.h
> 2005-05-05 23:07:23.000000000 +0200
> @@ -91,7 +91,6 @@
> u32 * xdr_encode_opaque_fixed(u32 *p, const void
> *ptr, unsigned int len);
> u32 * xdr_encode_opaque(u32 *p, const void *ptr,
> unsigned int len);
> u32 * xdr_encode_string(u32 *p, const char *s);
> -u32 * xdr_decode_string(u32 *p, char **sp, int *lenp,
> int maxlen);
> u32 * xdr_decode_string_inplace(u32 *p, char **sp,
> int *lenp, int maxlen);
> u32 * xdr_encode_netobj(u32 *p, const struct xdr_netobj *);
> u32 * xdr_decode_netobj(u32 *p, struct xdr_netobj *);
> @@ -147,7 +146,6 @@
> extern int xdr_buf_subsegment(struct xdr_buf *, struct
> xdr_buf *, int, int);
> extern int xdr_buf_read_netobj(struct xdr_buf *, struct
> xdr_netobj *, int);
> extern int read_bytes_from_xdr_buf(struct xdr_buf *, int,
> void *, int);
> -extern int write_bytes_to_xdr_buf(struct xdr_buf *, int,
> void *, int);
>
> /*
> * Helper structure for copying from an sk_buff.
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/xdr.c.old
> 2005-05-05 23:06:52.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/xdr.c
> 2005-05-05 23:07:56.000000000 +0200
> @@ -95,6 +95,7 @@
> return xdr_encode_array(p, string, strlen(string));
> }
>
> +#if 0
> u32 *
> xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen)
> {
> @@ -115,6 +116,7 @@
> *sp = string;
> return p + XDR_QUADLEN(len);
> }
> +#endif /* 0 */
>
> u32 *
> xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen)
> @@ -882,7 +884,7 @@
> }
>
> /* obj is assumed to point to allocated memory of size at
> least len: */
> -int
> +static int
> write_bytes_to_xdr_buf(struct xdr_buf *buf, int base, void
> *obj, int len)
> {
> struct xdr_buf subbuf;
> --- linux-2.6.12-rc3-mm3-full/net/sunrpc/sunrpc_syms.c.old
> 2005-05-05 23:07:30.000000000 +0200
> +++ linux-2.6.12-rc3-mm3-full/net/sunrpc/sunrpc_syms.c
> 2005-05-05 23:36:43.000000000 +0200
> @@ -29,15 +29,10 @@
> EXPORT_SYMBOL(rpc_execute);
> EXPORT_SYMBOL(rpc_init_task);
> EXPORT_SYMBOL(rpc_sleep_on);
> -EXPORT_SYMBOL(rpc_wake_up_next);
> EXPORT_SYMBOL(rpc_wake_up_task);
> -EXPORT_SYMBOL(rpc_new_child);
> -EXPORT_SYMBOL(rpc_run_child);
> EXPORT_SYMBOL(rpciod_down);
> EXPORT_SYMBOL(rpciod_up);
> -EXPORT_SYMBOL(rpc_new_task);
> EXPORT_SYMBOL(rpc_wake_up_status);
> -EXPORT_SYMBOL(rpc_release_task);
>
> /* RPC client functions */
> EXPORT_SYMBOL(rpc_create_client);
> @@ -45,7 +40,6 @@
> EXPORT_SYMBOL(rpc_bind_new_program);
> EXPORT_SYMBOL(rpc_destroy_client);
> EXPORT_SYMBOL(rpc_shutdown_client);
> -EXPORT_SYMBOL(rpc_release_client);
> EXPORT_SYMBOL(rpc_killall_tasks);
> EXPORT_SYMBOL(rpc_call_sync);
> EXPORT_SYMBOL(rpc_call_async);
> @@ -63,8 +57,6 @@
> /* Client transport */
> EXPORT_SYMBOL(xprt_create_proto);
> EXPORT_SYMBOL(xprt_set_timeout);
> -EXPORT_SYMBOL(xprt_udp_slot_table_entries);
> -EXPORT_SYMBOL(xprt_tcp_slot_table_entries);
>
> /* Client credential cache */
> EXPORT_SYMBOL(rpcauth_register);
> @@ -81,7 +73,6 @@
> EXPORT_SYMBOL(svc_create_thread);
> EXPORT_SYMBOL(svc_exit_thread);
> EXPORT_SYMBOL(svc_destroy);
> -EXPORT_SYMBOL(svc_drop);
> EXPORT_SYMBOL(svc_process);
> EXPORT_SYMBOL(svc_recv);
> EXPORT_SYMBOL(svc_wake_up);
> @@ -89,7 +80,6 @@
> EXPORT_SYMBOL(svc_reserve);
> EXPORT_SYMBOL(svc_auth_register);
> EXPORT_SYMBOL(auth_domain_lookup);
> -EXPORT_SYMBOL(svc_authenticate);
> EXPORT_SYMBOL(svc_set_client);
>
> /* RPC statistics */
> @@ -122,7 +112,6 @@
>
> /* Generic XDR */
> EXPORT_SYMBOL(xdr_encode_string);
> -EXPORT_SYMBOL(xdr_decode_string);
> EXPORT_SYMBOL(xdr_decode_string_inplace);
> EXPORT_SYMBOL(xdr_decode_netobj);
> EXPORT_SYMBOL(xdr_encode_netobj);
>
>
>
> -------------------------------------------------------
> This SF.Net email is sponsored by:
> Power Architecture Resource Center: Free content, downloads,
> discussions,
> and more. http://solutions.newsforge.com/ibmarch.tmpl
> _______________________________________________
> NFS maillist - [email protected]
> https://lists.sourceforge.net/lists/listinfo/nfs
>


2005-11-23 01:24:21

by Adrian Bunk

[permalink] [raw]
Subject: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:

> actually, can we hold off on this change? the RPC transport switch will
> eventually need most of those EXPORT_SYMBOLs.

Am I right to assume this will happen in the foreseeable future?

> the only harmless change i see below is removing xdr_decode_string().

Patch below.

cu
Adrian


<-- snip -->


This patch removes ths unused function xdr_decode_string().


Signed-off-by: Adrian Bunk <[email protected]>

---

include/linux/sunrpc/xdr.h | 1 -
net/sunrpc/sunrpc_syms.c | 1 -
net/sunrpc/xdr.c | 21 ---------------------
3 files changed, 23 deletions(-)

--- linux-2.6.15-rc1-mm2-full/include/linux/sunrpc/xdr.h.old 2005-11-23 02:03:01.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/include/linux/sunrpc/xdr.h 2005-11-23 02:03:08.000000000 +0100
@@ -91,7 +91,6 @@
u32 * xdr_encode_opaque_fixed(u32 *p, const void *ptr, unsigned int len);
u32 * xdr_encode_opaque(u32 *p, const void *ptr, unsigned int len);
u32 * xdr_encode_string(u32 *p, const char *s);
-u32 * xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen);
u32 * xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen);
u32 * xdr_encode_netobj(u32 *p, const struct xdr_netobj *);
u32 * xdr_decode_netobj(u32 *p, struct xdr_netobj *);
--- linux-2.6.15-rc1-mm2-full/net/sunrpc/xdr.c.old 2005-11-23 02:03:17.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/net/sunrpc/xdr.c 2005-11-23 02:03:27.000000000 +0100
@@ -93,27 +93,6 @@
}

u32 *
-xdr_decode_string(u32 *p, char **sp, int *lenp, int maxlen)
-{
- unsigned int len;
- char *string;
-
- if ((len = ntohl(*p++)) > maxlen)
- return NULL;
- if (lenp)
- *lenp = len;
- if ((len % 4) != 0) {
- string = (char *) p;
- } else {
- string = (char *) (p - 1);
- memmove(string, p, len);
- }
- string[len] = '\0';
- *sp = string;
- return p + XDR_QUADLEN(len);
-}
-
-u32 *
xdr_decode_string_inplace(u32 *p, char **sp, int *lenp, int maxlen)
{
unsigned int len;
--- linux-2.6.15-rc1-mm2-full/net/sunrpc/sunrpc_syms.c.old 2005-11-23 02:03:35.000000000 +0100
+++ linux-2.6.15-rc1-mm2-full/net/sunrpc/sunrpc_syms.c 2005-11-23 02:03:38.000000000 +0100
@@ -120,7 +120,6 @@

/* Generic XDR */
EXPORT_SYMBOL(xdr_encode_string);
-EXPORT_SYMBOL(xdr_decode_string);
EXPORT_SYMBOL(xdr_decode_string_inplace);
EXPORT_SYMBOL(xdr_decode_netobj);
EXPORT_SYMBOL(xdr_encode_netobj);

2005-11-23 12:31:21

by Lever, Charles

[permalink] [raw]
Subject: RE: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

> On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:
>
> > actually, can we hold off on this change? the RPC
> transport switch will
> > eventually need most of those EXPORT_SYMBOLs.
>
> Am I right to assume this will happen in the foreseeable future?

the first portion of the transport switch is in 2.6.15-rcX. at this
point i'm expecting the EXPORT_SYMBOL changes to go in 2.6.17 or later.

so i don't remember why you are removing xdr_decode_string. are we sure
that no-one will need this functionality in the future? it is harmless
to remove today, but i wonder if someone is just going to add it back
sometime.

2005-11-23 16:25:43

by Adrian Bunk

[permalink] [raw]
Subject: Re: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > On Thu, Oct 06, 2005 at 07:13:14AM -0700, Lever, Charles wrote:
> >
> > > actually, can we hold off on this change? the RPC
> > transport switch will
> > > eventually need most of those EXPORT_SYMBOLs.
> >
> > Am I right to assume this will happen in the foreseeable future?
>
> the first portion of the transport switch is in 2.6.15-rcX. at this
> point i'm expecting the EXPORT_SYMBOL changes to go in 2.6.17 or later.

OK.

> so i don't remember why you are removing xdr_decode_string. are we sure
> that no-one will need this functionality in the future? it is harmless
> to remove today, but i wonder if someone is just going to add it back
> sometime.

It's unused and you said:
the only harmless change i see below is removing xdr_decode_string().

cu
Adrian

--

"Is there not promise of rain?" Ling Tan asked suddenly out
of the darkness. There had been need of rain for many days.
"Only a promise," Lao Er said.
Pearl S. Buck - Dragon Seed

2005-11-23 23:07:31

by NeilBrown

[permalink] [raw]
Subject: Re: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

On Wednesday November 23, [email protected] wrote:
> On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > so i don't remember why you are removing xdr_decode_string. are we sure
> > that no-one will need this functionality in the future? it is harmless
> > to remove today, but i wonder if someone is just going to add it back
> > sometime.
>
> It's unused and you said:
> the only harmless change i see below is removing xdr_decode_string().
>

As 'xdr_decode_string' (sometimes) modifies the buffer that it is
decoding, I don't think it's usage should be encouraged. If it is no
longer in use, then I fully support and encourage removing it.

NeilBrown

2005-11-24 06:57:08

by Lever, Charles

[permalink] [raw]
Subject: RE: [2.6 patch] net/sunrpc/xdr.c: remove xdr_decode_string()

> On Wednesday November 23, [email protected] wrote:
> > On Wed, Nov 23, 2005 at 04:31:14AM -0800, Lever, Charles wrote:
> > > so i don't remember why you are removing
> xdr_decode_string. are we sure
> > > that no-one will need this functionality in the future?
> it is harmless
> > > to remove today, but i wonder if someone is just going to
> add it back
> > > sometime.
> >
> > It's unused and you said:
> > the only harmless change i see below is removing
> xdr_decode_string().
> >
>
> As 'xdr_decode_string' (sometimes) modifies the buffer that it is
> decoding, I don't think it's usage should be encouraged. If it is no
> longer in use, then I fully support and encourage removing it.

actually this is a good point. since it is unused, it is an untested
path as we continue to evolve the code base.