Merge lp://qastaging/~csurbhi/ubuntu/maverick/btrfs-tools/btrfs-tools.fix-601874 into lp://qastaging/ubuntu/maverick/btrfs-tools
Proposed by
Surbhi Palande
| Status: | Needs review | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Proposed branch: | lp://qastaging/~csurbhi/ubuntu/maverick/btrfs-tools/btrfs-tools.fix-601874 | ||||||||
| Merge into: | lp://qastaging/ubuntu/maverick/btrfs-tools | ||||||||
| Diff against target: |
44 lines (+23/-1) 3 files modified
debian/changelog (+8/-1) debian/patches/04-fix-601874.patch (+14/-0) debian/patches/series (+1/-0) |
||||||||
| To merge this branch: | bzr merge lp://qastaging/~csurbhi/ubuntu/maverick/btrfs-tools/btrfs-tools.fix-601874 | ||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Colin Watson (community) | Needs Fixing | ||
|
Review via email:
|
|||
Description of the change
btrfs_search_slot() returns
* -1 when some error is encountered
* +1 when the item to be searched is not found
* 0 when the item is found successfully.
When read_node_slot() fails, this can only happen because of an I/O error or because malloc failed. Since the item to be searched is actually found, we should return a -1 and not +1 as it was originally returning. The caller expected that on +1, the path[] was appropriately populated and due to the error, it was not. Hence accessing the unpopulated path[] was segfaulting.
Do consider merging this for Maverick.
To post a comment you must log in.
Unmerged revisions
- 25. By Surbhi Palande
-
Returns -1 when a block cannot be read from disk. Closes (LP: #601874) and
(LP: #601877)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
Could you send this upstream for comment, and link to the relevant thread? There are a few cases I was wondering about, such as:
* Is it an I/O error if the slot passed to read_node_slot is out of range?
* Are all the cases where we run off the end of the while (1) loop in read_tree_block I/O errors? I can't quite puzzle that out.
* The caller only seems to care that p->nodes[0] is populated; what if read_node_slot fails at a deeper level?
I agree with you that p->nodes[0] isn't being properly populated, but since this needs to go through upstream anyway it would be good to confirm that this is the right way to handle this error.
A couple of packaging nits while I'm here:
* The version number should be -4ubuntu1 for merging into maverick (though this is easy to fix up at sponsorship time).
* Please make sure that new Ubuntu patches are pushed at the end of the patch queue, rather than at the start. That is, 'quilt push -a' before you run 'quilt new', and since this is source format 1.0 you'll need to also 'quilt pop -a' before committing. Since you've already created the patch, in this case it should suffice to pop all patches, edit debian/
Thanks!