linux-kernel - Re: [RFC PATCH 1/1] cleanup: use bool as return type for rwsem_is

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <53926DC1.4050304@gmail.com>
Date:	Fri, 06 Jun 2014 21:41:21 -0400
From:	Pranith Kumar <bobby.prani@...il.com>
To:	Dave Chinner <david@...morbit.com>,
	Pranith Kumar <pranith@...ech.edu>
CC:	peterz@...radead.org, linux-kernel@...r.kernel.org,
	tim.c.chen@...ux.intel.com, davidlohr@...com, mingo@...hat.com,
	xfs@....sgi.com
Subject: Re: [RFC PATCH 1/1] cleanup: use bool as return type for rwsem_is_locked

On 06/06/2014 08:59 PM, Pranith Kumar wrote:
> On 06/06/2014 08:18 PM, Dave Chinner wrote:
>> On Fri, Jun 06, 2014 at 02:11:18PM -0400, Pranith Kumar wrote:
>>> On 06/06/2014 01:53 PM, Pranith Kumar wrote:
>>>> On Fri, Jun 6, 2014 at 3:35 AM, Peter Zijlstra <peterz@...radead.org> wrote:
>>>>>
>>>>> Now in general, I don't particularly like such superfluous changes, so
>>>>> unless you can show that GCC actually generates better code, I'd prefer
>>>>> to keep things as they are.
>>>>
>>>> Fixed and checked the assembly. It saves us 2 bytes of code, not much. I am not sure if that is worth it :(
>>>>
>>>> use bool as the return type for rwsem_is_locked() instead of int
>>>>
> 
>>
>> If you are going to change the return type to bool, then you should
>> also remove the manual "!!" conversions to a boolean return and let
>> the compiler do it in the most optimal way.
>>
>  
> Agreed, please find patch below:
> 

Simplify the "!!" condition. This is much simpler. :)

change return type to bool to follow rwsem_is_locked()

Signed-off-by: Pranith Kumar <bobby.prani@...il.com>
---
 fs/xfs/xfs_inode.c | 8 ++++----
 fs/xfs/xfs_inode.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index a6115fe..c02ac49 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -285,25 +285,25 @@ xfs_ilock_demote(
 }
 
 #if defined(DEBUG) || defined(XFS_WARN)
-int
+bool
 xfs_isilocked(
 	xfs_inode_t		*ip,
 	uint			lock_flags)
 {
 	if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) {
 		if (!(lock_flags & XFS_ILOCK_SHARED))
-			return !!ip->i_lock.mr_writer;
+			return (ip->i_lock.mr_writer != 0);
 		return rwsem_is_locked(&ip->i_lock.mr_lock);
 	}
 
 	if (lock_flags & (XFS_IOLOCK_EXCL|XFS_IOLOCK_SHARED)) {
 		if (!(lock_flags & XFS_IOLOCK_SHARED))
-			return !!ip->i_iolock.mr_writer;
+			return (ip->i_iolock.mr_writer != 0);
 		return rwsem_is_locked(&ip->i_iolock.mr_lock);
 	}
 
 	ASSERT(0);
-	return 0;
+	return false;
 }
 #endif
 
diff --git a/fs/xfs/xfs_inode.h b/fs/xfs/xfs_inode.h
index f72bffa..efebed6 100644
--- a/fs/xfs/xfs_inode.h
+++ b/fs/xfs/xfs_inode.h
@@ -346,7 +346,7 @@ void		xfs_ilock(xfs_inode_t *, uint);
 int		xfs_ilock_nowait(xfs_inode_t *, uint);
 void		xfs_iunlock(xfs_inode_t *, uint);
 void		xfs_ilock_demote(xfs_inode_t *, uint);
-int		xfs_isilocked(xfs_inode_t *, uint);
+bool		xfs_isilocked(xfs_inode_t *, uint);
 uint		xfs_ilock_data_map_shared(struct xfs_inode *);
 uint		xfs_ilock_attr_map_shared(struct xfs_inode *);
 int		xfs_ialloc(struct xfs_trans *, xfs_inode_t *, umode_t,
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/