[PATCH] btrfs: remove redundant assignment and check on variable ret

[PATCH] btrfs: remove redundant assignment and check on variable ret

[Colin King]

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

Variable ret is assigned to zero and is always zero throughout the
function.  Thus the check for ret being less than zero is always
false and so mapping_set_error always has an -EIO error passed to
it.  Hence we can remove the redundant assignment and check on ret.

Detected by CoverityScan, CID#1414312 ("Logically dead code")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 fs/btrfs/extent_io.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index d8da3edf2ac3..7922cd34ba82 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2447,7 +2447,6 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
 {
 	int uptodate = (err == 0);
 	struct extent_io_tree *tree;
-	int ret = 0;
 
 	tree = &BTRFS_I(page->mapping->host)->io_tree;
 
@@ -2458,8 +2457,7 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
 	if (!uptodate) {
 		ClearPageUptodate(page);
 		SetPageError(page);
-		ret = ret < 0 ? ret : -EIO;
-		mapping_set_error(page->mapping, ret);
+		mapping_set_error(page->mapping, -EIO);
 	}
 }
 
-- 
2.11.0

Re: [PATCH] btrfs: remove redundant assignment and check on variable ret

[David Sterba]

On Sat, May 06, 2017 at 11:01:05PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable ret is assigned to zero and is always zero throughout the
> function.  Thus the check for ret being less than zero is always
> false and so mapping_set_error always has an -EIO error passed to
> it.  Hence we can remove the redundant assignment and check on ret.
> 
> Detected by CoverityScan, CID#1414312 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Reviewed-by: David Sterba <dsterba@suse.com>

Re: [PATCH] btrfs: remove redundant assignment and check on variable ret

[Liu Bo]

On Sat, May 06, 2017 at 11:01:05PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Variable ret is assigned to zero and is always zero throughout the
> function.  Thus the check for ret being less than zero is always
> false and so mapping_set_error always has an -EIO error passed to
> it.  Hence we can remove the redundant assignment and check on ret.
> 
> Detected by CoverityScan, CID#1414312 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  fs/btrfs/extent_io.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index d8da3edf2ac3..7922cd34ba82 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -2447,7 +2447,6 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
>  {
>  	int uptodate = (err == 0);
>  	struct extent_io_tree *tree;
> -	int ret = 0;
>  
>  	tree = &BTRFS_I(page->mapping->host)->io_tree;
>  
> @@ -2458,8 +2457,7 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
>  	if (!uptodate) {
>  		ClearPageUptodate(page);
>  		SetPageError(page);
> -		ret = ret < 0 ? ret : -EIO;
> -		mapping_set_error(page->mapping, ret);
> +		mapping_set_error(page->mapping, -EIO);

The passed 'err' should be used as what ret did, ie.
ret = err < 0 ? err : -EIO;

Thanks,

-liubo
>  	}
>  }
>  
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] btrfs: remove redundant assignment and check on variable ret

[Colin Ian King]

On 09/05/17 17:55, Liu Bo wrote:
> On Sat, May 06, 2017 at 11:01:05PM +0100, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Variable ret is assigned to zero and is always zero throughout the
>> function.  Thus the check for ret being less than zero is always
>> false and so mapping_set_error always has an -EIO error passed to
>> it.  Hence we can remove the redundant assignment and check on ret.
>>
>> Detected by CoverityScan, CID#1414312 ("Logically dead code")
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  fs/btrfs/extent_io.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
>> index d8da3edf2ac3..7922cd34ba82 100644
>> --- a/fs/btrfs/extent_io.c
>> +++ b/fs/btrfs/extent_io.c
>> @@ -2447,7 +2447,6 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
>>  {
>>  	int uptodate = (err == 0);
>>  	struct extent_io_tree *tree;
>> -	int ret = 0;
>>  
>>  	tree = &BTRFS_I(page->mapping->host)->io_tree;
>>  
>> @@ -2458,8 +2457,7 @@ void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
>>  	if (!uptodate) {
>>  		ClearPageUptodate(page);
>>  		SetPageError(page);
>> -		ret = ret < 0 ? ret : -EIO;
>> -		mapping_set_error(page->mapping, ret);
>> +		mapping_set_error(page->mapping, -EIO);
> 
> The passed 'err' should be used as what ret did, ie.
> ret = err < 0 ? err : -EIO;

Ah, so the err passed into the function should be used instead of ret,
which makes more sense. Got it. I'll send a fix for that.

Colin

> 
> Thanks,
> 
> -liubo
>>  	}
>>  }
>>  
>> -- 
>> 2.11.0
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>