From owner-svn-src-user@freebsd.org Mon Jan 18 05:34:36 2016 Return-Path: Delivered-To: svn-src-user@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 19697A8687D for ; Mon, 18 Jan 2016 05:34:36 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail107.syd.optusnet.com.au (mail107.syd.optusnet.com.au [211.29.132.53]) by mx1.freebsd.org (Postfix) with ESMTP id B8FE31B02; Mon, 18 Jan 2016 05:34:35 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c211-30-166-197.carlnfd1.nsw.optusnet.com.au (c211-30-166-197.carlnfd1.nsw.optusnet.com.au [211.30.166.197]) by mail107.syd.optusnet.com.au (Postfix) with ESMTPS id 1DA29D40A3E; Mon, 18 Jan 2016 16:13:15 +1100 (AEDT) Date: Mon, 18 Jan 2016 16:13:15 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Garrett Cooper cc: src-committers@freebsd.org, svn-src-user@freebsd.org Subject: Re: svn commit: r294246 - user/ngie/socket-tests/tools/regression/sockets/unix_cmsg In-Reply-To: <201601180432.u0I4WJYa008708@repo.freebsd.org> Message-ID: <20160118153831.K1354@besplex.bde.org> References: <201601180432.u0I4WJYa008708@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=cK4dyQqN c=1 sm=1 tr=0 a=KA6XNC2GZCFrdESI5ZmdjQ==:117 a=PO7r1zJSAAAA:8 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=JzwRw_2MAAAA:8 a=kj9zAlcOel0A:10 a=oZL5hqpM5VHBIK8RlYQA:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-user@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the experimental " user" src tree" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 18 Jan 2016 05:34:36 -0000 On Mon, 18 Jan 2016, Garrett Cooper wrote: > Log: > Checkpoint work to bump WARNS to 6 > > Try to fix -Wcast-align issues by using bcopy to assign the values in > the structure. Using the standard memmove() instead of the BSD bcopy() might be considered a style bug. However, this is a case where memmove() works better. The compiler normally replaces it by __builtin_mmemove() which can be optimized better for small fixed-size copies. On arches without strict alignment requirements, this normally results in the same loads and stores that the pointer hack would give. However2, the kernel build environment is not normal, and changes related to this give a mess of style bugs and negative optimizations instead of the indended positive ones. Long ago, the style was broken by changing some bcopy()s (mainly in networking code) to memcpy()s and adding a fallback extern memcpy() for compilers that don't do the optimization (mainly for the -O0 case). Before that, it was easier to enforce the style by not supporting memcpy(). This was turned into less than a style bug by adding -ffreestanding to CFLAGS (mainly to fix the compiler replacing printf() by __builtin_printf() and turning that into puts() in some cases). -ffreestanding is technically correct, but it disables _all_ builtins. This made the changes to use __builtin_memcpy() less than style bugs since the fallback memcpy() didn't attempt to be optimal. However, it is more optimal than bcopy() for small copies on some arches. No one cares about the lost builtins since the optimizations that they give are tiny. Later, many more mem*() functions were added, mainly to support contribed software that doesn't follow BSD style. The result is a mess of different styles in non-contrib'ed code too. > ============================================================================== > --- user/ngie/socket-tests/tools/regression/sockets/unix_cmsg/unix_cmsg.c Mon Jan 18 04:24:43 2016 (r294245) > +++ user/ngie/socket-tests/tools/regression/sockets/unix_cmsg/unix_cmsg.c Mon Jan 18 04:32:19 2016 (r294246) > @@ -1027,60 +1027,60 @@ check_xucred(const struct xucred *xucred > static int > check_scm_creds_cmsgcred(struct cmsghdr *cmsghdr) > { > - const struct cmsgcred *cmsgcred; > + struct cmsgcred cmsgcred; The changes fix many style bugs (pointer variables not spelled with a p) but leave many others (verbose variable names). > @@ -1145,15 +1145,15 @@ check_scm_creds_sockcred(struct cmsghdr > static int > check_scm_timestamp(struct cmsghdr *cmsghdr) > { > - const struct timeval *timeval; > + const struct timeval timeval; 'timeval' is a bad enough name for a struct tag. 'timeval' is a worse variable name since it is the same as the struct tag. The normal name is 'tv'. > @@ -1161,15 +1161,15 @@ check_scm_timestamp(struct cmsghdr *cmsg > static int > check_scm_bintime(struct cmsghdr *cmsghdr) > { > - const struct bintime *bintime; > + const struct bintime bintime; Names related to bintimes are generally worse than for timevals, starting in the implementation not having prefixes in the struct member names. Timespecs are in between. They have prefixes in the struct member names, but POSIX broke these from ts_ to tv_. Bruce