Skip to contentSkip to conversation

fs/xfs: Correcting error-prone boolean-statement

PATCH In review Richard Knutsson 2 participants 5 messages Aug 31, 2006 – Sep 1, 2006 XFS filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 fs/xfs: Correcting error-prone boolean-statement

From: Richard Knutsson <[email protected]>

Converting error-prone statement:
"if (var == B_FALSE)" into "if (!var)"
"if (var == B_TRUE)" into "if (var)"

---

Compile-tested
Please Cc: since I'm not on [email protected]

xfs_log.c | 4 ++--
xfs_vfsops.c | 16 ++++++++--------

2 files changed, 10 insertions(+), 10 deletions(-)
fs/xfs/xfs_log.c +0 -0
diff -Narup a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
@@ -3484,7 +3484,7 @@ xlog_verify_iclog(xlog_t	 *log,
		/* clientid is only 1 byte */
		field_offset = (__psint_t)
			       ((xfs_caddr_t)&(ophead->oh_clientid) - base_ptr);
-		if (syncing == B_FALSE || (field_offset & 0x1ff)) {
+		if (!syncing || (field_offset & 0x1ff)) {
			clientid = ophead->oh_clientid;
		} else {
			idx = BTOBBT((xfs_caddr_t)&(ophead->oh_clientid) - iclog->ic_datap);
@@ -3504,7 +3504,7 @@ xlog_verify_iclog(xlog_t	 *log,
		/* check length */
		field_offset = (__psint_t)
			       ((xfs_caddr_t)&(ophead->oh_len) - base_ptr);
-		if (syncing == B_FALSE || (field_offset & 0x1ff)) {
+		if (!syncing || (field_offset & 0x1ff)) {
			op_len = INT_GET(ophead->oh_len, ARCH_CONVERT);
		} else {
			idx = BTOBBT((__psint_t)&ophead->oh_len -
fs/xfs/xfs_vfsops.c +0 -0
diff -Narup a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
@@ -935,7 +935,7 @@ xfs_sync_inodes(
 * longer be locked.
 */

#define IPOINTER_INSERT(ip, mp) { \
- ASSERT(ipointer_in == B_FALSE); \

+		ASSERT(!ipointer_in); \
		ipointer->ip_mnext = ip->i_mnext; \
		ipointer->ip_mprev = ip; \
		ip->i_mnext = (xfs_inode_t *)ipointer; \

@@ -952,7 +952,7 @@ xfs_sync_inodes(
* past us.
*/
#define IPOINTER_REMOVE(ip, mp) { \
- ASSERT(ipointer_in == B_TRUE); \

+		ASSERT(ipointer_in); \
		if (ipointer->ip_mnext != (xfs_inode_t *)ipointer) { \
			ip = ipointer->ip_mnext; \
			ip->i_mprev = ipointer->ip_mprev; \
@@ -1006,8 +1006,8 @@ xfs_sync_inodes(
	IPOINTER_CLR;

	do {

- ASSERT(ipointer_in == B_FALSE);
- ASSERT(vnode_refed == B_FALSE);
+ ASSERT(!ipointer_in);
+ ASSERT(!vnode_refed);

		lock_flags = base_lock_flags;

@@ -1372,7 +1372,7 @@ xfs_sync_inodes(

				IPOINTER_REMOVE(ip, mp);
			}
			XFS_MOUNT_IUNLOCK(mp);
-			ASSERT(ipointer_in == B_FALSE);
+			ASSERT(!ipointer_in);
			kmem_free(ipointer, sizeof(xfs_iptr_t));
			return XFS_ERROR(error);
		}
@@ -1387,21 +1387,21 @@ xfs_sync_inodes(
			}
		}

- if (mount_locked == B_FALSE) {

+		if (!mount_locked) {
			XFS_MOUNT_ILOCK(mp);
			mount_locked = B_TRUE;
			IPOINTER_REMOVE(ip, mp);
			continue;
		}

- ASSERT(ipointer_in == B_FALSE);

+		ASSERT(!ipointer_in);
		ip = ip->i_mnext;

	} while (ip != mp->m_inodes);

	XFS_MOUNT_IUNLOCK(mp);

- ASSERT(ipointer_in == B_FALSE);
+ ASSERT(!ipointer_in);

	kmem_free(ipointer, sizeof(xfs_iptr_t));
	return XFS_ERROR(last_error);
#1 lore raw
Quoted 5 lines · Richard Knutsson

From: Richard Knutsson <[email protected]>

Converting error-prone statement:
"if (var == B_FALSE)" into "if (!var)"
"if (var == B_TRUE)" into "if (var)"

This is my preference too, rather than the local boolean usage which isn't used with any consistency... but:

Quoted 1 line

Compile-tested

Are you using XFS on your systems? What is your strategy for getting this runtime tested going to be? Or are you delegating that responsibility? :)

cheers.

Signature · 1 line
Nathan
#2 lore raw
Quoted 16 lines · Nathan Scott
Quoted 7 lines · Richard Knutsson

From: Richard Knutsson <[email protected]>

Converting error-prone statement:
"if (var == B_FALSE)" into "if (!var)"
"if (var == B_TRUE)" into "if (var)"

This is my preference too, rather than the local boolean usage which isn't used with any consistency... but:

Quoted 8 lines
Quoted 3 lines

Compile-tested

Are you using XFS on your systems? What is your strategy for getting this runtime tested going to be? Or are you delegating that responsibility? :)

Sorry, can't say that I do. So pretty please... ;)
Seriously, I can not find a state when this may fail (if not "if (var == TRUE)" happend to be correct for 'var' != 0 != 1, but that is just a bug waiting to happend). But please correct me if I am wrong.

Quoted 3 lines

cheers.

cu

#3 lore raw
Quoted 9 lines · Richard Knutsson
Quoted 3 lines · Nathan Scott

Are you using XFS on your systems? What is your strategy for getting this runtime tested going to be? Or are you delegating that responsibility? :)

Sorry, can't say that I do. So pretty please... ;) Seriously, I can not find a state when this may fail (if not "if (var == TRUE)" happend to be correct for 'var' != 0 != 1, but that is just a bug waiting to happend).
But please correct me if I am wrong.

OK, I'll run with it in my own testing for awhile. I was also curious to why you didn't remove the other few B_TRUE/B_FALSE occurences? (and the typedef)?

cheers.

Signature · 1 line
Nathan
#4 lore raw
Quoted 11 lines · Nathan Scott
Quoted 8 lines · Richard Knutsson
Quoted 5 lines · Nathan Scott

Are you using XFS on your systems? What is your strategy for getting this runtime tested going to be? Or are you delegating that responsibility? :)

Quoted 8 lines
Quoted 5 lines

Sorry, can't say that I do. So pretty please... ;)
Seriously, I can not find a state when this may fail (if not "if (var == TRUE)" happend to be correct for 'var' != 0 != 1, but that is just a bug waiting to happend). But please correct me if I am wrong.

OK, I'll run with it in my own testing for awhile.

Thanks!

Quoted 5 lines

I was also curious to
why you didn't remove the other few B_TRUE/B_FALSE occurences? (and the typedef)?

Working on it. Should be out tomorrow(or in about 20 hours).
From the "Re: Conversion to generic boolean"-thread (started on 06-08-28), there were those who did not seem to like the conversion. But since no-one complained about removing "== B_FALSE/B_TRUE", I thought it best to remove them first and then take the rest from there.

Quoted 4 lines

cheers.

cu