Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1343148 > unrolled thread

[PATCH] xfs: remove impossible condition

Back to article view | Back to linux.kernel

Contents

  [PATCH] xfs: remove impossible condition Luis de Bethencourt <luisbg@osg.samsung.com> - 2016-02-25 13:50 +0100
    Re: [PATCH] xfs: remove impossible condition Christoph Hellwig <hch@infradead.org> - 2016-03-01 13:10 +0100
      Re: [PATCH] xfs: remove impossible condition Luis de Bethencourt <luisbg@osg.samsung.com> - 2016-03-01 14:30 +0100
        Re: [PATCH] xfs: remove impossible condition Christoph Hellwig <hch@infradead.org> - 2016-03-01 19:30 +0100

#1343148 — [PATCH] xfs: remove impossible condition

bp_release is set to 0 just before the breakpoint of the for loop before
the conditional check (in line 458). The other breakpoint is a goto that
skips the dead code.

Addresses-Coverity-Id: 102338

Signed-off-by: Luis de Bethencourt <luisbg@osg.samsung.com>
---

Hi,

Thanks for reviewing this :)

Luis

 fs/xfs/libxfs/xfs_bmap.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 6a05166..1171795 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -477,10 +477,7 @@ xfs_bmap_check_leaf_extents(
 		}
 		block = XFS_BUF_TO_BLOCK(bp);
 	}
-	if (bp_release) {
-		bp_release = 0;
-		xfs_trans_brelse(NULL, bp);
-	}
+
 	return;
 
 error0:
-- 
2.5.1

[toc] | [next] | [standalone]

#1346557

On Thu, Feb 25, 2016 at 12:43:57PM +0000, Luis de Bethencourt wrote:
> bp_release is set to 0 just before the breakpoint of the for loop before
> the conditional check (in line 458). The other breakpoint is a goto that
> skips the dead code.

Yes, this looks correct:

Reviewed-by: Christoph Hellwig <hch@lst.de>

The whole code around this is rather grotty and could use some more
work, though..

[toc] | [prev] | [next] | [standalone]

#1346613

On 01/03/16 12:08, Christoph Hellwig wrote:
> On Thu, Feb 25, 2016 at 12:43:57PM +0000, Luis de Bethencourt wrote:
>> bp_release is set to 0 just before the breakpoint of the for loop before
>> the conditional check (in line 458). The other breakpoint is a goto that
>> skips the dead code.
> 
> Yes, this looks correct:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
> The whole code around this is rather grotty and could use some more
> work, though..
> 

Thanks for the review Christoph!

If you have some specific things in mind that I could fix in the code around
this, I am happy to add it to my ToDo list. Or I can continue reading it and
let you know of anything I see, so you can confirm before I try to fix it.

Regards,
Luis

[toc] | [prev] | [next] | [standalone]

#1346846

On Tue, Mar 01, 2016 at 01:23:33PM +0000, Luis de Bethencourt wrote:
> If you have some specific things in mind that I could fix in the code around
> this, I am happy to add it to my ToDo list. Or I can continue reading it and
> let you know of anything I see, so you can confirm before I try to fix it.

I don't think this mess is something I want to let newbies loose on :)

Thanks a lot for the patch again!

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web