Re: [PATCH] fiemap: fix problem with setting FIEMAP_EXTENT_LAST

[Andrew Morton]

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.

- has kerneldoc which documents non-existent arguments and fails to
  document the actual arguments.

- has a local variable called `tmp'.

- 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.

- 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.


Sigh.

Does this bugfix need to be backported into 2.6.29?  Earlier?  If so, why?

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/