Date: Fri, 19 Jan 2018 15:14:36 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Eric van Gyzen <eric@vangyzen.net> Cc: Gleb Smirnoff <glebius@freebsd.org>, Konstantin Belousov <kib@freebsd.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r327354 - head/sys/vm Message-ID: <20180119131452.W1088@besplex.bde.org> In-Reply-To: <601ee1a2-8f4e-518d-4c86-89871cd652af@vangyzen.net> References: <201712291905.vBTJ57gI072871@repo.freebsd.org> <20180117224054.GO8113@FreeBSD.org> <601ee1a2-8f4e-518d-4c86-89871cd652af@vangyzen.net>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180119131452.W1088>