| To: | Dave Chinner <david@xxxxxxxxxxxxx> |
|---|---|
| Subject: | Re: [PATCH 2/5] repair: fix some valgrind reported errors on i686 |
| From: | Christoph Hellwig <hch@xxxxxxxxxxxxx> |
| Date: | Mon, 10 Oct 2011 09:14:49 -0400 |
| Cc: | Christoph Hellwig <hch@xxxxxxxxxxxxx>, xfs@xxxxxxxxxxx |
| In-reply-to: | <20111010002017.GQ3159@dastard> |
| References: | <1318201910-11144-1-git-send-email-david@xxxxxxxxxxxxx> <1318201910-11144-3-git-send-email-david@xxxxxxxxxxxxx> <20111009234529.GA13527@xxxxxxxxxxxxx> <20111010002017.GQ3159@dastard> |
| User-agent: | Mutt/1.5.21 (2010-09-15) |
On Mon, Oct 10, 2011 at 11:20:17AM +1100, Dave Chinner wrote:
> > Call me stupid, but I can't see how this could actually be a real
> > life issue. The first thing we do in the loop is to to write
> > to fsbno in btree_find. I'm fine adding this to shut up warnins,
> > but I can't see a real issue.
>
> If btree_find() fails to find the key being looked up, it returns
> without having initialised fsbno.
Indeed. The normal pattern for btree_find seems to be:
obj = btree_find(tree, key, &key_ptr);
if (!obj)
return;
which makes this fine.
The code in pf_batch_read effectively boils down to that due to
the
while (bplist[num] && num < MAX_BUFS && fsbno < max_fsbno) {
loop which stops executing the first conditional that evaluated to
false. So I can't see how this actually ever had an affect, but I'm
fine with fixing the warnings.
Btw, did I mention that the while loop over the bplist is completely
non-intuitive?
|
| <Prev in Thread] | Current Thread | [Next in Thread> |
|---|---|---|
| ||
| Previous by Date: | Re: [PATCH] Changes to received review comments, Christoph Hellwig |
|---|---|
| Next by Date: | Re: [PATCH 4/5] repair: don't cache large blkmap allocations, Christoph Hellwig |
| Previous by Thread: | Re: [PATCH 2/5] repair: fix some valgrind reported errors on i686, Dave Chinner |
| Next by Thread: | [PATCH 1/5] repair: handle repair of image files on large sector size filesystems, Dave Chinner |
| Indexes: | [Date] [Thread] [Top] [All Lists] |