Date: Wed, 3 Oct 2012 08:19:34 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: "Pedro F. Giffuni" <pfg@freebsd.org> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r241141 - in head: include/rpc lib/libc/rpc sys/rpc Message-ID: <20121003075143.N938@besplex.bde.org> In-Reply-To: <201210021900.q92J0uHT088549@svn.freebsd.org> References: <201210021900.q92J0uHT088549@svn.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121003075143.N938>