Date: Fri, 6 Jul 2018 03:48:51 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Brooks Davis <brooks@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r336002 - in head: usr.bin/netstat usr.sbin/tcpdrop Message-ID: <20180706031434.G2327@besplex.bde.org> In-Reply-To: <201807051702.w65H2AXB058831@repo.freebsd.org> References: <201807051702.w65H2AXB058831@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 5 Jul 2018, Brooks Davis wrote: > Log: > Work around lame warnings in ancient gcc on 32-bit platforms. > > Fixes r335979. These are correct warnings. Why not fix the bug instead of rorking around it? > Modified: head/usr.bin/netstat/inet.c > ============================================================================== > --- head/usr.bin/netstat/inet.c Thu Jul 5 16:43:15 2018 (r336001) > +++ head/usr.bin/netstat/inet.c Thu Jul 5 17:02:10 2018 (r336002) > @@ -159,12 +159,12 @@ sotoxsocket(struct socket *so, struct xsocket *xso) > > bzero(xso, sizeof *xso); > xso->xso_len = sizeof *xso; > - xso->xso_so = (kvaddr_t)so; > + xso->xso_so = (kvaddr_t)(long)so; This (and kvaddr_t having the same 64-bit size for all arches) are unportable. Converting object pointers to integers is only portable if one or both of intptr_t or uintptr_t exists and is used. The above assumes that intptr_t exists and is the same as long, and that there are no problems with the sign mismatch between long and kvaddr_t. But long != intptr_t on any supported 32-bit arches. It only has the same size. The sign mismatch probably doesn't matter, but this is unclear. Correct code: xso->xso_so = (kvaddr_t)(uintptr_t)so; This only assumes that uintptr_t exists and is not larger than kvaddr_t. FreeBSD and maybe POSIX require uintptr_t to exist, and kvaddr_t must be chosen to be no smaller than uintptr_t. The burden is larger for using these pointers represented as integers. On all 32-bit arches, converting kvaddr_t back to a pointer reduces its size, and only very broken compilers would not warn about that. But it is a bug for the API to even have kernel pointers, so hopefully this conversion is rarely done. An application might just print these pointers, and should do this without converting them to void so as to misprint them using %p format. ps(1) attempts to do some compression when printing kernel addresses, but IIRC this is quite broken especially on 64-bit arches where it is most needed. E.g, ps laxw -o wchan. wchan is documented to trim leading zeros when printing the value numerically. It needs to trim leading f's on at least amd64 and i386 3/1. But wchan is almost useless since it is rarely printed numerically, so it is the same as wchan for all processes on freefall now. The leading zeros in it are stripped by printing it in %lx format after bogusly casting ki_wchan to long. ki_wchan is still void *, so kinfo's ABI is very MD. kinfo has lots of other design errors like this, but its ABI has only been broken once or twice since FreeBSD-~5. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180706031434.G2327>