From owner-svn-src-head@freebsd.org Sat Mar 11 04:58:15 2017 Return-Path: Delivered-To: svn-src-head@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 66E5BD07106 for ; Sat, 11 Mar 2017 04:58:15 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from nm25-vm6.bullet.mail.ne1.yahoo.com (nm25-vm6.bullet.mail.ne1.yahoo.com [98.138.91.118]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 379441FEF for ; Sat, 11 Mar 2017 04:58:15 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1489208288; bh=LVD8RGdyPg8vqWjqji88yWh7VSG65RwlT6QCdB+cI5M=; h=Subject:To:References:Cc:From:Date:In-Reply-To:From:Subject; b=YHa/93ema+rFCdtSs2+tiUBI7nuImaeARJehvBfRyBho+MoC4/XSaAHuKLNg+q2aKP4WjTfzej48Pn0LxvNC8uAel8XxVuKjHAEmwJftr2JqTMXGlbgNzHos+SJj8VO/VBIaLUaruOBudmWKGDd2EAmq3ozY4TsTTOKbYoJfBssgsStmdtNlVRd6oZwa+nn8V4P3UJFnGCPrVrGUp5h5r1TQ9JaX8mkhm8thBlD28hkziKDZVhAgfoXuGewxPkCbLGZTkhjwZt7vIQwv7z1HOvyWIlhr79fwAyQ3jW04jtAAokVzejG2DIFJVVHyBa4OCYLOabPdIfhjzlcu86dF4A== Received: from [98.138.100.116] by nm25.bullet.mail.ne1.yahoo.com with NNFMP; 11 Mar 2017 04:58:08 -0000 Received: from [98.138.104.116] by tm107.bullet.mail.ne1.yahoo.com with NNFMP; 11 Mar 2017 04:58:08 -0000 Received: from [127.0.0.1] by smtp225.mail.ne1.yahoo.com with NNFMP; 11 Mar 2017 04:58:08 -0000 X-Yahoo-Newman-Id: 236259.48113.bm@smtp225.mail.ne1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: UJGu3RcVM1lOGrRfCnzABRk9xLkPNygfinvsPbXstNkEjdD QNEkNETsuu1_BUlX.rGHA_OEtZedldIa_lG0d3Aa.Jv41_ke7oJvzoVo_rzJ zpT_ksh4P92Sxf6NZKm9GiZ0ATPalLxwv8X_qOLk05f8ci3tiyuj86cC5XRs 6tZ6LXqJfXNEi1ETW_dlVaCWhmKXyB8XaqGYnJq48UsZ1GNb773jLT39fSlw 6maCcrpnxYOCJvTx_ZY_Z3Wn6rFuK1MkFOTIReZ2VpmfA4dzjuQYPkZa1DGm kMDfkAvV_llwEu9gl.4XTcXdQfFrHmpKSxySD3lcGexgKdO8y0t9QvEERLqw I_BwzxNPIhCUm4boVRy0eCt31wMVINrpbUCfsMdW7aat.MsrYW2BEgqgJLEK glakwGtSgE6dF0q67thwjRc9.pmXd7LfslTMs1Tlj9GzjxJAT8LQPVG_GFOR IAOBpaZUpkQosOFTF0UvAybkqAhRR2tUOlGff5fIBQQD8DuspajOVPmonT.k Eiz41Dt0TSpLZBYdwTRcgc.A3VhCV2c94C3unzlF438hm8.4- X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf Subject: Re: svn commit: r314989 - head/usr.bin/vmstat To: Bruce Evans , Ngie Cooper References: <201703100449.v2A4neTK046456@repo.freebsd.org> <20170310181404.C1416@besplex.bde.org> <0906af1d-09aa-fd1e-35cc-9361a68fc160@FreeBSD.org> <20170311120737.R1001@besplex.bde.org> Cc: Marcelo Araujo , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org From: Pedro Giffuni Message-ID: <1acd3d70-acf9-27a2-3a0f-874846d3c1f0@FreeBSD.org> Date: Sat, 11 Mar 2017 00:01:12 -0500 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20170311120737.R1001@besplex.bde.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 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: Sat, 11 Mar 2017 04:58:15 -0000 On 3/10/2017 8:29 PM, Bruce Evans wrote: > On Fri, 10 Mar 2017, Ngie Cooper wrote: > >>> On Mar 10, 2017, at 03:59, Pedro Giffuni wrote: >>> >>>> On 3/10/2017 2:45 AM, Bruce Evans wrote: >>>>> On Fri, 10 Mar 2017, Marcelo Araujo wrote: >>>>> >>>>> ... >>>>> Log: >>>>> Use nitems() from sys/param.h and also remove the cast. >>>>> >>>>> Reviewed by: markj >>>>> MFC after: 3 weeks. >>>>> Differential Revision: https://reviews.freebsd.org/D9937 >>>>> ... >>>>> Modified: head/usr.bin/vmstat/vmstat.c >>>>> ============================================================================== >>>>> >>>>> --- head/usr.bin/vmstat/vmstat.c Fri Mar 10 04:30:31 2017 >>>>> (r314988) >>>>> +++ head/usr.bin/vmstat/vmstat.c Fri Mar 10 04:49:40 2017 >>>>> (r314989) >>>>> @@ -288,17 +288,13 @@ retry_nlist: >>>>> namelist[X_SUM].n_name = "_cnt"; >>>>> goto retry_nlist; >>>>> } >>>>> - for (c = 0; >>>>> - c < (int)(sizeof(namelist)/sizeof(namelist[0])); >>>>> - c++) >>>>> + for (c = 0; c < nitems(namelist); c++) >>>>> if (namelist[c].n_type == 0) >>>>> bufsize += strlen(namelist[c].n_name) + 1; >>>> >>>> This undoes fixes to compile at WARNS=2 in r87690 and now breaks at >>>> WARNS=3. >>>> vmstat is still compiled at WARNS=1. >>>> >>>> nitems suffers from the same unsigned poisoning as the sizeof() >>>> expression >>>> (since it reduces to the same expression. Casting to int in the >>>> expression >>>> to fix the warning would break exotic cases. Of course, nitems is >>>> undocumented so no one knows when it is supposed to work). >>>> >>>> vmstat compiles with no errors at WARNS=2. At WARNS=3, it used to >>>> compile >>>> with 9 excessive warnings (about the good style of omitting redundant >>>> initializers). Now it compiles with 10 excessive warnings. 1 more >>>> about >>>> comparison between signed unsigned. This warning is a compiler >>>> bug. Both >>>> gcc-4.2.1 and clang-3.9.0 have it. It is enabled by -W, which is >>>> put in >>>> CFLAGS at WARNS >= 3, or by -Wsign-compare. >>>> >>>> These compilers even complain about: >>>> >>>> int c; >>>> >>>> for (c = 0; c < 1U; c++) >>>> foo(); >>>> >>>> where it is extremely clear that c never gets converted to a wrong >>>> value >>>> when it is promoted to unsigned for the comparison. Compilers should >>>> only warn about sign mismatches if they can't figure out the ranges or >>>> if they can figure out the ranges but dangerous promotiions occur. >>>> Compilers do excessive unrolling and other optimizations of loops like >>>> the above, and they must figure out the ranges for this. >>>> >>> >>> I haven't looked at the code but it would seem like you can unsign c >>> and avoid the cast. > > That would be much worse than the cast. The cast is at least in the > right place -- it unpoisons nitimems. "Unsigning" c would poison C. > >> Yeah. This might introduce a domino effect though of changes. > As I said I hadn't looked at the code; the "c" is holding the return value of getopt() and later kvm_nlist() before getting reused as a loop index so it must be an int. I would think it is such reuse of the same variable which is poisonous, although it does involve some efficiency. Pedro. > Domino effect = fast poisoning. The dominos collapse immediately, but > poisoning takes a long time to spread. In K&R-1 size_t didn't even > exist and the type of sizeof() was unspecified. size_t was born with > unsign poisoning a little later. That is almost 40 years ago, but its > poisoning hasn't spread to many loop variables yet. Full poisoning would > spread it to the closure of all uses of the loop variables, or better > stopped at the source of the poisoning. In the above, the variable is > not used outside of the loop, so the closure is small and easy to see. > > Bruce