From owner-svn-src-all@freebsd.org Thu Feb 11 16:55:06 2016 Return-Path: Delivered-To: svn-src-all@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 2E71BAA4CAE; Thu, 11 Feb 2016 16:55:06 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id EEB701DE2; Thu, 11 Feb 2016 16:55:05 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from c110-21-41-193.carlnfd1.nsw.optusnet.com.au (c110-21-41-193.carlnfd1.nsw.optusnet.com.au [110.21.41.193]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id 4E224D67146; Fri, 12 Feb 2016 03:54:57 +1100 (AEDT) Date: Fri, 12 Feb 2016 03:54:55 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: "Pedro F. Giffuni" cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r295523 - head/sys/fs/ext2fs In-Reply-To: <201602111527.u1BFRFMe011283@repo.freebsd.org> Message-ID: <20160212030505.C1943@besplex.bde.org> References: <201602111527.u1BFRFMe011283@repo.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=PfoC/XVd c=1 sm=1 tr=0 a=73JWPhLeruqQCjN69UNZtQ==:117 a=L9H7d07YOLsA:10 a=9cW_t1CCXrUA:10 a=s5jvgZ67dGcA:10 a=kj9zAlcOel0A:10 a=XoNei6wmGbNNJ5Xpwj4A:9 a=CjuIK1q_8ugA:10 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 11 Feb 2016 16:55:06 -0000 On Thu, 11 Feb 2016, Pedro F. Giffuni wrote: > Log: > Ext4: Use boolean type instead of '0' and '1' > > There are precedents of uses of bool in the kernel and > it is incorrect style to use integers as replacement for > a boolean type. This is all backwards. It is correct style to use integers to represent booleans. There are precedents for breaking this style by using the bool type, but not many in fs code: In all of /sys/fs/*, there were just 12 lines of declarations that use 'bool'. All of these were in autofs. Now there are 13 such lines (1 more in ext2fs). 1 use is especially useless and especially unlikely to be all uses of booleans. /sys/fs/nfsclient uses boolean_t in 1 whole line. boolean_t is an old spelling of 'bool'. It is a Mach type for representing booleans in /sys/vm. I used to like such typedefs 20-30 years ago (mine was spelled bool_t) and made the mistake of using the Mach type elsewhere in the kernel. The use in nfsclient is actually correct. It is used to hold the result of a vm function which returns boolean_t. /sys/fs/nfsserver uses bool_t in 1 whole line. bool_t is is like boolean_t except in is Sun(?) type for reprenting booleans in rpc. It use is again correct -- it is used to hold the result of an rpc function that returns bool_t. /sys/fs/tmpfs the Mach spelling on 5 lines. These uses are to sks for small pessimizations, but inlining and -O usually gives the same object code as using integers. The values start as flags which are naturally represented as integers and are tested by expressions of the form (de->td_cookie & FLAG). These simple flags tests are obfuscated using function calls. The functions return boolean_t, so must convert to a logical expression. They do this correctly. Then the compiler should optimize away all the obfuscations and pessimizations to give the same object code as if you wrote the tests directly. ffs and zfs provided perhaps better examples. /sys/cddl uses boolean_t on 102 lines. It isn't clear what this type is. compat/opensolaris/sys/types.h seems to use the FreeBSD kernel boolean_t in the kernel, but in userland it uses 2 different enum types depending on options. These types are incompatible so this depends on magic to work. /sys/cddl also uses bool_t twice, once each for rpc and nvpair. /sys/cddl never uses C99 bool. ffs used to use a very pure KNF style with integers and no typedefs, but it now uses C99 bool on 4 lines. These uses are correct. Bruce