From owner-svn-src-head@FreeBSD.ORG Wed Oct 3 01:52:32 2012 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F06BF1065670 for ; Wed, 3 Oct 2012 01:52:31 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from nm3-vm2.bullet.mail.ne1.yahoo.com (nm3-vm2.bullet.mail.ne1.yahoo.com [98.138.91.19]) by mx1.freebsd.org (Postfix) with SMTP id 72A3D8FC12 for ; Wed, 3 Oct 2012 01:52:31 +0000 (UTC) Received: from [98.138.90.52] by nm3.bullet.mail.ne1.yahoo.com with NNFMP; 03 Oct 2012 01:47:03 -0000 Received: from [98.138.226.124] by tm5.bullet.mail.ne1.yahoo.com with NNFMP; 03 Oct 2012 01:47:03 -0000 Received: from [127.0.0.1] by smtp203.mail.ne1.yahoo.com with NNFMP; 03 Oct 2012 01:47:03 -0000 X-Yahoo-Newman-Id: 103667.39632.bm@smtp203.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: BvwRHK0VM1mGHOwifzX1J.Sj_1TxxEn68J6GfEm2DGeeKAq kttTPVL3wuYGgYvAwZwoPL7M6L9bOllY_vAuCC.4v23ojHwKEqzZHQr5u6D6 Ggg1kbFlOz3B4pcgS_1VDNUciG2GcRjEt9vpvfPeR1wgYx24fhCXEttWIeXC 0DnUHbxNt_9.GJpLFCFYJ.i8zKp9a4kR2_kEQ.dYG_eNv6.bzrcSwvOonLvP ylQ9ng86.XPwbInPrHgMzOL_fzPoslcbiM1FZDIZeZHUQEgb4t7boH_CigNY V6C9u2AJmpLgePJHwXzTPIe1rQxXA1XQyHggAPPfoF1KpgfGv0cQU8l1I5mh d0gnXy0TQ34KNhlheEHBgPGcY9pH6.7IfuA0HdaGBB6hiAJAEYYvC9Ch0qQc z5YQeXkprdzSLOJsLrSDUKEiB1CH7KFjjlBV.kUbc9f18OMIlBYpW1ca5Bzr B1bGIS0ygz7k- X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf Received: from [192.168.10.101] (pfg@200.118.157.7 with plain) by smtp203.mail.ne1.yahoo.com with SMTP; 02 Oct 2012 18:47:02 -0700 PDT Message-ID: <506B991E.6010207@FreeBSD.org> Date: Tue, 02 Oct 2012 20:47:10 -0500 From: Pedro Giffuni User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:13.0) Gecko/20120621 Thunderbird/13.0.1 MIME-Version: 1.0 To: Bruce Evans References: <201210021900.q92J0uHT088549@svn.freebsd.org> <20121003075143.N938@besplex.bde.org> In-Reply-To: <20121003075143.N938@besplex.bde.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: Wed, 03 Oct 2012 01:52:32 -0000 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.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.) > 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.