On Mon, Aug 04, 2014 at 01:42:23PM -0500, Eric Sandeen wrote: > On 8/4/14, 1:35 PM, Zach Brown wrote: > > On Fri, Aug 01, 2014 at 06:12:37PM -0500, Eric Sandeen wrote: > >> Reading the quota tree root may fail with ENOENT > >> if there is no quota, which is fine, but the code was > >> ignoring every other error as well, which is not fine. > > > > Kinda makes you want to write a test that would have caught this. > > > > Kinda. > > /me looks at ground, shuffles feet ... > > > Also, if you're still keen to iterate on this series, it looks like this > > pattern is copied and pasted a few times in open_ctree(). With > > temporary root pointers for each block, for some reason. A little > > helper function could take a bite out of open_ctree(). > > Hm, the uuid tree is roughly similar, but not exactly. I think those > are the only 2 "optional" roots (uuid because it'll get regenerated). > > I'm guessing the temporary root pointer is so we don't ever assign a > PTR_ERR to the root in fs_info? It took me a while to see what you meant. Yeah, using a temporary root makes sense. Using a different one for each block makes less sense. a = f(A); if (a) goto out; info->a = a; b = f(B); if (b) goto out; info->b = b; vs. r = f(A); if (r) goto out; info->a = r; r = f(B); if (r) goto out; info->b = r; - z -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
