[PATCH] AFS: Fix compilation warning

[PATCH] AFS: Fix compilation warning

[David Howells]

From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

Fix the following warning:

fs/afs/dir.c: In function 'afs_d_revalidate':
fs/afs/dir.c:567: warning: 'fid.vnode' may be used uninitialized in this function
fs/afs/dir.c:567: warning: 'fid.unique' may be used uninitialized in this function

by marking the 'fid' variable as an uninitialized_var.  The problem is that gcc
doesn't always manage to work out that fid is always set on the path through
the function that uses it.

Cc: linux-afs@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---

 fs/afs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)


diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 5272872..790ba9d 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -566,7 +566,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
 static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	struct afs_vnode *vnode, *dir;
-	struct afs_fid fid;
+	struct afs_fid uninitialized_var(fid);
 	struct dentry *parent;
 	struct key *key;
 	void *dir_version;

Re: [PATCH] AFS: Fix compilation warning

[Jaswinder Singh Rajput]

On Thu, 2009-07-09 at 10:44 +0100, David Howells wrote:
> From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> 
> Fix the following warning:
> 
> fs/afs/dir.c: In function 'afs_d_revalidate':
> fs/afs/dir.c:567: warning: 'fid.vnode' may be used uninitialized in this function
> fs/afs/dir.c:567: warning: 'fid.unique' may be used uninitialized in this function
> 
> by marking the 'fid' variable as an uninitialized_var.  The problem is that gcc
> doesn't always manage to work out that fid is always set on the path through
> the function that uses it.
> 
> Cc: linux-afs@lists.infradead.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 

Have you tried this approach :

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 9bd7577..09cb5bb 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -607,53 +607,56 @@ static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 
 	/* search the directory for this vnode */
 	ret = afs_do_lookup(&dir->vfs_inode, dentry, &fid, key);
-	switch (ret) {
-	case 0:
-		/* the filename maps to something */
-		if (!dentry->d_inode)
-			goto out_bad;
-		if (is_bad_inode(dentry->d_inode)) {
-			printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
-			       parent->d_name.name, dentry->d_name.name);
+	if (ret < 0) {
+		switch (ret) {
+		case -ENOENT:
+			/* the filename is unknown */
+			_debug("%s: dirent not found", dentry->d_name.name);
+			if (dentry->d_inode)
+				goto not_found;
+			goto out_valid;
+
+		default:
+			_debug("failed to iterate dir %s: %d",
+			       parent->d_name.name, ret);
 			goto out_bad;
 		}
+	}
 
-		/* if the vnode ID has changed, then the dirent points to a
-		 * different file */
-		if (fid.vnode != vnode->fid.vnode) {
-			_debug("%s: dirent changed [%u != %u]",
-			       dentry->d_name.name, fid.vnode,
-			       vnode->fid.vnode);
-			goto not_found;
-		}
-
-		/* if the vnode ID uniqifier has changed, then the file has
-		 * been deleted and replaced, and the original vnode ID has
-		 * been reused */
-		if (fid.unique != vnode->fid.unique) {
-			_debug("%s: file deleted (uq %u -> %u I:%llu)",
-			       dentry->d_name.name, fid.unique,
-			       vnode->fid.unique,
-			       (unsigned long long)dentry->d_inode->i_version);
-			spin_lock(&vnode->lock);
-			set_bit(AFS_VNODE_DELETED, &vnode->flags);
-			spin_unlock(&vnode->lock);
-			goto not_found;
-		}
-		goto out_valid;
-
-	case -ENOENT:
-		/* the filename is unknown */
-		_debug("%s: dirent not found", dentry->d_name.name);
-		if (dentry->d_inode)
-			goto not_found;
-		goto out_valid;
-
-	default:
-		_debug("failed to iterate dir %s: %d",
-		       parent->d_name.name, ret);
+	/* the filename maps to something */
+	if (!dentry->d_inode)
 		goto out_bad;
+	if (is_bad_inode(dentry->d_inode)) {
+		printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
+		       parent->d_name.name, dentry->d_name.name);
+		goto out_bad;
+	}
+
+	/*
+	 * if the vnode ID has changed, then the dirent points to a
+	 * different file
+	 */
+	if (fid.vnode != vnode->fid.vnode) {
+		_debug("%s: dirent changed [%u != %u]",
+		       dentry->d_name.name, fid.vnode, vnode->fid.vnode);
+		goto not_found;
+	}
+
+	/*
+	 * if the vnode ID uniqifier has changed, then the file has been
+	 * deleted and replaced, and the original vnode ID has been reused
+	 */
+	if (fid.unique != vnode->fid.unique) {
+		_debug("%s: file deleted (uq %u -> %u I:%llu)",
+		       dentry->d_name.name, fid.unique,
+		       vnode->fid.unique,
+		       (unsigned long long)dentry->d_inode->i_version);
+		spin_lock(&vnode->lock);
+		set_bit(AFS_VNODE_DELETED, &vnode->flags);
+		spin_unlock(&vnode->lock);
+		goto not_found;
 	}
+	goto out_valid;
 
 out_valid:
 	dentry->d_fsdata = dir_version;

>  fs/afs/dir.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> 
> diff --git a/fs/afs/dir.c b/fs/afs/dir.c
> index 5272872..790ba9d 100644
> --- a/fs/afs/dir.c
> +++ b/fs/afs/dir.c
> @@ -566,7 +566,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
>  static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
>  {
>  	struct afs_vnode *vnode, *dir;
> -	struct afs_fid fid;
> +	struct afs_fid uninitialized_var(fid);
>  	struct dentry *parent;
>  	struct key *key;
>  	void *dir_version;
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] AFS: Fix compilation warning

[Jaswinder Singh Rajput]

On Tue, 2009-07-14 at 14:17 +0530, Jaswinder Singh Rajput wrote:
> On Thu, 2009-07-09 at 10:44 +0100, David Howells wrote:
> > From: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > 
> > Fix the following warning:
> > 
> > fs/afs/dir.c: In function 'afs_d_revalidate':
> > fs/afs/dir.c:567: warning: 'fid.vnode' may be used uninitialized in this function
> > fs/afs/dir.c:567: warning: 'fid.unique' may be used uninitialized in this function
> > 
> > by marking the 'fid' variable as an uninitialized_var.  The problem is that gcc
> > doesn't always manage to work out that fid is always set on the path through
> > the function that uses it.
> > 
> > Cc: linux-afs@lists.infradead.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
> > Signed-off-by: David Howells <dhowells@redhat.com>
> > ---
> > 
> 
> Have you tried this approach :
> 
> diff --git a/fs/afs/dir.c b/fs/afs/dir.c
> index 9bd7577..09cb5bb 100644
> --- a/fs/afs/dir.c
> +++ b/fs/afs/dir.c
> @@ -607,53 +607,56 @@ static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
>  
>  	/* search the directory for this vnode */
>  	ret = afs_do_lookup(&dir->vfs_inode, dentry, &fid, key);
> -	switch (ret) {
> -	case 0:
> -		/* the filename maps to something */
> -		if (!dentry->d_inode)
> -			goto out_bad;
> -		if (is_bad_inode(dentry->d_inode)) {
> -			printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
> -			       parent->d_name.name, dentry->d_name.name);
> +	if (ret < 0) {
> +		switch (ret) {
> +		case -ENOENT:
> +			/* the filename is unknown */
> +			_debug("%s: dirent not found", dentry->d_name.name);
> +			if (dentry->d_inode)
> +				goto not_found;
> +			goto out_valid;
> +
> +		default:
> +			_debug("failed to iterate dir %s: %d",
> +			       parent->d_name.name, ret);
>  			goto out_bad;
>  		}
> +	}
>  
> -		/* if the vnode ID has changed, then the dirent points to a
> -		 * different file */
> -		if (fid.vnode != vnode->fid.vnode) {
> -			_debug("%s: dirent changed [%u != %u]",
> -			       dentry->d_name.name, fid.vnode,
> -			       vnode->fid.vnode);
> -			goto not_found;
> -		}
> -
> -		/* if the vnode ID uniqifier has changed, then the file has
> -		 * been deleted and replaced, and the original vnode ID has
> -		 * been reused */
> -		if (fid.unique != vnode->fid.unique) {
> -			_debug("%s: file deleted (uq %u -> %u I:%llu)",
> -			       dentry->d_name.name, fid.unique,
> -			       vnode->fid.unique,
> -			       (unsigned long long)dentry->d_inode->i_version);
> -			spin_lock(&vnode->lock);
> -			set_bit(AFS_VNODE_DELETED, &vnode->flags);
> -			spin_unlock(&vnode->lock);
> -			goto not_found;
> -		}
> -		goto out_valid;
> -
> -	case -ENOENT:
> -		/* the filename is unknown */
> -		_debug("%s: dirent not found", dentry->d_name.name);
> -		if (dentry->d_inode)
> -			goto not_found;
> -		goto out_valid;
> -
> -	default:
> -		_debug("failed to iterate dir %s: %d",
> -		       parent->d_name.name, ret);
> +	/* the filename maps to something */
> +	if (!dentry->d_inode)
>  		goto out_bad;
> +	if (is_bad_inode(dentry->d_inode)) {
> +		printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
> +		       parent->d_name.name, dentry->d_name.name);
> +		goto out_bad;
> +	}
> +
> +	/*
> +	 * if the vnode ID has changed, then the dirent points to a
> +	 * different file
> +	 */
> +	if (fid.vnode != vnode->fid.vnode) {
> +		_debug("%s: dirent changed [%u != %u]",
> +		       dentry->d_name.name, fid.vnode, vnode->fid.vnode);
> +		goto not_found;
> +	}
> +
> +	/*
> +	 * if the vnode ID uniqifier has changed, then the file has been
> +	 * deleted and replaced, and the original vnode ID has been reused
> +	 */
> +	if (fid.unique != vnode->fid.unique) {
> +		_debug("%s: file deleted (uq %u -> %u I:%llu)",
> +		       dentry->d_name.name, fid.unique,
> +		       vnode->fid.unique,
> +		       (unsigned long long)dentry->d_inode->i_version);
> +		spin_lock(&vnode->lock);
> +		set_bit(AFS_VNODE_DELETED, &vnode->flags);
> +		spin_unlock(&vnode->lock);
> +		goto not_found;
>  	}
> +	goto out_valid;

You can also remove this goto. So by this way, you can get rid of :

1. compiler warning
2. fix 80 column wrap problem
3. remove one goto

Enjoy.
--
JSR

Re: [PATCH] AFS: Fix compilation warning

[David Howells]

Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> 2. fix 80 column wrap problem

I don't see an 80-column wrap problem that you've removed.

David

Re: [PATCH] AFS: Fix compilation warning

[Jaswinder Singh Rajput]

On Tue, 2009-07-14 at 14:10 +0100, David Howells wrote:
> Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:
> 
> > 2. fix 80 column wrap problem
> 
> I don't see an 80-column wrap problem that you've removed.
> 

ok here it is :

-               if (fid.vnode != vnode->fid.vnode) {
-                       _debug("%s: dirent changed [%u != %u]",
-                              dentry->d_name.name, fid.vnode,
-                              vnode->fid.vnode);
-                       goto not_found;
-               }

+       if (fid.vnode != vnode->fid.vnode) {
+               _debug("%s: dirent changed [%u != %u]",
+                      dentry->d_name.name, fid.vnode, vnode->fid.vnode);
+               goto not_found;
+       }


-               /* if the vnode ID uniqifier has changed, then the file has
-                * been deleted and replaced, and the original vnode ID has
-                * been reused */

+       /*
+        * if the vnode ID uniqifier has changed, then the file has been
+        * deleted and replaced, and the original vnode ID has been reused
+        */

--
JSR

Re: [PATCH] AFS: Fix compilation warning

[David Howells]

Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> > I don't see an 80-column wrap problem that you've removed.
> > 
> 
> ok here it is :
> 
> -               if (fid.vnode != vnode->fid.vnode) {
> -                       _debug("%s: dirent changed [%u != %u]",
> -                              dentry->d_name.name, fid.vnode,
> -                              vnode->fid.vnode);

Ah, I see.  I thought you meant there was a line of >80 chars that got fixed.

David

Re: [PATCH] AFS: Fix compilation warning

[David Howells]

Hi Artem,

Can you try Jaswinder's patch?  I don't see the warning even without your
patch, so I can't test it.

David
---
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 9bd7577..09cb5bb 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -607,53 +607,56 @@ static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 
 	/* search the directory for this vnode */
 	ret = afs_do_lookup(&dir->vfs_inode, dentry, &fid, key);
-	switch (ret) {
-	case 0:
-		/* the filename maps to something */
-		if (!dentry->d_inode)
-			goto out_bad;
-		if (is_bad_inode(dentry->d_inode)) {
-			printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
-			       parent->d_name.name, dentry->d_name.name);
+	if (ret < 0) {
+		switch (ret) {
+		case -ENOENT:
+			/* the filename is unknown */
+			_debug("%s: dirent not found", dentry->d_name.name);
+			if (dentry->d_inode)
+				goto not_found;
+			goto out_valid;
+
+		default:
+			_debug("failed to iterate dir %s: %d",
+			       parent->d_name.name, ret);
 			goto out_bad;
 		}
+	}
 
-		/* if the vnode ID has changed, then the dirent points to a
-		 * different file */
-		if (fid.vnode != vnode->fid.vnode) {
-			_debug("%s: dirent changed [%u != %u]",
-			       dentry->d_name.name, fid.vnode,
-			       vnode->fid.vnode);
-			goto not_found;
-		}
-
-		/* if the vnode ID uniqifier has changed, then the file has
-		 * been deleted and replaced, and the original vnode ID has
-		 * been reused */
-		if (fid.unique != vnode->fid.unique) {
-			_debug("%s: file deleted (uq %u -> %u I:%llu)",
-			       dentry->d_name.name, fid.unique,
-			       vnode->fid.unique,
-			       (unsigned long long)dentry->d_inode->i_version);
-			spin_lock(&vnode->lock);
-			set_bit(AFS_VNODE_DELETED, &vnode->flags);
-			spin_unlock(&vnode->lock);
-			goto not_found;
-		}
-		goto out_valid;
-
-	case -ENOENT:
-		/* the filename is unknown */
-		_debug("%s: dirent not found", dentry->d_name.name);
-		if (dentry->d_inode)
-			goto not_found;
-		goto out_valid;
-
-	default:
-		_debug("failed to iterate dir %s: %d",
-		       parent->d_name.name, ret);
+	/* the filename maps to something */
+	if (!dentry->d_inode)
 		goto out_bad;
+	if (is_bad_inode(dentry->d_inode)) {
+		printk("kAFS: afs_d_revalidate: %s/%s has bad inode\n",
+		       parent->d_name.name, dentry->d_name.name);
+		goto out_bad;
+	}
+
+	/*
+	 * if the vnode ID has changed, then the dirent points to a
+	 * different file
+	 */
+	if (fid.vnode != vnode->fid.vnode) {
+		_debug("%s: dirent changed [%u != %u]",
+		       dentry->d_name.name, fid.vnode, vnode->fid.vnode);
+		goto not_found;
+	}
+
+	/*
+	 * if the vnode ID uniqifier has changed, then the file has been
+	 * deleted and replaced, and the original vnode ID has been reused
+	 */
+	if (fid.unique != vnode->fid.unique) {
+		_debug("%s: file deleted (uq %u -> %u I:%llu)",
+		       dentry->d_name.name, fid.unique,
+		       vnode->fid.unique,
+		       (unsigned long long)dentry->d_inode->i_version);
+		spin_lock(&vnode->lock);
+		set_bit(AFS_VNODE_DELETED, &vnode->flags);
+		spin_unlock(&vnode->lock);
+		goto not_found;
 	}
+	goto out_valid;
 
 out_valid:
 	dentry->d_fsdata = dir_version;

Re: [PATCH] AFS: Fix compilation warning

[Artem Bityutskiy]

Hi,

David Howells wrote:
> Can you try Jaswinder's patch?  I don't see the warning even without your
> patch, so I can't test it.

Linus has already applied my patch which you sent him:

commit dd0d9a46f573b086a67522f819566427dba9c4c7
Author: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>
Date:   Thu Jul 9 10:44:30 2009 +0100

    AFS: Fix compilation warning

But this patch applies to both current linux-2.6.git and to
pre-"AFS: Fix compilation warning" tree.

I've compile-tested the latter and yes, this patch fixes
the warning.

FWIW, if you want to apply this patch, then you probably
should first revert my patch, or make this patch undo my
"unitinialized_var" changes. And probably this stuff is
anyway not 2.6.31 material, because the warning is fixed
anyway :-) Anyway, not my area :-)

-- 
Best Regards,
Artem Bityutskiy (Артём Битюцкий)

Re: [PATCH] AFS: Fix compilation warning

[David Howells]

Artem Bityutskiy <Artem.Bityutskiy@nokia.com> wrote:

> FWIW, if you want to apply this patch, then you probably
> should first revert my patch, or make this patch undo my
> "unitinialized_var" changes. And probably this stuff is
> anyway not 2.6.31 material, because the warning is fixed
> anyway :-) Anyway, not my area :-)

Thanks for testing it.  I'll keep it on ice till 2.6.31.

David

[PATCH] AFS: fix compilation warning

[Artem Bityutskiy]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 915 bytes --]

Fix the following warning:

fs/afs/dir.c: In function ‘afs_d_revalidate’:
fs/afs/dir.c:567: warning: ‘fid.vnode’ may be used uninitialized in this function
fs/afs/dir.c:567: warning: ‘fid.unique’ may be used uninitialized in this function

Cc: linux-afs@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Artem Bityutskiy <Artem.Bityutskiy@nokia.com>

---
 fs/afs/dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index 9bd7577..88067f3 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -564,7 +564,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct dentry *dentry,
 static int afs_d_revalidate(struct dentry *dentry, struct nameidata *nd)
 {
 	struct afs_vnode *vnode, *dir;
-	struct afs_fid fid;
+	struct afs_fid uninitialized_var(fid);
 	struct dentry *parent;
 	struct key *key;
 	void *dir_version;
-- 
1.6.0.6