On Thu, Apr 30, 2009 at 03:40:47PM -0700, Andrew Morton wrote: > On Thu, 30 Apr 2009 13:44:51 -0400 > Josef Bacik <jbacik@xxxxxxxxxx> wrote: > > > This patch fixes a problem where the generic block based fiemap stuff would not > > properly set FIEMAP_EXTENT_LAST on the last extent. I've reworked things to > > keep track if we go past the EOF, and mark the last extent properly. The > > problem was reported by and tested by Eric Sandeen. > > > > bleeearrggh. __generic_block_fiemap() needs to be dragged out, shot, > stabbed and doused in gasoline. > > - uses stupid helper macros (blk_to_logical, logical_to_blk), thus > carefully obscuring the types of their incoming args and return value. > I did this to make it a bit more cleaner, so I didn't have a bunch of (blk << (inode)->i_blkbits) type statements everywhere. Do you have a better suggestion? Would you like an inline function, or do you want me to pepper the function with this stuff? > - has kerneldoc which documents non-existent arguments and fails to > document the actual arguments. > Yes thats my fault, sorry. Things got changed between my original submissions and I never fixed that, I will take care of that now. > - has a local variable called `tmp'. > Sorry, fixing that as well. > - Uses random and seemingly irrational mixture of u64's and `long > long's, thus carefully confusing readers who would prefer to see > loff_t, sector_t, etc so they have a fighting chance of understanding > what the hell the thing does. > Hrm well I needed the long long to see if we mapped past where we wanted to, but I can do that a different way. I will fix this as well. > - exists as useful counterexample for Documentation/CodingStyle > > - has undocumented locals called "length", "len" and "map_len", to > avoid any possibility of confusion. > > - has a local called `start_blk' which has a suspiciously-small > 32-bit type. Because of all the above intense obfuscation efforts I > just cannot be assed working out whether or not if this is an actual > bug. > > > Who the heck merged this turkey? > > <checks> > > Oh good, it wasn't me. > > your patch: > > - adds a string of booleans, unhelpfully typed with plain old `int'. > > - seems to think that sentences start with lower-case letters. > > I will fix all of this as well, sorry. > Sigh. > > Does this bugfix need to be backported into 2.6.29? Earlier? If so, why? > Yes it does, it will screw anybody up who tries to use fiemap beyond just the simple cases. I will send you a updated patch shortly. Thanks for the review, Josef -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
