linux-kernel - Re: [PATCH 4/5] mm/lru: is_file/active

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Wed, 23 Mar 2016 11:44:15 +0000
From:	Mel Gorman <mgorman@...hsingularity.net>
To:	Joe Perches <joe@...ches.com>
Cc:	Yaowei Bai <baiyaowei@...s.chinamobile.com>,
	akpm@...ux-foundation.org, n-horiguchi@...jp.nec.com,
	kirill.shutemov@...ux.intel.com, rientjes@...gle.com,
	iamjoonsoo.kim@....com, hannes@...xchg.org, vdavydov@...tuozzo.com,
	kuleshovmail@...il.com, vbabka@...e.cz, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 4/5] mm/lru: is_file/active_lru can be boolean

On Tue, Mar 22, 2016 at 08:17:08PM -0700, Joe Perches wrote:
> On Wed, 2016-03-23 at 10:26 +0800, Yaowei Bai wrote:
> > This patch makes is_file/active_lru return bool to improve
> > readability due to these particular functions only using either
> > one or zero as their return value.
> > 
> > No functional change.
> 
> These assignments to int should likely be modified too
> 

Which would lead to oddities as the ints are used as offsets within
enums. Patch 2 has a problem where a bool is then used as part of a
bitmask operation.

I stopped looking fairly early on. Conversions from int to bool as part
of a cleanup-only series are almost never useful and sometimes introduce
subtle breakage. It is only worth the conversion when the helper is being
modified for some other purpose.

On that grounds, NAK to the whole series as small problems were quickly
obvious and it's not worth the brainpower to find all the problems when
the end result does not give us a tangible improvement.

-- 
Mel Gorman
SUSE Labs