svn commit: r241141 - in head: include/rpc lib/libc/rpc sys/rpc

2012-10-02 Thread Pedro F. Giffuni
Author: pfg
Date: Tue Oct  2 19:00:56 2012
New Revision: 241141
URL: http://svn.freebsd.org/changeset/base/241141

Log:
  RPC: Convert all uid and gid variables of the type uid_t and gid_t.
  
  This matches what upstream (OpenSolaris) does.
  
  Tested by:David Wolfskill
  Obtained from:Bull GNU/Linux NFSv4 project (libtirpc)
  MFC after:3 days

Modified:
  head/include/rpc/auth.h
  head/include/rpc/auth_unix.h
  head/lib/libc/rpc/auth_unix.c
  head/lib/libc/rpc/authunix_prot.c
  head/lib/libc/rpc/rpc_soc.3
  head/lib/libc/rpc/svc_auth_unix.c
  head/sys/rpc/auth.h

Modified: head/include/rpc/auth.h
==
--- head/include/rpc/auth.h Tue Oct  2 18:38:05 2012(r241140)
+++ head/include/rpc/auth.h Tue Oct  2 19:00:56 2012(r241141)
@@ -243,14 +243,13 @@ __END_DECLS
  * System style authentication
  * AUTH *authunix_create(machname, uid, gid, len, aup_gids)
  * char *machname;
- * int uid;
- * int gid;
+ * uid_t uid;
+ * gid_t gid;
  * int len;
- * int *aup_gids;
+ * gid_t *aup_gids;
  */
 __BEGIN_DECLS
-extern AUTH *authunix_create(char *, int, int, int,
-int *);
+extern AUTH *authunix_create(char *, uid_t, gid_t, int, gid_t *);
 extern AUTH *authunix_create_default(void);/* takes no parameters */
 extern AUTH *authnone_create(void);/* takes no parameters */
 __END_DECLS

Modified: head/include/rpc/auth_unix.h
==
--- head/include/rpc/auth_unix.hTue Oct  2 18:38:05 2012
(r241140)
+++ head/include/rpc/auth_unix.hTue Oct  2 19:00:56 2012
(r241141)
@@ -60,10 +60,10 @@
 struct authunix_parms {
u_long   aup_time;
char*aup_machname;
-   int  aup_uid;
-   int  aup_gid;
+   uid_taup_uid;
+   gid_taup_gid;
u_intaup_len;
-   int *aup_gids;
+   gid_t   *aup_gids;
 };
 
 #define authsys_parms authunix_parms

Modified: head/lib/libc/rpc/auth_unix.c
==
--- head/lib/libc/rpc/auth_unix.c   Tue Oct  2 18:38:05 2012
(r241140)
+++ head/lib/libc/rpc/auth_unix.c   Tue Oct  2 19:00:56 2012
(r241141)
@@ -94,10 +94,10 @@ struct audata {
 AUTH *
 authunix_create(machname, uid, gid, len, aup_gids)
char *machname;
-   int uid;
-   int gid;
+   uid_t uid;
+   gid_t gid;
int len;
-   int *aup_gids;
+   gid_t *aup_gids;
 {
struct authunix_parms aup;
char mymem[MAX_AUTH_BYTES];
@@ -207,9 +207,7 @@ authunix_create_default()
abort();
if (ngids  NGRPS)
ngids = NGRPS;
-   /* XXX: interface problem; those should all have been unsigned */
-   auth = authunix_create(machname, (int)uid, (int)gid, ngids,
-   (int *)gids);
+   auth = authunix_create(machname, uid, gid, ngids, gids);
free(gids);
return (auth);
 }

Modified: head/lib/libc/rpc/authunix_prot.c
==
--- head/lib/libc/rpc/authunix_prot.c   Tue Oct  2 18:38:05 2012
(r241140)
+++ head/lib/libc/rpc/authunix_prot.c   Tue Oct  2 19:00:56 2012
(r241141)
@@ -60,7 +60,7 @@ xdr_authunix_parms(xdrs, p)
XDR *xdrs;
struct authunix_parms *p;
 {
-   int **paup_gids;
+   gid_t **paup_gids;
 
assert(xdrs != NULL);
assert(p != NULL);
@@ -69,8 +69,8 @@ xdr_authunix_parms(xdrs, p)
 
if (xdr_u_long(xdrs, (p-aup_time))
 xdr_string(xdrs, (p-aup_machname), MAX_MACHINE_NAME)
-xdr_int(xdrs, (p-aup_uid))
-xdr_int(xdrs, (p-aup_gid))
+xdr_u_int(xdrs, (p-aup_uid))
+xdr_u_int(xdrs, (p-aup_gid))
 xdr_array(xdrs, (char **) paup_gids,
(p-aup_len), NGRPS, sizeof(int), (xdrproc_t)xdr_int) ) {
return (TRUE);

Modified: head/lib/libc/rpc/rpc_soc.3
==
--- head/lib/libc/rpc/rpc_soc.3 Tue Oct  2 18:38:05 2012(r241140)
+++ head/lib/libc/rpc/rpc_soc.3 Tue Oct  2 19:00:56 2012(r241141)
@@ -148,7 +148,7 @@ default authentication used by
 .Ft AUTH *
 .Xc
 .It Xo
-.Fn authunix_create char *host int uid int gid int len int *aup_gids
+.Fn authunix_create char *host uid_t uid gid_t gid int len gid_t 
*aup_gids
 .Xc
 .Pp
 Create and return an

Modified: head/lib/libc/rpc/svc_auth_unix.c
==
--- head/lib/libc/rpc/svc_auth_unix.c   Tue Oct  2 18:38:05 2012
(r241140)
+++ head/lib/libc/rpc/svc_auth_unix.c   Tue Oct  2 19:00:56 2012
(r241141)
@@ -68,7 +68,7 @@ _svcauth_unix(rqst, msg)
struct area {
struct authunix_parms area_aup;
 

Re: svn commit: r241141 - in head: include/rpc lib/libc/rpc sys/rpc

2012-10-02 Thread Bruce Evans

On Tue, 2 Oct 2012, Pedro F. Giffuni wrote:


Log:
 RPC: Convert all uid and gid variables of the type uid_t and gid_t.

 This matches what upstream (OpenSolaris) does.

 Tested by: David Wolfskill
 Obtained from: Bull GNU/Linux NFSv4 project (libtirpc)
 MFC after: 3 days


This still assumes that uid_t and gid_t are precisely u_int, in stronger
ways than before.


Modified: head/lib/libc/rpc/authunix_prot.c
==
--- head/lib/libc/rpc/authunix_prot.c   Tue Oct  2 18:38:05 2012
(r241140)
+++ head/lib/libc/rpc/authunix_prot.c   Tue Oct  2 19:00:56 2012
(r241141)
@@ -60,7 +60,7 @@ xdr_authunix_parms(xdrs, p)
XDR *xdrs;
struct authunix_parms *p;
{
-   int **paup_gids;
+   gid_t **paup_gids;

assert(xdrs != NULL);
assert(p != NULL);
@@ -69,8 +69,8 @@ xdr_authunix_parms(xdrs, p)

if (xdr_u_long(xdrs, (p-aup_time))
 xdr_string(xdrs, (p-aup_machname), MAX_MACHINE_NAME)
-xdr_int(xdrs, (p-aup_uid))
-xdr_int(xdrs, (p-aup_gid))
+xdr_u_int(xdrs, (p-aup_uid))
+xdr_u_int(xdrs, (p-aup_gid))
 xdr_array(xdrs, (char **) paup_gids,
(p-aup_len), NGRPS, sizeof(int), (xdrproc_t)xdr_int) ) {
return (TRUE);



xdr doesn't support arbitrary types.  Here the very name of xdr_u_int()
indicates that it only works on u_int's.  Its second arg must be a
pointer to u_int (misspelled unsigned in the man page, so it doesn't
match the function name in a different, harmless way).  The arg used
to be a pointer to an int, and the call to xdr_int() used to match that.
The arg is now a pointer to a uid_t or gid_t, and the call to xdr_u_int()
only matches that accidentally.  (The types happen to be uint32_t, which
happens to be u_int.)

More careful code would select an xdr translation function based on
sizeof(uid_t) etc.

The above xdr_array() call takes a element size arg that is necessary
for stepping through the array, but isn't careful to use sizeof() on
the correct type.  It uses sizeof() on a hard-coded type, and you just
changed the element type without changing the hard-coded type.  It used
to match (int == int), but now doesn't (int != gid_t).  sizeof() should
be applied to objects and not types to get the object size right without
hard-coding its type.

The first type of type error should be detected at compile time.  The
second one (the hard-coded sizeof(int)) probably cannot be.  And there
is yet another new type error in the xdr_array() call.  It takes a
pointer to a translation function.  The function used to match the elemennt
type, but now doesn't (int != gid_t, and also int != underlying type
of gid_t == u_int).  The API requires casting the pointer to a generic
one using an obfuscated typedef, so the compiler cannot detect this
type mismatch at compile time (without breaking the API generally).

Bruce
___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org


Re: svn commit: r241141 - in head: include/rpc lib/libc/rpc sys/rpc

2012-10-02 Thread Pedro Giffuni

Thank you Bruce;


On 10/02/2012 17:19, Bruce Evans wrote:

On Tue, 2 Oct 2012, Pedro F. Giffuni wrote:


Log:
 RPC: Convert all uid and gid variables of the type uid_t and gid_t.

 This matches what upstream (OpenSolaris) does.

 Tested by:David Wolfskill
 Obtained from:Bull GNU/Linux NFSv4 project (libtirpc)
 MFC after:3 days


This still assumes that uid_t and gid_t are precisely u_int, in stronger
ways than before.


Modified: head/lib/libc/rpc/authunix_prot.c
== 

--- head/lib/libc/rpc/authunix_prot.cTue Oct  2 18:38:05 2012
(r241140)
+++ head/lib/libc/rpc/authunix_prot.cTue Oct  2 19:00:56 2012
(r241141)

@@ -60,7 +60,7 @@ xdr_authunix_parms(xdrs, p)
XDR *xdrs;
struct authunix_parms *p;
{
-int **paup_gids;
+gid_t **paup_gids;

assert(xdrs != NULL);
assert(p != NULL);
@@ -69,8 +69,8 @@ xdr_authunix_parms(xdrs, p)

if (xdr_u_long(xdrs, (p-aup_time))
 xdr_string(xdrs, (p-aup_machname), MAX_MACHINE_NAME)
- xdr_int(xdrs, (p-aup_uid))
- xdr_int(xdrs, (p-aup_gid))
+ xdr_u_int(xdrs, (p-aup_uid))
+ xdr_u_int(xdrs, (p-aup_gid))
 xdr_array(xdrs, (char **) paup_gids,
(p-aup_len), NGRPS, sizeof(int), (xdrproc_t)xdr_int) ) {
return (TRUE);



xdr doesn't support arbitrary types.  Here the very name of xdr_u_int()
indicates that it only works on u_int's.  Its second arg must be a
pointer to u_int (misspelled unsigned in the man page, so it doesn't
match the function name in a different, harmless way).  The arg used
to be a pointer to an int, and the call to xdr_int() used to match that.
The arg is now a pointer to a uid_t or gid_t, and the call to xdr_u_int()
only matches that accidentally.  (The types happen to be uint32_t, which
happens to be u_int.)



OK, to solve this I was thinking of adding a cast to uint32_t, but this is
ugly and doesn't really do nothing. Looking at OpenSolaris they do a
cast but more than a cosmetical fix it is a requirement because they
still use xdr_int there. This sounds like a better approach for us too.



More careful code would select an xdr translation function based on
sizeof(uid_t) etc.

The above xdr_array() call takes a element size arg that is necessary
for stepping through the array, but isn't careful to use sizeof() on
the correct type.  It uses sizeof() on a hard-coded type, and you just
changed the element type without changing the hard-coded type.  It used
to match (int == int), but now doesn't (int != gid_t).  sizeof() should
be applied to objects and not types to get the object size right without
hard-coding its type.



Nice catch, that is certainly wrong, and libtirpc still has it. This isn't
necessary when adopting the solution above though.



The first type of type error should be detected at compile time.  The
second one (the hard-coded sizeof(int)) probably cannot be.  And there
is yet another new type error in the xdr_array() call.  It takes a
pointer to a translation function.  The function used to match the 
elemennt

type, but now doesn't (int != gid_t, and also int != underlying type
of gid_t == u_int).  The API requires casting the pointer to a generic
one using an obfuscated typedef, so the compiler cannot detect this
type mismatch at compile time (without breaking the API generally).



I only changed authunix_create and its parameters, any other underlying
issue is preexistent :).

Pedro.



___
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to svn-src-head-unsubscr...@freebsd.org