2006-08-31 23:46:16

by Richard Knutsson

[permalink] [raw]
Subject: [PATCH 2.6.18-rc4-mm3 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)"

Signed-off-by: Richard Knutsson <[email protected]>

---

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(-)


diff -Narup a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
--- a/fs/xfs/xfs_log.c 2006-08-31 22:03:56.000000000 +0200
+++ b/fs/xfs/xfs_log.c 2006-09-01 00:14:02.000000000 +0200
@@ -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 -
diff -Narup a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
--- a/fs/xfs/xfs_vfsops.c 2006-08-31 22:03:56.000000000 +0200
+++ b/fs/xfs/xfs_vfsops.c 2006-09-01 00:14:36.000000000 +0200
@@ -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);



2006-09-01 00:08:15

by Nathan Scott

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-rc4-mm3 2/2] fs/xfs: Correcting error-prone boolean-statement

On Fri, Sep 01, 2006 at 01:52:51AM +0200, Richard Knutsson wrote:
> 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:

> 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.

--
Nathan

2006-09-01 01:11:46

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-rc4-mm3 2/2] fs/xfs: Correcting error-prone boolean-statement

Nathan Scott wrote:

>On Fri, Sep 01, 2006 at 01:52:51AM +0200, Richard Knutsson wrote:
>
>
>>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:
>
>
>
>>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.

>cheers.
>
>
cu

2006-09-01 01:16:33

by Nathan Scott

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-rc4-mm3 2/2] fs/xfs: Correcting error-prone boolean-statement

On Fri, Sep 01, 2006 at 03:18:31AM +0200, Richard Knutsson wrote:
> Nathan Scott wrote:
> >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.

--
Nathan

2006-09-01 01:31:31

by Richard Knutsson

[permalink] [raw]
Subject: Re: [PATCH 2.6.18-rc4-mm3 2/2] fs/xfs: Correcting error-prone boolean-statement

Nathan Scott wrote:

>On Fri, Sep 01, 2006 at 03:18:31AM +0200, Richard Knutsson wrote:
>
>
>>Nathan Scott wrote:
>>
>>
>>>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.
>
Thanks!

> 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.

>cheers.
>
>
>
cu