From owner-svn-src-head@freebsd.org Fri Jan 19 04:14:41 2018 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 5179AEC0967; Fri, 19 Jan 2018 04:14:41 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail108.syd.optusnet.com.au (mail108.syd.optusnet.com.au [211.29.132.59]) by mx1.freebsd.org (Postfix) with ESMTP id F348570466; Fri, 19 Jan 2018 04:14:39 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from [192.168.0.102] (c110-21-101-228.carlnfd1.nsw.optusnet.com.au [110.21.101.228]) by mail108.syd.optusnet.com.au (Postfix) with ESMTPS id 8AB1B1A2EA4; Fri, 19 Jan 2018 15:14:37 +1100 (AEDT) Date: Fri, 19 Jan 2018 15:14:36 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Eric van Gyzen cc: Gleb Smirnoff , Konstantin Belousov , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r327354 - head/sys/vm In-Reply-To: <601ee1a2-8f4e-518d-4c86-89871cd652af@vangyzen.net> Message-ID: <20180119131452.W1088@besplex.bde.org> References: <201712291905.vBTJ57gI072871@repo.freebsd.org> <20180117224054.GO8113@FreeBSD.org> <601ee1a2-8f4e-518d-4c86-89871cd652af@vangyzen.net> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.2 cv=LKgWeNe9 c=1 sm=1 tr=0 a=PalzARQSbocsUSjMRkwAPg==:117 a=PalzARQSbocsUSjMRkwAPg==:17 a=kj9zAlcOel0A:10 a=6I5d2MoRAAAA:8 a=2I0WkOh0JBamTqr7hv8A:9 a=CjuIK1q_8ugA:10 a=IjZwj45LgO3ly-622nXo:22 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.25 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: Fri, 19 Jan 2018 04:14:41 -0000 On Thu, 18 Jan 2018, Eric van Gyzen wrote: > On 01/17/2018 16:40, Gleb Smirnoff wrote: >> On Fri, Dec 29, 2017 at 07:05:07PM +0000, Konstantin Belousov wrote: >> K> Author: kib >> K> Date: Fri Dec 29 19:05:07 2017 >> K> New Revision: 327354 >> K> URL: https://svnweb.freebsd.org/changeset/base/327354 >> K> >> K> Log: >> K> Style. >> K> >> K> Reviewed by: alc >> K> Sponsored by: The FreeBSD Foundation >> K> MFC after: 1 week >> K> Differential revision: https://reviews.freebsd.org/D13678 >> K> >> K> Modified: >> K> head/sys/vm/vm_swapout.c >> K> >> K> Modified: head/sys/vm/vm_swapout.c >> K> ============================================================================== >> K> --- head/sys/vm/vm_swapout.c Fri Dec 29 18:42:55 2017 (r327353) >> K> +++ head/sys/vm/vm_swapout.c Fri Dec 29 19:05:07 2017 (r327354) >> K> @@ -556,16 +556,14 @@ vm_thread_swapin(struct thread *td) >> K> { >> K> vm_object_t ksobj; >> K> vm_page_t ma[KSTACK_MAX_PAGES]; >> K> - int pages; >> K> + int a, count, i, j, pages, rv; >> K> >> K> pages = td->td_kstack_pages; >> K> ksobj = td->td_kstack_obj; >> K> VM_OBJECT_WLOCK(ksobj); >> K> (void)vm_page_grab_pages(ksobj, 0, VM_ALLOC_NORMAL | VM_ALLOC_WIRED, ma, >> K> pages); >> K> - for (int i = 0; i < pages;) { >> K> - int j, a, count, rv; >> K> - >> K> + for (i = 0; i < pages;) { >> K> vm_page_assert_xbusied(ma[i]); >> K> if (ma[i]->valid == VM_PAGE_BITS_ALL) { >> K> vm_page_xunbusy(ma[i]); >> >> Yeah, style is sacred, but is there a single person on Earth who would >> not agree that moving variables from smaller blocks to function block >> reduces readability of the code? > > I agree that it reduces the readability. Not only that, it also > encourages real bugs by allowing access to the variable when it does not > make sense. I disagree that it reduces the readability. Not only that, it also encourages real bugs by shadowing variables in outer scope, unless a limited scope is _always_ used for variables of the same name. Just the smaller code size increases the readability. The above is only 2 lines shorter, but without the C++ declaration the nested version would be much larger and require ugly indentation to declare i: /* Next line is not 1TBS. */ { /* Everything marches to the right. */ int i; for (i = 0; i < pages;) { /* Random order of next line not fixed. */ int j, a, count, rv; vm_page_assert_xbusied(ma[i]); .... } C++ also allows the larger style bug of declaring variables in the same scope (after statements). This doesn't even give the benefit of limiting the scope to the end of the block. It does limit the scope to after the declaration, but that is obvious from when the variable is initialized, except with the style bug of initializing variables in declarations. I use C++ style declarations only in temporary code since they are easier to write but harder to read. Bruce