Code review comment for lp://qastaging/~csurbhi/ubuntu/maverick/btrfs-tools/btrfs-tools.fix-601874

Revision history for this message
Colin Watson (cjwatson) wrote :

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

« Back to merge proposal