All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.