All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long
@ 2009-04-22 21:45 Frank Filz
  2009-04-22 21:46 ` Chuck Lever
  2009-04-22 21:56 ` Chuck Lever
  0 siblings, 2 replies; 7+ messages in thread
From: Frank Filz @ 2009-04-22 21:45 UTC (permalink / raw)
  To: NFS List, libtirpc, Steve Dickson; +Cc: Chuck Lever

This fixes a problem where "rpcinfo -T transport host prognum" fails on a PPC64
because CLNT_CONTROL expects the version number to be a 32 bit quantity. u_long
probably works fine on little endian machines, but won't work on big endian
machines.


Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
---
 src/rpcinfo.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/src/rpcinfo.c b/src/rpcinfo.c
index 0170f65..698f4ca 100644
--- a/src/rpcinfo.c
+++ b/src/rpcinfo.c
@@ -1591,7 +1591,7 @@ progping (netid, argc, argv)
   CLIENT *client;
   struct timeval to;
   enum clnt_stat rpc_stat;
-  u_long prognum, versnum, minvers, maxvers;
+  u_int32_t prognum, versnum, minvers, maxvers;
   struct rpc_err rpcerr;
   int failure = 0;
   struct netconfig *nconf;
-- 
1.5.2.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long
  2009-04-22 21:45 [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long Frank Filz
@ 2009-04-22 21:46 ` Chuck Lever
  2009-04-22 22:19   ` Frank Filz
  2009-04-22 21:56 ` Chuck Lever
  1 sibling, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2009-04-22 21:46 UTC (permalink / raw)
  To: Frank Filz; +Cc: NFS List, libtirpc, Steve Dickson

On Apr 22, 2009, at 5:45 PM, Frank Filz wrote:
> This fixes a problem where "rpcinfo -T transport host prognum" fails  
> on a PPC64
> because CLNT_CONTROL expects the version number to be a 32 bit  
> quantity. u_long
> probably works fine on little endian machines, but won't work on big  
> endian
> machines.

Hrm.  These variables are unsigned long pretty much everywhere in the  
legacy RPC code.  And, isn't SPARC big endian?  Is the real problem  
32- v. 64- bit?

This might be fixed more generically by adding a type cast to  
CLNT_CONTROL.

> Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
> ---
> src/rpcinfo.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/rpcinfo.c b/src/rpcinfo.c
> index 0170f65..698f4ca 100644
> --- a/src/rpcinfo.c
> +++ b/src/rpcinfo.c
> @@ -1591,7 +1591,7 @@ progping (netid, argc, argv)
>   CLIENT *client;
>   struct timeval to;
>   enum clnt_stat rpc_stat;
> -  u_long prognum, versnum, minvers, maxvers;
> +  u_int32_t prognum, versnum, minvers, maxvers;
>   struct rpc_err rpcerr;
>   int failure = 0;
>   struct netconfig *nconf;

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long
  2009-04-22 21:45 [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long Frank Filz
  2009-04-22 21:46 ` Chuck Lever
@ 2009-04-22 21:56 ` Chuck Lever
  2009-04-22 22:08   ` [Libtirpc-devel] " Chuck Lever
  1 sibling, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2009-04-22 21:56 UTC (permalink / raw)
  To: Frank Filz; +Cc: NFS List, libtirpc, Steve Dickson

On Apr 22, 2009, at 5:45 PM, Frank Filz wrote:
> This fixes a problem where "rpcinfo -T transport host prognum" fails  
> on a PPC64
> because CLNT_CONTROL expects the version number to be a 32 bit  
> quantity. u_long
> probably works fine on little endian machines, but won't work on big  
> endian
> machines.
>
>
> Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
> ---
> src/rpcinfo.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/src/rpcinfo.c b/src/rpcinfo.c
> index 0170f65..698f4ca 100644
> --- a/src/rpcinfo.c
> +++ b/src/rpcinfo.c
> @@ -1591,7 +1591,7 @@ progping (netid, argc, argv)
>   CLIENT *client;
>   struct timeval to;
>   enum clnt_stat rpc_stat;
> -  u_long prognum, versnum, minvers, maxvers;
> +  u_int32_t prognum, versnum, minvers, maxvers;

Right, looking at the TI-RPC type definitions, we really want:

   rpcprog_t prognum;
   rpcvers_t versnum, minvers, maxvers;

Now, this won't work if rpcinfo is built against the legacy RPC  
headers in /usr/include.  The TI-RPC headers define these types  
correctly as u_int32_t, but the legacy headers use u_long.

>   struct rpc_err rpcerr;
>   int failure = 0;
>   struct netconfig *nconf;
> -- 
> 1.5.2.2
>
>
>

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [Libtirpc-devel] [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long
  2009-04-22 21:56 ` Chuck Lever
@ 2009-04-22 22:08   ` Chuck Lever
  2009-04-22 22:36     ` [PATCH] rpcbind: Change prognum, versnum, minvers, and maxvers in progping to rpcprog_t and rpcvers_t " Frank Filz
  0 siblings, 1 reply; 7+ messages in thread
From: Chuck Lever @ 2009-04-22 22:08 UTC (permalink / raw)
  To: Frank Filz; +Cc: NFS List, Steve Dickson, libtirpc

On Apr 22, 2009, at 5:56 PM, Chuck Lever wrote:
> On Apr 22, 2009, at 5:45 PM, Frank Filz wrote:
>> This fixes a problem where "rpcinfo -T transport host prognum" fails
>> on a PPC64
>> because CLNT_CONTROL expects the version number to be a 32 bit
>> quantity. u_long
>> probably works fine on little endian machines, but won't work on big
>> endian
>> machines.
>>
>>
>> Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
>> ---
>> src/rpcinfo.c |    2 +-
>> 1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/src/rpcinfo.c b/src/rpcinfo.c
>> index 0170f65..698f4ca 100644
>> --- a/src/rpcinfo.c
>> +++ b/src/rpcinfo.c
>> @@ -1591,7 +1591,7 @@ progping (netid, argc, argv)
>>  CLIENT *client;
>>  struct timeval to;
>>  enum clnt_stat rpc_stat;
>> -  u_long prognum, versnum, minvers, maxvers;
>> +  u_int32_t prognum, versnum, minvers, maxvers;
>
> Right, looking at the TI-RPC type definitions, we really want:
>
>   rpcprog_t prognum;
>   rpcvers_t versnum, minvers, maxvers;
>
> Now, this won't work if rpcinfo is built against the legacy RPC
> headers in /usr/include.  The TI-RPC headers define these types
> correctly as u_int32_t, but the legacy headers use u_long.

I take that back.  Replacing u_long with rpcvers_t will work in both  
cases.  glibc's CLNT_CONTROL expects a u_long here, but TI-RPC expects  
an u_int32_t.  These match the typedef for rpcvers_t for each of these.

So, you have to make sure you are building with the headers that go  
with your RPC library.  The headers in /usr/include/rpc go with glibc,  
and the headers in /usr/include/tirpc/rpc go with libtirpc.

>>  struct rpc_err rpcerr;
>>  int failure = 0;
>>  struct netconfig *nconf;

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long
  2009-04-22 21:46 ` Chuck Lever
@ 2009-04-22 22:19   ` Frank Filz
  0 siblings, 0 replies; 7+ messages in thread
From: Frank Filz @ 2009-04-22 22:19 UTC (permalink / raw)
  To: Chuck Lever; +Cc: NFS List, libtirpc, Steve Dickson

On Wed, 2009-04-22 at 17:46 -0400, Chuck Lever wrote:
> On Apr 22, 2009, at 5:45 PM, Frank Filz wrote:
> > This fixes a problem where "rpcinfo -T transport host prognum" fails  
> > on a PPC64
> > because CLNT_CONTROL expects the version number to be a 32 bit  
> > quantity. u_long
> > probably works fine on little endian machines, but won't work on big  
> > endian
> > machines.
> 
> Hrm.  These variables are unsigned long pretty much everywhere in the  
> legacy RPC code.  And, isn't SPARC big endian?  Is the real problem  
> 32- v. 64- bit?

I at first though it would be a 64/32 bit problen, but now I think it
would still affect 32 bit, since unsigned long would still be a 64 bit
quantity, with the low order 32 bits being in the 5th through 8th bytes
on big endian, and therefore the 1st through 4th bytes which will be
accessed by casting a (void *) t (int *) would be 0.

> This might be fixed more generically by adding a type cast to  
> CLNT_CONTROL.

I did try a type cast at first, but it didn't work, though I might not
have done it right.

But look at these definitions from /usr/include/rpc/types.h:

/* This needs to be changed to uint32_t in the future */
typedef unsigned long rpcprog_t;
typedef unsigned long rpcvers_t;
typedef unsigned long rpcproc_t;
typedef unsigned long rpcprot_t;
typedef unsigned long rpcport_t;

Now look at the same definitions from /usr/include/tirpc/rpc/types.h:

typedef u_int32_t rpcprog_t;
typedef u_int32_t rpcvers_t;
typedef u_int32_t rpcproc_t;
typedef u_int32_t rpcprot_t;
typedef u_int32_t rpcport_t;

Actually, the patch probably should change the definitions to:

  rpcprog_t prognum;
  rpcvers_t versnum, minvers, maxvers;

(though internally, the CLNT_CONTROL is casting to u_int32_t, not
rpcvers_t or rpcprog_t).

It is curious though that in RFC 1833, there is this definition:

struct rpcb {
   unsigned long r_prog; /* program number */
   unsigned long r_vers; /* version number */
   string r_netid<>; /* network id */
   string r_addr<>; /* universal address */
   string r_owner<>; /* owner of this service */
};

But pretty much every where else, including the RPC protocol, prog and
vers are unsigned int...


> > Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
> > ---
> > src/rpcinfo.c |    2 +-
> > 1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/src/rpcinfo.c b/src/rpcinfo.c
> > index 0170f65..698f4ca 100644
> > --- a/src/rpcinfo.c
> > +++ b/src/rpcinfo.c
> > @@ -1591,7 +1591,7 @@ progping (netid, argc, argv)
> >   CLIENT *client;
> >   struct timeval to;
> >   enum clnt_stat rpc_stat;
> > -  u_long prognum, versnum, minvers, maxvers;
> > +  u_int32_t prognum, versnum, minvers, maxvers;
> >   struct rpc_err rpcerr;
> >   int failure = 0;
> >   struct netconfig *nconf;
> 
> --
> Chuck Lever
> chuck[dot]lever[at]oracle[dot]com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] rpcbind: Change prognum, versnum, minvers, and maxvers in progping to rpcprog_t and rpcvers_t from u_long
  2009-04-22 22:08   ` [Libtirpc-devel] " Chuck Lever
@ 2009-04-22 22:36     ` Frank Filz
  2009-04-22 22:37       ` Chuck Lever
  0 siblings, 1 reply; 7+ messages in thread
From: Frank Filz @ 2009-04-22 22:36 UTC (permalink / raw)
  To: Chuck Lever; +Cc: NFS List, Steve Dickson, libtirpc

On Wed, 2009-04-22 at 18:08 -0400, Chuck Lever wrote:
> I take that back.  Replacing u_long with rpcvers_t will work in both  
> cases.  glibc's CLNT_CONTROL expects a u_long here, but TI-RPC expects  
> an u_int32_t.  These match the typedef for rpcvers_t for each of these.
> 
> So, you have to make sure you are building with the headers that go  
> with your RPC library.  The headers in /usr/include/rpc go with glibc,  
> and the headers in /usr/include/tirpc/rpc go with libtirpc.

Ok, here's a patch that implements it that way. It works fine on PPC64.

This fixes a problem where "rpcinfo -T transport host prognum" fails on a PPC64
because CLNT_CONTROL expects the version number to be a 32 bit quantity. u_long
probably works fine on little endian machines, but won't work on big endian
machines.

Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
---
 src/rpcinfo.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/src/rpcinfo.c b/src/rpcinfo.c
index 0170f65..c9f51cf 100644
--- a/src/rpcinfo.c
+++ b/src/rpcinfo.c
@@ -1591,7 +1591,8 @@ progping (netid, argc, argv)
   CLIENT *client;
   struct timeval to;
   enum clnt_stat rpc_stat;
-  u_long prognum, versnum, minvers, maxvers;
+  rpcprog_t prognum;
+  rpcvers_t versnum, minvers, maxvers;
   struct rpc_err rpcerr;
   int failure = 0;
   struct netconfig *nconf;
-- 
1.5.2.2




^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] rpcbind: Change prognum, versnum, minvers, and maxvers in progping to rpcprog_t and rpcvers_t from u_long
  2009-04-22 22:36     ` [PATCH] rpcbind: Change prognum, versnum, minvers, and maxvers in progping to rpcprog_t and rpcvers_t " Frank Filz
@ 2009-04-22 22:37       ` Chuck Lever
  0 siblings, 0 replies; 7+ messages in thread
From: Chuck Lever @ 2009-04-22 22:37 UTC (permalink / raw)
  To: Frank Filz; +Cc: NFS List, Steve Dickson, libtirpc

On Apr 22, 2009, at 6:36 PM, Frank Filz wrote:
> On Wed, 2009-04-22 at 18:08 -0400, Chuck Lever wrote:
>> I take that back.  Replacing u_long with rpcvers_t will work in both
>> cases.  glibc's CLNT_CONTROL expects a u_long here, but TI-RPC  
>> expects
>> an u_int32_t.  These match the typedef for rpcvers_t for each of  
>> these.
>>
>> So, you have to make sure you are building with the headers that go
>> with your RPC library.  The headers in /usr/include/rpc go with  
>> glibc,
>> and the headers in /usr/include/tirpc/rpc go with libtirpc.
>
> Ok, here's a patch that implements it that way. It works fine on  
> PPC64.
>
> This fixes a problem where "rpcinfo -T transport host prognum" fails  
> on a PPC64
> because CLNT_CONTROL expects the version number to be a 32 bit  
> quantity. u_long
> probably works fine on little endian machines, but won't work on big  
> endian
> machines.

Thanks for testing.

The description is somewhat incorrect, though.  TI-RPC's CLNT_CONTROL  
expects the version number to be a 32-bit quantity.  glibc's RPC  
implementation of CLNT_CONTROL is just dandy with u_long.  I think  
that does bear some emphasis in the description.

Changing the types of these variables means that this code will now  
work unchanged with both glibc and TI-RPC (with a recompile, of course).

> Signed-off-by: Frank Filz <ffilzlnx@us.ibm.com>
> ---
> src/rpcinfo.c |    3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/src/rpcinfo.c b/src/rpcinfo.c
> index 0170f65..c9f51cf 100644
> --- a/src/rpcinfo.c
> +++ b/src/rpcinfo.c
> @@ -1591,7 +1591,8 @@ progping (netid, argc, argv)
>   CLIENT *client;
>   struct timeval to;
>   enum clnt_stat rpc_stat;
> -  u_long prognum, versnum, minvers, maxvers;
> +  rpcprog_t prognum;
> +  rpcvers_t versnum, minvers, maxvers;
>   struct rpc_err rpcerr;
>   int failure = 0;
>   struct netconfig *nconf;
> -- 
> 1.5.2.2
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs"  
> in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2009-04-22 22:38 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-22 21:45 [PATCH] Change prognum, versnum, minvers, and maxvers in progping to u_int32_t from u_long Frank Filz
2009-04-22 21:46 ` Chuck Lever
2009-04-22 22:19   ` Frank Filz
2009-04-22 21:56 ` Chuck Lever
2009-04-22 22:08   ` [Libtirpc-devel] " Chuck Lever
2009-04-22 22:36     ` [PATCH] rpcbind: Change prognum, versnum, minvers, and maxvers in progping to rpcprog_t and rpcvers_t " Frank Filz
2009-04-22 22:37       ` Chuck Lever

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.