From owner-svn-src-head@FreeBSD.ORG Tue Oct 2 22:19:38 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 36FE2106566B; Tue, 2 Oct 2012 22:19:38 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail04.syd.optusnet.com.au (mail04.syd.optusnet.com.au [211.29.132.185]) by mx1.freebsd.org (Postfix) with ESMTP id BED308FC0C; Tue, 2 Oct 2012 22:19:37 +0000 (UTC) Received: from c122-106-157-84.carlnfd1.nsw.optusnet.com.au (c122-106-157-84.carlnfd1.nsw.optusnet.com.au [122.106.157.84]) by mail04.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id q92MJYD8017365 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 3 Oct 2012 08:19:35 +1000 Date: Wed, 3 Oct 2012 08:19:34 +1000 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" In-Reply-To: <201210021900.q92J0uHT088549@svn.freebsd.org> Message-ID: <20121003075143.N938@besplex.bde.org> References: <201210021900.q92J0uHT088549@svn.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed 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 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Oct 2012 22:19:38 -0000 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