- [7/15] - remove defconfig ptr comparisons to 0 - fs/lockd

PATCH In review Joe Perches 3 participants 4 messages Nov 14, 2007

Subsystems (2)

NFS client NFS server

lore

Share & tools

lore

Joe Perches

author

2007-11-14 02:05 UTC #0 lore raw

PATCH 1/1 - [7/15] - remove defconfig ptr comparisons to 0 - fs/lockd

Remove defconfig ptr comparison to 0

Remove sparse warning: Using plain integer as NULL pointer

Signed-off-by Joe Perches <[email protected]>

---

fs/lockd/svcshare.c +1 -1

index 068886d..98548ad 100644
@@ -71,7 +71,7 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	struct nlm_share	*share, **shpp;
 	struct xdr_netobj	*oh = &argp->lock.oh;
 
-	for (shpp = &file->f_shares; (share = *shpp) != 0; shpp = &share->s_next) {
+	for (shpp = &file->f_shares; (share = *shpp); shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
 			*shpp = share->s_next;
 			kfree(share);

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
NFS maillist - [email protected]
https://lists.sourceforge.net/lists/listinfo/nfs

Neil Brown

2007-11-14 02:40 UTC #1 lore raw

Quoted 14 lines · On Tuesday November 13, [email protected] wrote:

Remove defconfig ptr comparison to 0
Remove sparse warning: Using plain integer as NULL pointer
Signed-off-by Joe Perches <[email protected]>
---
fs/lockd/svcshare.c +0 -0
index 068886d..98548ad 100644

fs/lockd/svcshare.c nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file, · line 77

Quoted 8 lines

fs/lockd/svcshare.c +1 -1

@@ -71,7 +71,7 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	struct nlm_share	*share, **shpp;
 	struct xdr_netobj	*oh = &argp->lock.oh;
 
-	for (shpp = &file->f_shares; (share = *shpp) != 0; shpp = &share->s_next) {
+	for (shpp = &file->f_shares; (share = *shpp); shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
 			*shpp = share->s_next;
 			kfree(share);

I particularly disagree with this change as it now looked like it could be an '==' comparison that was mistyped. Making it

        ....; (share = *shpp) != NULL; ....

makes the intent clear.

NeilBrown

Trond Myklebust

2007-11-14 04:44 UTC #2 lore raw

Quoted 26 lines · Neil Brown

Quoted 14 lines · On Tuesday November 13, [email protected] wrote:
Remove defconfig ptr comparison to 0
Remove sparse warning: Using plain integer as NULL pointer
Signed-off-by Joe Perches <[email protected]>
---
fs/lockd/svcshare.c +0 -0
index 068886d..98548ad 100644
fs/lockd/svcshare.c nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file, · line 77
Quoted 8 lines
fs/lockd/svcshare.c +1 -1
@@ -71,7 +71,7 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	struct nlm_share	*share, **shpp;
 	struct xdr_netobj	*oh = &argp->lock.oh;
 
-	for (shpp = &file->f_shares; (share = *shpp) != 0; shpp = &share->s_next) {
+	for (shpp = &file->f_shares; (share = *shpp); shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
 			*shpp = share->s_next;
 			kfree(share);
I particularly disagree with this change as it now looked like it could be an '==' comparison that was mistyped. Making it

Quoted 1 line

        ....; (share = *shpp) != NULL; ....

There would also be the minor fact that the original test is being inverted in this 'fix'. An accurate fix should at the very least be
!(share = *shpp).

Quoted 1 line

makes the intent clear.

It would be a lot cleaner just to pull the entire assignment out of the for() statement. IOW:

	for (shpp = &file->f_shares; *shpp != NULL; shpp = &(*shpp)->s_next) {
		struct nlm_share *share = *shpp;

Trond Myklebust

2007-11-14 17:50 UTC #3 lore raw

Quoted 29 lines · Trond Myklebust

Quoted 26 lines · Neil Brown
Quoted 14 lines · On Tuesday November 13, [email protected] wrote:
Remove defconfig ptr comparison to 0
Remove sparse warning: Using plain integer as NULL pointer
Signed-off-by Joe Perches <[email protected]>
---
fs/lockd/svcshare.c +0 -0
index 068886d..98548ad 100644
fs/lockd/svcshare.c nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file, · line 77
Quoted 8 lines
fs/lockd/svcshare.c +1 -1
@@ -71,7 +71,7 @@ nlmsvc_unshare_file(struct nlm_host *host, struct nlm_file *file,
 	struct nlm_share	*share, **shpp;
 	struct xdr_netobj	*oh = &argp->lock.oh;
 
-	for (shpp = &file->f_shares; (share = *shpp) != 0; shpp = &share->s_next) {
+	for (shpp = &file->f_shares; (share = *shpp); shpp = &share->s_next) {
 		if (share->s_host == host && nlm_cmp_owner(share, oh)) {
 			*shpp = share->s_next;
 			kfree(share);
I particularly disagree with this change as it now looked like it could be an '==' comparison that was mistyped. Making it
Quoted 1 line
        ....; (share = *shpp) != NULL; ....

Quoted 3 lines

There would also be the minor fact that the original test is being inverted in this 'fix'. An accurate fix should at the very least be
!(share = *shpp).

Apologies to Joe. I must have been tired when I typed the above. However I'm still NACKing the patch: removing the '!= 0' altogether reduces code legibility rather than improving it, particularly when we have that ugly assignment.
If the intent is just to silence 'sparse', then replace with '!= NULL'.

Quoted 7 lines

Quoted 1 line
makes the intent clear.
It would be a lot cleaner just to pull the entire assignment out of the for() statement. IOW:
	for (shpp = &file->f_shares; *shpp != NULL; shpp = &(*shpp)->s_next) {
		struct nlm_share *share = *shpp;

...however doing something like the above would be altogether preferable, since that cleans up the assignment which is the source of the ugliness.

Cheers
Trond