Skip to contentSkip to conversation

Cleanup explicit check for mandatory locks

PATCH In review Pavel Emelyanov 2 participants 4 messages Sep 17, 2007 – Sep 18, 2007 NFS client

lore
Share & tools
lore
#0 lore raw

PATCH 5/5 Cleanup explicit check for mandatory locks

The __mandatory_lock(inode) macro makes the same check, but makes the code more readable.

---

 fs/nfs/file.c |    3 +--
 1 files changed, 1 insertion(+), 2 deletions(-)
fs/nfs/file.c +1 -2
index 73ddd2e..7a07be1 100644
@@ -605,8 +605,7 @@ static int nfs_lock(struct file *filp, i
 	nfs_inc_stats(inode, NFSIOS_VFSLOCK);
 
 	/* No mandatory locks over NFS */
-	if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID &&
-	    fl->fl_type != F_UNLCK)
+	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		return -ENOLCK;
 
 	if (IS_GETLK(cmd))
#1 lore raw
Quoted 2 lines · Pavel Emelyanov

The __mandatory_lock(inode) macro makes the same check, but makes the code more readable.

Could we please avoid using underscores in macros. Also, why are we breaking the usual convention of capitalising macro names?

Cheers
Trond

Quoted 23 lines

---

 fs/nfs/file.c |    3 +--
 1 files changed, 1 insertion(+), 2 deletions(-)
fs/nfs/file.c +1 -2
index 73ddd2e..7a07be1 100644
@@ -605,8 +605,7 @@ static int nfs_lock(struct file *filp, i
 	nfs_inc_stats(inode, NFSIOS_VFSLOCK);
 
 	/* No mandatory locks over NFS */
-	if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID &&
-	    fl->fl_type != F_UNLCK)
+	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		return -ENOLCK;
 
 	if (IS_GETLK(cmd))
#2 lore raw
Quoted 6 lines · Trond Myklebust
Quoted 2 lines · Pavel Emelyanov

The __mandatory_lock(inode) macro makes the same check, but makes the code more readable.

Could we please avoid using underscores in macros. Also, why are we breaking the usual convention of capitalising macro names?

Sorry, I've forgot to change all the log - this is not a macro, but a static inline function. The underscores are here, because the mandatory_lock() one already exists and additionally checks for "if (IS_MANDLOCK(inode))"

Thanks,
Pavel

Quoted 28 lines

Cheers
Trond

Quoted 23 lines

---

 fs/nfs/file.c |    3 +--
 1 files changed, 1 insertion(+), 2 deletions(-)
fs/nfs/file.c +1 -2
index 73ddd2e..7a07be1 100644
@@ -605,8 +605,7 @@ static int nfs_lock(struct file *filp, i
 	nfs_inc_stats(inode, NFSIOS_VFSLOCK);
 
 	/* No mandatory locks over NFS */
-	if ((inode->i_mode & (S_ISGID | S_IXGRP)) == S_ISGID &&
-	    fl->fl_type != F_UNLCK)
+	if (__mandatory_lock(inode) && fl->fl_type != F_UNLCK)
 		return -ENOLCK;
 
 	if (IS_GETLK(cmd))
#3 lore raw
Quoted 5 lines · Pavel Emelyanov
Quoted 4 lines · Trond Myklebust
Quoted 2 lines · Pavel Emelyanov

The __mandatory_lock(inode) macro makes the same check, but makes the code more readable.

Quoted 7 lines
Quoted 2 lines

Could we please avoid using underscores in macros. Also, why are we breaking the usual convention of capitalising macro names?

Sorry, I've forgot to change all the log - this is not a macro, but a static inline function. The underscores are here, because the mandatory_lock() one already exists and additionally checks for "if (IS_MANDLOCK(inode))"

OK. I withdraw my objection then.

Cheers
Trond