From owner-svn-src-head@freebsd.org Mon Aug 1 15:50:38 2016 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 B6693BAB9C3 for ; Mon, 1 Aug 2016 15:50:38 +0000 (UTC) (envelope-from pfg@FreeBSD.org) Received: from nm4.bullet.mail.bf1.yahoo.com (nm4.bullet.mail.bf1.yahoo.com [98.139.212.163]) (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 7AEDC13BA for ; Mon, 1 Aug 2016 15:50:38 +0000 (UTC) (envelope-from pfg@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=yahoo.com; s=s2048; t=1470066631; bh=RsaYBCjRJCIjosOiolOgOb5zi8w8RhWjFJ+8IXpjqaE=; h=Subject:To:References:Cc:From:Date:In-Reply-To:From:Subject; b=QStc1K/zDyDuVpeml9wPO6W46fyoc7F1kr7YHs7FIkiVGAOINoNdNplHdJL8HDcvAh7KjEF1ndVUNLBsTOVvkP4K9mmO6D5ZPPEe2y9JF3sbjle05pwCAD5p+tU6IcMm3ZeuoyBby8D7XlhEEGnW6r2vy14fP4V+s1nkIM053qLa8ekY1UtcjuE+q7BLbV19SaHHb6Cvwts47SBI1aicK7FBcpoELtS/gnp+/VV/As/2wo3BH15aS0dQUJsfN2O7xJMGiZV7XUu9qh2Y0tUPTZ4qWUAaOFNRKgOoGnuZi4g21U3BEytPuOmidnutZUe6dLRUAEyRwXAnYJDgZEF9/A== Received: from [98.139.170.179] by nm4.bullet.mail.bf1.yahoo.com with NNFMP; 01 Aug 2016 15:50:31 -0000 Received: from [68.142.230.70] by tm22.bullet.mail.bf1.yahoo.com with NNFMP; 01 Aug 2016 15:50:31 -0000 Received: from [127.0.0.1] by smtp227.mail.bf1.yahoo.com with NNFMP; 01 Aug 2016 15:50:31 -0000 X-Yahoo-Newman-Id: 388207.28399.bm@smtp227.mail.bf1.yahoo.com X-Yahoo-Newman-Property: ymail-3 X-YMail-OSG: MwqOBrEVM1mE.X5MQgfYFTSozDKhd0aajj9q1yFozt0jSdW M7mQpZxYk7ROb2LfOwWuudBj9O2c.ExSYO9GPehd.HQjCPtjTCcy1PwlQ.AV u6p9zgo.pLOyrZlAWUZkgx.lqUpdsD5KBQ.yt3IFzTBMA52CEPi6gLvpM6CN 4vnyPgOrT8Chrcy58..kv.7YU2ey0lpUdoAlny7W9d.6M9lXVFuCA7MZF0lc g7vIsCnpcNjL1kjF0mY7R3iVo2lhaIKakfYo5GGtkMNPT0L1gdY6dngW8fED oCQ4LUDLmRTdXpxieaYAV8lXwk4Uplqr6.4Zbj0hTDmfW6SIwyZ3m5S0dP0k VcHhnECxvJnnd1sETPNejagoMoDO7N6YNHF22jjXD2Uok3OUnRM4nT70h0ec pz.Gq8NgR3XUvQINSSQ88USrUhM7Oan1MGuaauD465t3qrtRF1wL6QR2Uagj cjIhMYngWXticOiyvfe6pfIUPaAbSzHz2bF98cSFgtCYltKkS8IynlTEjI7Y CW9i7uBcygxDfAxk2290SgqQLqNv1My3OHn6ouo.MQpqWmw-- X-Yahoo-SMTP: xcjD0guswBAZaPPIbxpWwLcp9Unf Subject: Re: svn commit: r303600 - head/usr.bin/indent To: Piotr Stefaniak , Bruce Evans , Xin Li References: <201607312136.u6VLaeRb058693@repo.freebsd.org> <8d6114b7-cd71-239a-dcc4-03a6d568482c@delphij.net> <20160801155148.I884@besplex.bde.org> Cc: "svn-src-head@freebsd.org" , "d@delphij.net" , "svn-src-all@freebsd.org" , "src-committers@freebsd.org" From: Pedro Giffuni Message-ID: <93515f84-5c0e-2957-ce63-4a52ba09c50d@FreeBSD.org> Date: Mon, 1 Aug 2016 10:50:43 -0500 User-Agent: Mozilla/5.0 (X11; FreeBSD amd64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: 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.22 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: Mon, 01 Aug 2016 15:50:38 -0000 On 08/01/16 10:24, Piotr Stefaniak wrote: > On 2016-08-01 08:08, Bruce Evans wrote: >> On Sun, 31 Jul 2016, Xin Li wrote: >> >>> On 7/31/16 14:36, Pedro F. Giffuni wrote: > >>>> - bzero(f, sizeof *f); >>>> + memset(f, 0, sizeof(struct fstate)); >>> ^^^^^^^^^^^^^^^^^^^^^ This is much more error-prone >>> than sizeof(*f) IMHO. >> >> I also prefer bzero(). > > I hope this is merely a preference and not a hard rule, because I'm of > the opinion that the memset()-based equivalent of bzero() has fewer > portability consequences, which is worth paying attention to. Please > consider the fact that NetBSD has done this replacement. > It is a preference, not a hard rule. In general, replacing bzero with memset is seen as a portability enhancement but bzero is not going away from FreeBSD and replacing it is not a priority. > I do agree that replacing the expression with the type name was a > regression; it was my mistake. > >> Removal of the space after sizeof is another regression. KNF disallows >> the space, but indent's style is very far from KNF. It isn't clear if >> indent's style is to require the space, since old versions of indent >> never used sizeof(typename), and 'sizeof object' requires the space. > > I was specifically asked in the D6966 differential review to adhere to > style(9). I have changed both my own code submitted for review and the > rest of style violations of this kind as a separate patch > (https://github.com/pstef/freebsd_indent/commit/a2befd74fa54c91d96a38317e90d38ef17682f4b). > I had expected the style fixes to get committed before the change in > r303600, in belief that doing so would render possible complaints as the > one quoted above as not relevant anymore. > The idea is that new code should try to adhere to style(9) but it is not an obligation to do so if the codebase already uses a different style. >> Regressions started in r93440 with sizeof(object) in an nitems() expansion. >> The style of this is very different from an nitems() expansion in r1590. >> There was 1 more sizeof(object) and 1 sizeof(int). This is the first >> sizeof(typename) where 'sizeof object' cannot be used for technical >> reasons. >> >> KNF also requires parentheses for sizeof(object). Then the space is >> unnecessary and disallowed. > > On a more general note, I imagined we're heading towards slowly changing > indent(1)'s code to make it more style(9)-compliant (not least because > it's tempting to imagine indent(1) being able to re-indent itself in > accordance with style(9) at some point) but right now I'm confused as to > what style decisions in the patches I submit are expected of me. > You are doing a great job! In the case of indent, while it would be desirable to move it all to KNF, that involves way too many cosmetic changes that have no use. Reviewers can get things wrong, and in general it is up to the committer, IMO, style cleanups are generally very low priority. It is preferable to have smaller to-the-point changes than style issues mixed with effective code changes. Going out of your way to clean indent style --> KNF is a waste of time. IMHO, of course. Pedro.