[PATCH] afs: remove redundant assignment of dvnode to itself

[PATCH] afs: remove redundant assignment of dvnode to itself

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The assignment of dvnode to itself is redundant and can be removed.
Cleans up warning detected by cppcheck:

fs/afs/dir.c:975: (warning) Redundant assignment of 'dvnode' to itself.

Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/afs/dir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index ab618d32554c..a5e121e55e57 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -972,7 +972,7 @@ static int afs_create(struct inode *dir, struct dentry *dentry, umode_t mode,
 	struct afs_fs_cursor fc;
 	struct afs_file_status newstatus;
 	struct afs_callback newcb;
-	struct afs_vnode *dvnode = dvnode = AFS_FS_I(dir);
+	struct afs_vnode *dvnode = AFS_FS_I(dir);
 	struct afs_fid newfid;
 	struct key *key;
 	int ret;
-- 
2.14.1

Re: [PATCH] afs: remove redundant assignment of dvnode to itself

[David Howells]

Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> The assignment of dvnode to itself is redundant and can be removed.
> Cleans up warning detected by cppcheck:
> 
> fs/afs/dir.c:975: (warning) Redundant assignment of 'dvnode' to itself.
> 
> Fixes: d2ddc776a458 ("afs: Overhaul volume and server record caching and fileserver rotation")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied, thanks.

David

[PATCH] afs: remove redundant assignment to variable ret

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The variable ret is being assigned a value however this is never
read and later it is being reassigned to a new value. The assignment
is redundant and hence can be removed.

Addresses-Coverity: ("Unused Value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/afs/xattr.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/afs/xattr.c b/fs/afs/xattr.c
index c81f85003fc7..25cb7ad4fca3 100644
--- a/fs/afs/xattr.c
+++ b/fs/afs/xattr.c
@@ -71,7 +71,6 @@ static int afs_xattr_get_acl(const struct xattr_handler *handler,
 	if (ret = 0) {
 		ret = acl->size;
 		if (size > 0) {
-			ret = -ERANGE;
 			if (acl->size > size)
 				return -ERANGE;
 			memcpy(buffer, acl->data, acl->size);
-- 
2.20.1

Re: [PATCH] afs: remove redundant assignment to variable ret

[Joe Perches]

On Sat, 2019-05-11 at 13:36 +0100, Colin King wrote:
> The variable ret is being assigned a value however this is never
> read and later it is being reassigned to a new value. The assignment
> is redundant and hence can be removed.
[]
> diff --git a/fs/afs/xattr.c b/fs/afs/xattr.c
[]
> @@ -71,7 +71,6 @@ static int afs_xattr_get_acl(const struct xattr_handler *handler,
>  	if (ret = 0) {
>  		ret = acl->size;
>  		if (size > 0) {
> -			ret = -ERANGE;
>  			if (acl->size > size)
>  				return -ERANGE;
>  			memcpy(buffer, acl->data, acl->size);

It looks like the ret = acl->size immediately
after the memcpy should be removed as well.
---
 fs/afs/xattr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/afs/xattr.c b/fs/afs/xattr.c
index c81f85003fc7..e21de2f166a4 100644
--- a/fs/afs/xattr.c
+++ b/fs/afs/xattr.c
@@ -71,11 +71,9 @@ static int afs_xattr_get_acl(const struct xattr_handler *handler,
 	if (ret = 0) {
 		ret = acl->size;
 		if (size > 0) {
-			ret = -ERANGE;
 			if (acl->size > size)
 				return -ERANGE;
 			memcpy(buffer, acl->data, acl->size);
-			ret = acl->size;
 		}
 		kfree(acl);
 	}

Re: [PATCH] afs: remove redundant assignment to variable ret

[David Howells]

Joe Perches <joe@perches.com> wrote:

> @@ -71,11 +71,9 @@ static int afs_xattr_get_acl(const struct xattr_handler *handler,
>  	if (ret = 0) {
>  		ret = acl->size;
>  		if (size > 0) {
> -			ret = -ERANGE;
>  			if (acl->size > size)
>  				return -ERANGE;
>  			memcpy(buffer, acl->data, acl->size);
> -			ret = acl->size;
>  		}
>  		kfree(acl);
>  	}

This is also the wrong solution.  See my reply to Colin.

David

Re: [PATCH] afs: remove redundant assignment to variable ret

[David Howells]

Colin King <colin.king@canonical.com> wrote:

> The variable ret is being assigned a value however this is never
> read and later it is being reassigned to a new value. The assignment
> is redundant and hence can be removed.

No.

>  	if (ret = 0) {
>  		ret = acl->size;
>  		if (size > 0) {
> -			ret = -ERANGE;
>  			if (acl->size > size)
>  				return -ERANGE;
>  			memcpy(buffer, acl->data, acl->size);

This is the wrong solution.  acl and key need releasing, so the return should
be a goto.

David

[PATCH] afs: remove redundant assignment to variable ret

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The variable ret is being assigned a value that is never read. The
assignment is redundant and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/afs/dir.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index d1e1caa23c8b..5608665ccb71 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -579,7 +579,6 @@ static int afs_do_lookup_one(struct inode *dir, struct dentry *dentry,
 		return ret;
 	}
 
-	ret = -ENOENT;
 	if (!cookie.found) {
 		_leave(" = -ENOENT [not found]");
 		return -ENOENT;
-- 
2.25.1

[PATCH] afs: remove redundant assignment to variable ret

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

The variable ret is being assigned a value that is never read because
the next statment returns from the function. The assignment is redundant
and can be removed.

Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/afs/server.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/afs/server.c b/fs/afs/server.c
index e82e452e2612..429cd226e4da 100644
--- a/fs/afs/server.c
+++ b/fs/afs/server.c
@@ -701,7 +701,6 @@ bool afs_check_server_record(struct afs_operation *op, struct afs_server *server
 	retries++;
 	if (retries = 4) {
 		_leave(" = f [stale]");
-		ret = -ESTALE;
 		return false;
 	}
 	goto retry;
-- 
2.27.0