Skip to contentSkip to conversation

Silence compiler warning

PATCH In review Jim Rees 2 participants 5 messages Nov 1, 2011

Subsystems (2)
lore
Share & tools
lore

Jim Rees

author
#0 lore raw

PATCH 1/1 Silence compiler warning

I think this is still needed, isn't it? I haven't tried compiling nfs-for-next but I don't see a fix for it in there.

From: Jim Rees <[email protected]>
Date: Sun, 16 Oct 2011 20:06:52 -0400
Subject: [PATCH] Silence compiler warning

fs/nfs/callback_proc.c: In function ‘do_callback_layoutrecall’:
fs/nfs/callback_proc.c:115:26: warning: ‘lo’ may be used uninitialized in this function

No functional change. If no layout is found, we'll return before using "lo".

---
 fs/nfs/callback_proc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
fs/nfs/callback_proc.c +2 -2
index 43926ad..f384eb1 100644
@@ -112,8 +112,8 @@ static u32 initiate_file_draining(struct nfs_client *clp,
 				  struct cb_layoutrecallargs *args)
 {
 	struct nfs_server *server;
-	struct pnfs_layout_hdr *lo;
-	struct inode *ino;
+	struct pnfs_layout_hdr *lo = NULL;
+	struct inode *ino = NULL;
 	bool found = false;
 	u32 rv = NFS4ERR_NOMATCHING_LAYOUT;
 	LIST_HEAD(free_me_list);
Signature · 1 line
1.7.4.1
#1 lore raw
Quoted 2 lines · Jim Rees

I think this is still needed, isn't it? I haven't tried compiling nfs-for-next but I don't see a fix for it in there.

AFAICS, this is a bogus warning: if lo is uninitialised, we will automatically exit with a return value of NFS4ERR_NOMATCHING_LAYOUT.

Signature · 6 lines
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com
author
#2 lore raw

Trond Myklebust wrote:

Quoted 2 lines · Jim Rees

I think this is still needed, isn't it? I haven't tried compiling nfs-for-next but I don't see a fix for it in there.

AFAICS, this is a bogus warning: if lo is uninitialised, we will automatically exit with a return value of NFS4ERR_NOMATCHING_LAYOUT.

Yes, the warning is bogus. I just want the damn compiler to shut up so I can see if there are any real warnings.

#3 lore raw
Quoted 11 lines · Jim Rees

Trond Myklebust wrote:

Quoted 2 lines · Jim Rees

I think this is still needed, isn't it? I haven't tried compiling nfs-for-next but I don't see a fix for it in there.

AFAICS, this is a bogus warning: if lo is uninitialised, we will automatically exit with a return value of NFS4ERR_NOMATCHING_LAYOUT.

Yes, the warning is bogus. I just want the damn compiler to shut up so I can see if there are any real warnings.

How about cleaning up the whole unnecessary "bool found" crap that has it confused? You can easily replace that with a test for 'ino != NULL'. We don't usually "fix" compiler bugs by changing valid kernel code, but cleanups are acceptable if they help to clarify what is going on.

Oh, another thing: that code will currently race very nicely against 'umount', and looks capable of triggering the 'VFS: Busy inodes after unmount of foo. Self-destruct in 5 seconds. Have a nice day..." since it doesn't do anything to pin the super block while holding a reference to the inode.

Signature · 6 lines
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
www.netapp.com
author
#4 lore raw

Trond Myklebust wrote:

Quoted 11 lines · Jim Rees

Trond Myklebust wrote:

Quoted 2 lines · Jim Rees

I think this is still needed, isn't it? I haven't tried compiling nfs-for-next but I don't see a fix for it in there.

AFAICS, this is a bogus warning: if lo is uninitialised, we will automatically exit with a return value of NFS4ERR_NOMATCHING_LAYOUT.

Yes, the warning is bogus. I just want the damn compiler to shut up so I can see if there are any real warnings.

How about cleaning up the whole unnecessary "bool found" crap that has it confused? You can easily replace that with a test for 'ino != NULL'. We don't usually "fix" compiler bugs by changing valid kernel code, but cleanups are acceptable if they help to clarify what is going on.

Oh, another thing: that code will currently race very nicely against 'umount', and looks capable of triggering the 'VFS: Busy inodes after unmount of foo. Self-destruct in 5 seconds. Have a nice day..." since it doesn't do anything to pin the super block while holding a reference to the inode.

There's a patch in Benny's tree that I think addresses these issues in addition to the compiler warnings. Maybe Benny would like to move it forward?