linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Josef Bacik <jbacik@redhat.com>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	sandeen@redhat.com, jbacik@redhat.com, stable@kernel.org
Subject: Re: [PATCH] fiemap: fix problem with setting FIEMAP_EXTENT_LAST
Date: Thu, 30 Apr 2009 15:40:47 -0700	[thread overview]
Message-ID: <20090430154047.5485759f.akpm@linux-foundation.org> (raw)
In-Reply-To: <1241113491-9031-1-git-send-email-jbacik@redhat.com>

On Thu, 30 Apr 2009 13:44:51 -0400
Josef Bacik <jbacik@redhat.com> 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.

  reply	other threads:[~2009-04-30 22:45 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-04-30 17:44 [PATCH] fiemap: fix problem with setting FIEMAP_EXTENT_LAST Josef Bacik
2009-04-30 22:40 ` Andrew Morton [this message]
2009-05-01 13:32   ` Josef Bacik
2009-05-01 18:26     ` Andrew Morton
2009-05-01 14:35   ` Josef Bacik
2009-05-01 19:50     ` Andrew Morton
2009-05-01 20:16       ` Josef Bacik
2009-05-01 20:47         ` Andrew Morton

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090430154047.5485759f.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=jbacik@redhat.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=stable@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).