From: Artem Bityutskiy <[email protected]>
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: [email protected]
Cc: [email protected]
Signed-off-by: Artem Bityutskiy <[email protected]>
Signed-off-by: David Howells <[email protected]>
---
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;
On Thu, 2009-07-09 at 10:44 +0100, David Howells wrote:
> From: Artem Bityutskiy <[email protected]>
>
> 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: [email protected]
> Cc: [email protected]
> Signed-off-by: Artem Bityutskiy <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> ---
>
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
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 <[email protected]>
> >
> > 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: [email protected]
> > Cc: [email protected]
> > Signed-off-by: Artem Bityutskiy <[email protected]>
> > Signed-off-by: David Howells <[email protected]>
> > ---
> >
>
> 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
Jaswinder Singh Rajput <[email protected]> wrote:
> 2. fix 80 column wrap problem
I don't see an 80-column wrap problem that you've removed.
David
On Tue, 2009-07-14 at 14:10 +0100, David Howells wrote:
> Jaswinder Singh Rajput <[email protected]> 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
Jaswinder Singh Rajput <[email protected]> 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
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;
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 <[email protected]>
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 (Артём Битюцкий)
Artem Bityutskiy <[email protected]> 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