[PATCH-next] afs: Remove logically dead code

Advait Dhamorikar posted 1 patch 1 week, 1 day ago
fs/afs/dir.c | 2 --
1 file changed, 2 deletions(-)
[PATCH-next] afs: Remove logically dead code
Posted by Advait Dhamorikar 1 week, 1 day ago
Initially ret is initialized to 0 and its value is then never updated 
again, thus the indicated dead code may have performed some action;
that action will never occur.

Signed-off-by: Advait Dhamorikar <advaitdhamorikar@gmail.com>
---
 fs/afs/dir.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index b6a202fd9926..afa7c24828ec 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -502,8 +502,6 @@ static int afs_dir_iterate_contents(struct inode *dir, struct dir_context *ctx)
 	iterate_folioq(&iter, iov_iter_count(&iter), dvnode, ctx,
 		       afs_dir_iterate_step);
 
-	if (ret == -ESTALE)
-		afs_invalidate_dir(dvnode, afs_dir_invalid_iter_stale);
 	return ret;
 }
 
-- 
2.34.1
Re: [PATCH-next] afs: Remove logically dead code
Posted by David Howells 6 days, 12 hours ago
Advait Dhamorikar <advaitdhamorikar@gmail.com> wrote:

>  	iterate_folioq(&iter, iov_iter_count(&iter), dvnode, ctx,
>  		       afs_dir_iterate_step);
>  
> -	if (ret == -ESTALE)
> -		afs_invalidate_dir(dvnode, afs_dir_invalid_iter_stale);
>  	return ret;

Removing this is the wrong thing to do.  However, you're correct that ret is
never set to -ESTALE.

A better solution, I think, is to check the result of iterate_folioq(),
invalidating the dir and returning -ESTALE if 0 and if the iterator count is
not 0.

David
Re: [PATCH-next] afs: Remove logically dead code
Posted by Advait Dhamorikar 6 days, 8 hours ago
Hello David,

> A better solution, I think, is to check the result of iterate_folioq(),
> invalidating the dir and returning -ESTALE if 0 and if the iterator count is
> not 0.

Thanks for the insights and the feedback, however it looks like this patch
is no longer relevant as there have been significant changes to the
file since then.

Best regards,
Advait
Re: [PATCH-next] afs: Remove logically dead code
Posted by David Howells 6 days, 11 hours ago
I think the attached change is what I want.

David
---
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index b6a202fd9926..2adc034603f2 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -499,11 +499,11 @@ static int afs_dir_iterate_contents(struct inode *dir, struct dir_context *ctx)
 	iov_iter_folio_queue(&iter, ITER_SOURCE, dvnode->directory, 0, 0, i_size);
 	iov_iter_advance(&iter, round_down(ctx->pos, AFS_DIR_BLOCK_SIZE));
 
-	iterate_folioq(&iter, iov_iter_count(&iter), dvnode, ctx,
-		       afs_dir_iterate_step);
-
-	if (ret == -ESTALE)
+	if (!iterate_folioq(&iter, iov_iter_count(&iter), dvnode, ctx,
+			    afs_dir_iterate_step)) {
 		afs_invalidate_dir(dvnode, afs_dir_invalid_iter_stale);
+		ret = -ESTALE;
+	}
 	return ret;
 }