From owner-svn-src-user@freebsd.org Thu Jan 21 06:21:19 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 52FBFA8B34C for ; Thu, 21 Jan 2016 06:21:19 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: from mail-oi0-x234.google.com (mail-oi0-x234.google.com [IPv6:2607:f8b0:4003:c06::234]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 156631CF6; Thu, 21 Jan 2016 06:21:19 +0000 (UTC) (envelope-from yaneurabeya@gmail.com) Received: by mail-oi0-x234.google.com with SMTP id o124so20487039oia.3; Wed, 20 Jan 2016 22:21:19 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=JfTdtUhS6yU+tot3Fzkx9k32GrCBmQw1v1Mqc+unJkg=; b=QzjG/MhTgujAhLAklFOSsNpaNoiMtPYeTw6SHSYXQ+PxAgxg0ot5edVyZcuvJp6B2L IRmHJQtX+2AOtCx3B+61Q46THpQJb7YI0ZFOGHXx1QNYtETRdHXANIjIgYlMdNlxZ5BT xJGBGT5AvGEja8M6uCADZq7ym6HtbG9rU61DsgsbUjzXg2moOF5kj+XC2zr58eUMCr7O 5CB8mtJHUzh77TD4TLmzZC0uxWyFXVXPeNMfU60wltkN7FYR6EJI1HS6L141IAQ7IAeG BtBMiKRuNSLXR6MfSb+4mz1ol1dQxKjWdkU/+RyJVkiex1bsn2Ch/JI6Y6BekIjLbGzM NwHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:content-type:mime-version:subject:from :in-reply-to:date:cc:content-transfer-encoding:message-id:references :to; bh=JfTdtUhS6yU+tot3Fzkx9k32GrCBmQw1v1Mqc+unJkg=; b=PiHNxmPpdJXXwYVfmrjDoH3AAFDPBy2EAIQavUCLWHKNWfeYD6ZDCxURxhs9NxMzOF 51P9pDRFhl1KZs6+1Z4jF21/SokNvHWPByiF3vw2zjfMDkoMC6dWFOiTTXhhUWOrcarN 3oeloRPypeN4Oi/7dAllB5FWEjOKd5M0SXB3xJ4odMUOEedF9vwTwpFlC4t6KZEPRK1C b6LLb2dGGbPK2EasyOtxvk8/jgDG8YHMgNG2yxflEuDlh1b4r7kb/KaynJJ//4CUJWrD atUR4uVYxSbn2xCe+82JfER5dEQNd0QCphpWc2WWbHlvuHfGJ2pHtz1jqnKqbDtLKOKH iQIQ== X-Gm-Message-State: ALoCoQn1aQVCVPU6fskCfO11YgIOWZCJRM9FusIkOy0aS+NUk2WiH/SEjoOa1PKUZ03gl2lMXIrBE0nBWoUsmp2cZEqXJ0R5Ig== X-Received: by 10.202.73.67 with SMTP id w64mr29643867oia.84.1453357278278; Wed, 20 Jan 2016 22:21:18 -0800 (PST) Received: from ?IPv6:2601:601:800:126d:8d00:ce96:4479:35a1? ([2601:601:800:126d:8d00:ce96:4479:35a1]) by smtp.gmail.com with ESMTPSA id v142sm19837280oie.28.2016.01.20.22.21.16 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 20 Jan 2016 22:21:16 -0800 (PST) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) Subject: Re: svn commit: r294246 - user/ngie/socket-tests/tools/regression/sockets/unix_cmsg From: NGie Cooper In-Reply-To: <20160118153831.K1354@besplex.bde.org> Date: Wed, 20 Jan 2016 22:21:14 -0800 Cc: Garrett Cooper , src-committers@freebsd.org, svn-src-user@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <347D22CE-6DDB-4AE9-879A-73743723B481@gmail.com> References: <201601180432.u0I4WJYa008708@repo.freebsd.org> <20160118153831.K1354@besplex.bde.org> To: Bruce Evans X-Mailer: Apple Mail (2.2104) 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: Thu, 21 Jan 2016 06:21:19 -0000 > On Jan 17, 2016, at 21:13, Bruce Evans wrote: >=20 > On Mon, 18 Jan 2016, Garrett Cooper wrote: >=20 >> Log: >> Checkpoint work to bump WARNS to 6 >>=20 >> Try to fix -Wcast-align issues by using bcopy to assign the values in >> the structure. >=20 > 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. >=20 > 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. >=20 > 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. I tried memmove and unfortunately it failed similar to bcopy. I think I = need to do =E2=80=9Cdeeper copying=E2=80=9D in the structures to ensure = that all of the fields are properly copied over in the bcopy, but I=E2=80=99= ll look at what gdb says first. >> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >> --- = 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; >=20 > The changes fix many style bugs (pointer variables not spelled with > a p) but leave many others (verbose variable names). Agreed. I=E2=80=99ll fix this in the next commit. >> @@ -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; >=20 > '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=E2=80=99. I agree in these cases. Unless it=E2=80=99s standard variable naming = convention for C (rc/retval/rv being shorthand for =E2=80=9Creturn_value=E2= =80=9D, etc), I try to stick with more verbose names to make the meaning = of variables unambiguous. I agree though=E2=80=A6 the variable name = `timeval` is not incredibly helpful to the reader... >> @@ -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; >=20 > Names related to bintimes are generally worse than for timevals, = starting > in the implementation not having prefixes in the struct member names. >=20 > Timespecs are in between. They have prefixes in the struct member = names, > but POSIX broke these from ts_ to tv_. Thanks for the feedback! -NGie=