headers
Josef Bacik | 1 May 2009 15:32
Picon
Favicon

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

On Thu, Apr 30, 2009 at 03:40:47PM -0700, Andrew Morton wrote:
> On Thu, 30 Apr 2009 13:44:51 -0400
> Josef Bacik <jbacik <at> 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.
>

I did this to make it a bit more cleaner, so I didn't have a bunch of 

(blk << (inode)->i_blkbits)

type statements everywhere.  Do you have a better suggestion?  Would you like an
inline function, or do you want me to pepper the function with this stuff?

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

Yes thats my fault, sorry.  Things got changed between my original submissions
and I never fixed that, I will take care of that now.
(Continue reading)

Permalink | Reply | 2 comments |
headers
Josef Bacik | 1 May 2009 16:35
Picon
Favicon

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

On Thu, Apr 30, 2009 at 03:40:47PM -0700, Andrew Morton wrote:
> On Thu, 30 Apr 2009 13:44:51 -0400
> Josef Bacik <jbacik <at> 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.
(Continue reading)

Permalink | Reply | 6 comments |
headers
Dave Kleikamp | 1 May 2009 19:41
Picon

[PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

From: Andrew Tridgell <tridge <at> samba.org>
Subject: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

When this option is enabled the VFAT filesystem will refuse to create
new files with long names. Accessing existing files with long names
will continue to work.

File names to be created must conform to the 8.3 format.  Mixed case is
not allowed in either the prefix or the suffix.

Signed-off-by: Andrew Tridgell <tridge <at> samba.org>
Signed-off-by: Dave Kleikamp <shaggy <at> linux.vnet.ibm.com>
Acked-by: Steve French <sfrench <at> us.ibm.com>
Cc: Ogawa Hirofumi <hirofumi <at> mail.parknet.co.jp>
Cc: Mingming Cao <cmm <at> us.ibm.com>
---
 fs/fat/Kconfig      |   18 ++++++++++++++++++
 fs/fat/namei_vfat.c |   26 +++++++++++++++++++++-----
 2 files changed, 39 insertions(+), 5 deletions(-)

diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
index 182f9ff..1439681 100644
--- a/fs/fat/Kconfig
+++ b/fs/fat/Kconfig
 <at>  <at>  -98,3 +98,21  <at>  <at>  config FAT_DEFAULT_IOCHARSET

 	  Enable any character sets you need in File Systems/Native Language
 	  Support.
+
+config VFAT_NO_CREATE_WITH_LONGNAMES
(Continue reading)

Permalink | Reply | 176 comments |
headers
Christoph Hellwig | 1 May 2009 19:47
Favicon

Re: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

On Fri, May 01, 2009 at 12:41:29PM -0500, Dave Kleikamp wrote:
> From: Andrew Tridgell <tridge <at> samba.org>
> Subject: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option
> 
> When this option is enabled the VFAT filesystem will refuse to create
> new files with long names. Accessing existing files with long names
> will continue to work.
> 
> File names to be created must conform to the 8.3 format.  Mixed case is
> not allowed in either the prefix or the suffix.

This doesn't make any sense as a compile time option. Might make sense
as a mount option, but I'd like to hear a rationale for it first.

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 8 comments |
headers
Dave Kleikamp | 1 May 2009 20:12
Picon

Re: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

On Fri, 2009-05-01 at 13:47 -0400, Christoph Hellwig wrote:
> On Fri, May 01, 2009 at 12:41:29PM -0500, Dave Kleikamp wrote:
> > From: Andrew Tridgell <tridge <at> samba.org>
> > Subject: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option
> > 
> > When this option is enabled the VFAT filesystem will refuse to create
> > new files with long names. Accessing existing files with long names
> > will continue to work.
> > 
> > File names to be created must conform to the 8.3 format.  Mixed case is
> > not allowed in either the prefix or the suffix.
> 
> This doesn't make any sense as a compile time option. Might make sense
> as a mount option, but I'd like to hear a rationale for it first.

Some linux-based devices would be happy not to contain code to create
the long name at all.

Thanks,
Shaggy
--

-- 
David Kleikamp
IBM Linux Technology Center

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
(Continue reading)

Permalink | Reply | 6 comments |
headers
Michael Tokarev | 1 May 2009 20:19
Picon

Re: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

Dave Kleikamp wrote:
> On Fri, 2009-05-01 at 13:47 -0400, Christoph Hellwig wrote:
>> On Fri, May 01, 2009 at 12:41:29PM -0500, Dave Kleikamp wrote:
>>> From: Andrew Tridgell <tridge <at> samba.org>
>>> Subject: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option
>>>
>>> When this option is enabled the VFAT filesystem will refuse to create
>>> new files with long names. Accessing existing files with long names
>>> will continue to work.
>>>
>>> File names to be created must conform to the 8.3 format.  Mixed case is
>>> not allowed in either the prefix or the suffix.
>> This doesn't make any sense as a compile time option. Might make sense
>> as a mount option, but I'd like to hear a rationale for it first.
> 
> Some linux-based devices would be happy not to contain code to create
> the long name at all.

Well, is that a rationale per se?  Which devices they are and why?

But besides, why `msdos' filesystem is not sufficient?
It contains no code to create long file names and no code to
read such names either.

/mjt
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
(Continue reading)

Permalink | Reply | 2 comments |
headers
Christoph Hellwig | 1 May 2009 20:30
Favicon

Re: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

On Fri, May 01, 2009 at 01:12:42PM -0500, Dave Kleikamp wrote:
> Some linux-based devices would be happy not to contain code to create
> the long name at all.

Then patch it out.

--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo <at> vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Permalink | Reply | 0 comments |
headers
Andrew Morton | 1 May 2009 20:26

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

On Fri, 1 May 2009 09:32:26 -0400 Josef Bacik <josef <at> redhat.com> wrote:

> On Thu, Apr 30, 2009 at 03:40:47PM -0700, Andrew Morton wrote:
> > On Thu, 30 Apr 2009 13:44:51 -0400
> > Josef Bacik <jbacik <at> 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.
> >
> 
> I did this to make it a bit more cleaner, so I didn't have a bunch of 
> 
> (blk << (inode)->i_blkbits)
> 
> type statements everywhere.  Do you have a better suggestion?  Would you like an
> inline function, or do you want me to pepper the function with this stuff?

An inline would be much better - the C type information in this sort of
code can be a big help if well-used.

> > - Uses random and seemingly irrational mixture of u64's and `long
(Continue reading)

Permalink | Reply | 0 comments |
headers
Dave Kleikamp | 1 May 2009 21:09
Picon

Re: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option

On Fri, 2009-05-01 at 22:19 +0400, Michael Tokarev wrote:
> Dave Kleikamp wrote:
> > On Fri, 2009-05-01 at 13:47 -0400, Christoph Hellwig wrote:
> >> On Fri, May 01, 2009 at 12:41:29PM -0500, Dave Kleikamp wrote:
> >>> From: Andrew Tridgell <tridge <at> samba.org>
> >>> Subject: [PATCH] Add CONFIG_VFAT_NO_CREATE_WITH_LONGNAMES option
> >>>
> >>> When this option is enabled the VFAT filesystem will refuse to create
> >>> new files with long names. Accessing existing files with long names
> >>> will continue to work.
> >>>
> >>> File names to be created must conform to the 8.3 format.  Mixed case is
> >>> not allowed in either the prefix or the suffix.
> >> This doesn't make any sense as a compile time option. Might make sense
> >> as a mount option, but I'd like to hear a rationale for it first.
> > 
> > Some linux-based devices would be happy not to contain code to create
> > the long name at all.
> 
> Well, is that a rationale per se?  Which devices they are and why?

Could be anything.  cameras, phones, etc.  Anything that might be
mountable by a host computer in order to share files, or to write to a
device that can be shared by other computers or devices.

> But besides, why `msdos' filesystem is not sufficient?
> It contains no code to create long file names and no code to
> read such names either.

An example, an mp3 player wants to read files with long mixed-case
(Continue reading)

Permalink | Reply | 0 comments |
headers
Andrew Morton | 1 May 2009 21:50

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

On Fri, 1 May 2009 10:35:17 -0400
Josef Bacik <josef <at> redhat.com> wrote:

> 
> Here is an updated patch.

This looks heaps better to me.

Officially we shouldn't do this sort of thing in a -stable/-rc4 patch. 
We should go for a minimal backportable fixup then do the cleanups
later, in an -rc1.

But we could sneak this in anyway, I expect.  Which way would you
prefer?

>  What am I supposed to do about these types?
> inode->i_size is loff_t, bh->b_size is size_t, bh->b_blocknr is sector_t, not to
> mention that all of the fieinfo fields are either u64 or u32.

We're missing a number of types and sometimes it's hard to find one
which is appropriate for the problem at hand.

It is appropriate that the fields which are communicated with userspace
have the bare u32/u64 types.  One way of handling these
information-free types is to name them well.  Another is to avoid using
them until the last possible moment - choose well-typed and well-named
locals and copy then to/from the target structure immediately
before/after the userspace copy in/out.

>  Is what I've done
(Continue reading)

Permalink | Reply | 4 comments |

Gmane