Merge lp:~csurbhi/ubuntu/maverick/btrfs-tools/btrfs-tools.fix-601874 into lp:ubuntu/maverick/btrfs-tools

[Surbhi Palande]

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.

[Colin Watson (cjwatson)]

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/patches/series to move the new patch to the end, and then push up to the new patch, refresh, and pop back down.

Thanks!

review: Needs Fixing

Unmerged revisions

25. By Surbhi Palande on 2010-07-28

Returns -1 when a block cannot be read from disk. Closes (LP: #601874) and
(LP: #601877)

=== modified file 'debian/changelog'
--- debian/changelog	2010-06-20 21:22:57 +0000
+++ debian/changelog	2010-08-04 08:45:54 +0000
@@ -1,4 +1,11 @@
-btrfs-tools (0.19+20100601-3) unstable; urgency=low
+btrfs-tools (0.19+20100601-4ubuntu0+2) maverick; urgency=low
+
+  * Returns -1 when a block cannot be read from disk. Closes (LP: #601874) and 
+    (LP: #601877)
+
+ -- Surbhi Palande <[e-mail]>  Tue, 27 Jul 2010 19:45:00 +0300
+
+btrfs-tools (0.19+20100601-4) unstable; urgency=low
 
   * Updating year in copyright file.
   * Adding overrides for clean target to make sure package builds twice
 
=== added file 'debian/patches/04-fix-601874.patch'
--- debian/patches/04-fix-601874.patch	1970-01-01 00:00:00 +0000
+++ debian/patches/04-fix-601874.patch	2010-08-04 08:45:54 +0000
@@ -0,0 +1,14 @@
+Index: b/ctree.c
+===================================================================
+--- a/ctree.c
++++ b/ctree.c
+@@ -1264,6 +1264,9 @@
+ 						 key->objectid);
+ 
+ 			b = read_node_slot(root, b, slot);
++			if (!b) {
++				return -1;	
++			}
+ 		} else {
+ 			p->slots[level] = slot;
+ 			if (ins_len > 0 &&
 
=== modified file 'debian/patches/series'
--- debian/patches/series	2010-06-20 21:22:57 +0000
+++ debian/patches/series	2010-08-04 08:45:54 +0000
@@ -1,3 +1,4 @@
+04-fix-601874.patch
 01-labels.patch
 02-ftbfs.patch
 03-glibc.patch