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