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.
next prev parent 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.