[PATCH vfat] allow retrieving entries with trailing dots

[PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

Hello Ogawa,

This fixes accessing vfat entries with trailing dots created by an external
vfat driver (like the one in IOMEGA home network hard drives)

Philippe

--

Some vfat-formatted network disks that are also usb disk do not discard
trailing dots when creating files or directories via ftp.
Connecting afterwards this drive via usb to a linux machine leads to the
following problem :
if one issues the `ls' or `find' command, one gets this message :

find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory

Fix that by first trying to retrieve the entry with the full name, and only if
that fails and there are trailing dots in the searched name, try then to find
the truncated name.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>

--- a/fs/fat/namei_vfat.c	2009-09-10 00:13:59.000000000 +0200
+++ b/fs/fat/namei_vfat.c	2010-02-08 02:28:37.010096903 +0100
@@ -702,9 +702,22 @@
 static int vfat_find(struct inode *dir, struct qstr *qname,
 		     struct fat_slot_info *sinfo)
 {
-	unsigned int len = vfat_striptail_len(qname);
+	int err;
+	unsigned int len;
+
+	/* Some combined ethernet + usb external hard drive do not
+	 * remove the trailing dots when creating entries in ethernet mode.
+	 * (e.g. Iomega Home Network Hard Drive)
+	 * Make accessing those entries possible.
+	 */
+	err = fat_search_long(dir, qname->name, qname->len, sinfo);
+	if (!err)
+		return err;
+	len = vfat_striptail_len(qname);
 	if (len == 0)
 		return -ENOENT;
+	if (len == qname->len)
+		return err;
 	return fat_search_long(dir, qname->name, len, sinfo);
 }
 

Re: [PATCH vfat] allow retrieving entries with trailing dots

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

> --- a/fs/fat/namei_vfat.c	2009-09-10 00:13:59.000000000 +0200
> +++ b/fs/fat/namei_vfat.c	2010-02-08 02:28:37.010096903 +0100
> @@ -702,9 +702,22 @@
>  static int vfat_find(struct inode *dir, struct qstr *qname,
>  		     struct fat_slot_info *sinfo)
>  {
> -	unsigned int len = vfat_striptail_len(qname);
> +	int err;
> +	unsigned int len;
> +
> +	/* Some combined ethernet + usb external hard drive do not
> +	 * remove the trailing dots when creating entries in ethernet mode.
> +	 * (e.g. Iomega Home Network Hard Drive)
> +	 * Make accessing those entries possible.
> +	 */
> +	err = fat_search_long(dir, qname->name, qname->len, sinfo);
> +	if (!err)
> +		return err;
> +	len = vfat_striptail_len(qname);
>  	if (len == 0)
>  		return -ENOENT;
> +	if (len == qname->len)
> +		return err;
>  	return fat_search_long(dir, qname->name, len, sinfo);
>  }

This would be bad for both (standard and IO-MEGA hack).

This introduces unneeded directory-parse to standard one.  And for
IO-MEGA, this wouldn't provide proper filename handling.

If it wants to handle the tailing-dot as a part of filename, it
shouldn't be able to access to the stripped-dots filename. (For simple
example, I guess you can't do "mv a a." with this patch.)

I'm not still sure if we should include this or not, however, I think
improper/imperfect quick hack wouldn't be acceptable at least.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

Hello Ogawa

Thanks for the quick answer.

On Wed, Mar 10, 2010 at 11:44:27PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> > --- a/fs/fat/namei_vfat.c	2009-09-10 00:13:59.000000000 +0200
> > +++ b/fs/fat/namei_vfat.c	2010-02-08 02:28:37.010096903 +0100
> > @@ -702,9 +702,22 @@
> >  static int vfat_find(struct inode *dir, struct qstr *qname,
> >  		     struct fat_slot_info *sinfo)
> >  {
> > -	unsigned int len = vfat_striptail_len(qname);
> > +	int err;
> > +	unsigned int len;
> > +
> > +	/* Some combined ethernet + usb external hard drive do not
> > +	 * remove the trailing dots when creating entries in ethernet mode.
> > +	 * (e.g. Iomega Home Network Hard Drive)
> > +	 * Make accessing those entries possible.
> > +	 */
> > +	err = fat_search_long(dir, qname->name, qname->len, sinfo);
> > +	if (!err)
> > +		return err;
> > +	len = vfat_striptail_len(qname);
> >  	if (len == 0)
> >  		return -ENOENT;
> > +	if (len == qname->len)
> > +		return err;
> >  	return fat_search_long(dir, qname->name, len, sinfo);
> >  }
> 
> This would be bad for both (standard and IO-MEGA hack).
> 
> This introduces unneeded directory-parse to standard one.  And for

No.  There will only be a second parse if someone is looking for a filename
with a trailing dot, and it is not found (*).  I there is no trailing dot in qname, only the first fat_search_long will be called, because len after vfat_striptail_len would be equal to qname->len.

> IO-MEGA, this wouldn't provide proper filename handling.

For accessing IO-MEGA disk in read mode, this is perfect.  I didn't want
to replicate IO-MEGA write behaviour here, only fix the read behaviour for
such simple commands as 'ls', 'find' and all the directory browsers.

> 
> If it wants to handle the tailing-dot as a part of filename, it
> shouldn't be able to access to the stripped-dots filename. (For simple
> example, I guess you can't do "mv a a." with this patch.)

As I explained above, I only fix read-access on IO-MEGA drives, while
preserving standard behaviour for write mode.

But I'll try your testcase asap.  Which behaviour do you expect ?
I would expect a no-op, because I did not change the write-behaviour.

Best regards

Philippe

(*) This will never happen with 'ls', 'find' and friends who get their name
list from getdents.  This could only happen if someone tries to open
a file called 'a.' on his vfat file-system, which of course does not exist
in his normal vfat file-system.

Re: [PATCH vfat] allow retrieving entries with trailing dots

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> This introduces unneeded directory-parse to standard one.  And for
>
> No.  There will only be a second parse if someone is looking for a filename
> with a trailing dot, and it is not found (*).  I there is no trailing dot in qname, only the first fat_search_long will be called, because len after vfat_striptail_len would be equal to qname->len.

Yes, I'm saying about trailing-dot filename case. Any disadvantage to
standard one is unacceptable to workaround.

>> IO-MEGA, this wouldn't provide proper filename handling.
>
> For accessing IO-MEGA disk in read mode, this is perfect.  I didn't want
> to replicate IO-MEGA write behaviour here, only fix the read behaviour for
> such simple commands as 'ls', 'find' and all the directory browsers.
>
>> If it wants to handle the tailing-dot as a part of filename, it
>> shouldn't be able to access to the stripped-dots filename. (For simple
>> example, I guess you can't do "mv a a." with this patch.)
>
> As I explained above, I only fix read-access on IO-MEGA drives, while
> preserving standard behaviour for write mode.
>
> But I'll try your testcase asap.  Which behaviour do you expect ?
> I would expect a no-op, because I did not change the write-behaviour.

Those sound like strange.  Well, I expect there is no any change to
standard one for IO-MEGA.

And I can't see what is your read-access mean in here. What did this
expect to behave like e.g. following operations,

	$ ls
        a.. a. a
        $ rm -rf *

        $ ls
        a..
        $ touch a.
        $ touch a
...

I assumed you want to define "a." and "a" are different name on
"mv a a.", and _totally_.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

Hi Ogawa,

On Thu, Mar 11, 2010 at 02:16:26AM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> >> This introduces unneeded directory-parse to standard one.  And for
> >
> > No.  There will only be a second parse if someone is looking for a filename
> > with a trailing dot, and it is not found (*).  I there is no trailing dot in qname, only the first fat_search_long will be called, because len after vfat_striptail_len would be equal to qname->len.
> 
> Yes, I'm saying about trailing-dot filename case. Any disadvantage to
> standard one is unacceptable to workaround.

This is unavoidable.
Let me explain again, with better words I hope,
IOMEGA firmware allows creation of files/directories whose name have
trailing dots.

When I connect such a disk (which is vfat-formatted) via USB to my
linux computer, I want consistency between what getdents says and what
open or stat will do : if getdents says there is a file called "a.",
currently open("a.") and stat("a.") fail with ENOENT No such file or directory.
I would like open("a.") or stat("a.") to succeed.  Nothing more.
For that, I need to first search for the not truncated name.  If that fails
and the filename can be truncated then and only then, try again with the
truncated name.  I cannot let getdents truncate the name, because there 
could also be a different file really called "a".

There would be no advantage in making the change in fat_search_long,
because while searching for "a.", even if it finds a "a" entry, it must
continue till the end of the directory to be sure there is no "a.".

> 
> >> IO-MEGA, this wouldn't provide proper filename handling.
> >
> > For accessing IO-MEGA disk in read mode, this is perfect.  I didn't want
> > to replicate IO-MEGA write behaviour here, only fix the read behaviour for
> > such simple commands as 'ls', 'find' and all the directory browsers.
> >
> >> If it wants to handle the tailing-dot as a part of filename, it
> >> shouldn't be able to access to the stripped-dots filename. (For simple
> >> example, I guess you can't do "mv a a." with this patch.)
> >
> > As I explained above, I only fix read-access on IO-MEGA drives, while
> > preserving standard behaviour for write mode.
> >
> > But I'll try your testcase asap.  Which behaviour do you expect ?
> > I would expect a no-op, because I did not change the write-behaviour.
> 
> Those sound like strange.  Well, I expect there is no any change to
> standard one for IO-MEGA.
> 
> And I can't see what is your read-access mean in here. What did this
> expect to behave like e.g. following operations,

Sorry bad wording. I meant I do not want to change the behaviour for file
creation, only to fix the behaviour while accessi8ng existing files.

> 
> 	$ ls
>         a.. a. a
>         $ rm -rf *
> 
>         $ ls
>         a..
>         $ touch a.
>         $ touch a
> ...
> 
> I assumed you want to define "a." and "a" are different name on
> "mv a a.", and _totally_.

For file creation and renaming, I want to introduce no change, because there
is no problem.  If one wants to create a "a." file on a IO-MEGA disk suing
linux and USB, it is currently called "a", and that will remain exactly the
same.

Best regards

Philippe

Re: [PATCH vfat] allow retrieving entries with trailing dots

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> >> This introduces unneeded directory-parse to standard one.  And for
>> >
>> > No.  There will only be a second parse if someone is looking for a filename
>> > with a trailing dot, and it is not found (*).  I there is no trailing dot in qname, only the first fat_search_long will be called, because len after vfat_striptail_len would be equal to qname->len.
>> 
>> Yes, I'm saying about trailing-dot filename case. Any disadvantage to
>> standard one is unacceptable to workaround.
>
> This is unavoidable.

We would be able to introduce new mount option to do it if needed.

>> 	$ ls
>>         a.. a. a
>>         $ rm -rf *
>> 
>>         $ ls
>>         a..
>>         $ touch a.
>>         $ touch a
>> ...
>> 
>> I assumed you want to define "a." and "a" are different name on
>> "mv a a.", and _totally_.
>
> For file creation and renaming, I want to introduce no change, because there
> is no problem.  If one wants to create a "a." file on a IO-MEGA disk suing
> linux and USB, it is currently called "a", and that will remain exactly the
> same.

This changed vfat_find(), so, this patch will change the behavior of all
callers more or less. And the behavior seems to be really strange, you
can remove "a.", but you can't create it? The behavior sounds random,
right?

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

Hello Ogawa,

On Thu, Mar 11, 2010 at 06:26:54PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> >> >> This introduces unneeded directory-parse to standard one.  And for
> >> >
> >> > No.  There will only be a second parse if someone is looking for a filename
> >> > with a trailing dot, and it is not found (*).  I there is no trailing dot in qname, only the first fat_search_long will be called, because len after vfat_striptail_len would be equal to qname->len.
> >> 
> >> Yes, I'm saying about trailing-dot filename case. Any disadvantage to
> >> standard one is unacceptable to workaround.
> >
> > This is unavoidable.

We could perhaps reduce the disadvantage by moving the logic (test first
with the trailing dots, and then without if needed) into fat_search_long
instead of putting it in vfat_find as my patch proposal does.  This way
the name to find would only be compared once to unrelated entries,
instead of possibly twice as my patch does.

But therefore I need your help : fat_search_long isn't easy to read/modify.

The logic could be modified as such :
    - if the searched name contains trailing dots, first compare
	the truncated part to the same length of the directory entry
    - if they are the same, test the length of the rest of the directory entry
	- if length_of_rest is 0, this could be the matching entry,
	    but there could be a better one later; keep searching till the end.
	- if length_of_rest is not 0, compare the rest with the trailing dots
	    - if rests are equal, we have found it; return
	    - if rests are unequal, continue searching

> 
> We would be able to introduce new mount option to do it if needed.

A new mount option should be the last programming option.  It is better to
automatically do the right thing than to require the user to figure out
he must add a mount option in /etc/fstab or whatever.  Remember this is
for hot-plug disks.

> 
> >> 	$ ls
> >>         a.. a. a
> >>         $ rm -rf *
> >> 
> >>         $ ls
> >>         a..
> >>         $ touch a.
> >>         $ touch a
> >> ...
> >> 
> >> I assumed you want to define "a." and "a" are different name on
> >> "mv a a.", and _totally_.
> >
> > For file creation and renaming, I want to introduce no change, because there
> > is no problem.  If one wants to create a "a." file on a IO-MEGA disk suing
> > linux and USB, it is currently called "a", and that will remain exactly the
> > same.
> 
> This changed vfat_find(), so, this patch will change the behavior of all
> callers more or less. And the behavior seems to be really strange, you
> can remove "a.", but you can't create it?

Yes, you can remove any existing file, but if you want to create a file, 
the name creation rules are kept unchanged. So, creating "a." will
succeed, it will actually be called "a" on disk, but you can still refer
to it as "a." : that will succeed.  That's the strange part of the behavior
but that part is already present for compatibility reasons nor you nor me
can do anything about :(

> The behavior sounds random, right?

It's a compromise to avoid creating name entries that are not universally
accepted, but to allow accessing existing files and directories.

The mount option could be useful then to allow the creation of file and
directory names with trailing dot, but consistency between getdents and
stat/open must be automatic.

Best regards

Philippe

Re: [PATCH vfat] allow retrieving entries with trailing dots

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> > This is unavoidable.
>
> We could perhaps reduce the disadvantage by moving the logic (test first
> with the trailing dots, and then without if needed) into fat_search_long
> instead of putting it in vfat_find as my patch proposal does.  This way
> the name to find would only be compared once to unrelated entries,
> instead of possibly twice as my patch does.
>
> But therefore I need your help : fat_search_long isn't easy to read/modify.
>
> The logic could be modified as such :
>     - if the searched name contains trailing dots, first compare
> 	the truncated part to the same length of the directory entry
>     - if they are the same, test the length of the rest of the directory entry
> 	- if length_of_rest is 0, this could be the matching entry,
> 	    but there could be a better one later; keep searching till the end.
> 	- if length_of_rest is not 0, compare the rest with the trailing dots
> 	    - if rests are equal, we have found it; return
> 	    - if rests are unequal, continue searching
>
>> 
>> We would be able to introduce new mount option to do it if needed.
>
> A new mount option should be the last programming option.  It is better to
> automatically do the right thing than to require the user to figure out
> he must add a mount option in /etc/fstab or whatever.  Remember this is
> for hot-plug disks.

Sorry, but I'm not thinking this is primary one. So, requiring option
for it to avoid disadvantage of normal users, it sounds good.

>> This changed vfat_find(), so, this patch will change the behavior of all
>> callers more or less. And the behavior seems to be really strange, you
>> can remove "a.", but you can't create it?
>
> Yes, you can remove any existing file, but if you want to create a file, 
> the name creation rules are kept unchanged. So, creating "a." will
> succeed, it will actually be called "a" on disk, but you can still refer
> to it as "a." : that will succeed.  That's the strange part of the behavior
> but that part is already present for compatibility reasons nor you nor me
> can do anything about :(
>
>> The behavior sounds random, right?
>
> It's a compromise to avoid creating name entries that are not universally
> accepted, but to allow accessing existing files and directories.
>
> The mount option could be useful then to allow the creation of file and
> directory names with trailing dot, but consistency between getdents and
> stat/open must be automatic.

No, this breaks consistency. With this patch, unlink("a."), then
open("a.", O_CREAT) and write(), the result depend on existent
files. This patch is providing two files on one name.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

Hello Ogawa,

On Thu, Mar 11, 2010 at 09:41:33PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> >> > This is unavoidable.
> >
> > We could perhaps reduce the disadvantage by moving the logic (test first
> > with the trailing dots, and then without if needed) into fat_search_long
> > instead of putting it in vfat_find as my patch proposal does.  This way
> > the name to find would only be compared once to unrelated entries,
> > instead of possibly twice as my patch does.
> >
> > But therefore I need your help : fat_search_long isn't easy to read/modify.
> >
> > The logic could be modified as such :
> >     - if the searched name contains trailing dots, first compare
> > 	the truncated part to the same length of the directory entry
> >     - if they are the same, test the length of the rest of the directory entry
> > 	- if length_of_rest is 0, this could be the matching entry,
> > 	    but there could be a better one later; keep searching till the end.
> > 	- if length_of_rest is not 0, compare the rest with the trailing dots
> > 	    - if rests are equal, we have found it; return
> > 	    - if rests are unequal, continue searching
> >
> >> 
> >> We would be able to introduce new mount option to do it if needed.
> >
> > A new mount option should be the last programming option.  It is better to
> > automatically do the right thing than to require the user to figure out
> > he must add a mount option in /etc/fstab or whatever.  Remember this is
> > for hot-plug disks.
> 
> Sorry, but I'm not thinking this is primary one. So, requiring option
> for it to avoid disadvantage of normal users, it sounds good.
> 
> >> This changed vfat_find(), so, this patch will change the behavior of all
> >> callers more or less. And the behavior seems to be really strange, you
> >> can remove "a.", but you can't create it?
> >
> > Yes, you can remove any existing file, but if you want to create a file, 
> > the name creation rules are kept unchanged. So, creating "a." will
> > succeed, it will actually be called "a" on disk, but you can still refer
> > to it as "a." : that will succeed.  That's the strange part of the behavior
> > but that part is already present for compatibility reasons nor you nor me
> > can do anything about :(
> >
> >> The behavior sounds random, right?
> >
> > It's a compromise to avoid creating name entries that are not universally
> > accepted, but to allow accessing existing files and directories.
> >
> > The mount option could be useful then to allow the creation of file and
> > directory names with trailing dot, but consistency between getdents and
> > stat/open must be automatic.
> 
> No, this breaks consistency. With this patch, unlink("a."), then
> open("a.", O_CREAT) and write(), the result depend on existent
> files. This patch is providing two files on one name.

To avoid that, we could remember that we have found a filename with a trailing
dot (in that directory or in the whole disk), and if that's the case then
we are allowed to create filenames with trailing dots.

Philippe

Re: [PATCH vfat] allow retrieving entries with trailing dots

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> No, this breaks consistency. With this patch, unlink("a."), then
>> open("a.", O_CREAT) and write(), the result depend on existent
>> files. This patch is providing two files on one name.
>
> To avoid that, we could remember that we have found a filename with a trailing
> dot (in that directory or in the whole disk), and if that's the case then
> we are allowed to create filenames with trailing dots.

It sounds like dirty hack, and it would make more complex situations.
And the user can umount/mount (or reboot, etc.) before open().

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] IOMEGA network drive compatibility

[Philippe De Muyter]

Hello Ogawa,

I changed the subject to make it more explicit.

On Sat, Mar 13, 2010 at 10:06:25PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> >> No, this breaks consistency. With this patch, unlink("a."), then
> >> open("a.", O_CREAT) and write(), the result depend on existent
> >> files. This patch is providing two files on one name.
> >
> > To avoid that, we could remember that we have found a filename with a trailing
> > dot (in that directory or in the whole disk), and if that's the case then
> > we are allowed to create filenames with trailing dots.
> 
> It sounds like dirty hack, and it would make more complex situations.
> And the user can umount/mount (or reboot, etc.) before open().

You can call that a hack, but it certainly is better than the current situation.
IOMEGA network drive firmware allows to create two files in the same directory
differing only by the trailing dots.  Suppose you have in the same directory
a file called "123456789" and a file called "123456789." (same name + a
trailing dot).  When afterwards connected via USB to a linux computer,
ls or any GUI equivalent will show you two files, one called "123456789"
and one called "123456789.".  If you compare those two files, linux/vfat
will tell you that they are equal, because the vfat driver will wrongly
access twice the same "123456789" file and never the "123456789." file.

-rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789
-rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789.

If you then decide to remove the "123456789." file (the one with the
trailing dot), because it is an useless copy of your "123456789" file,
linux/vfat will silently remove the "123456789" file.  Afterwards,
if you're lucky, you'll see ls complain :

	ls: cannot access a.: No such file or directory

, but some GUI interfaces won't say anything.

In the listing, ls will show :

-????????? ? ?    ?        ?                ? 123456789.
(The "123456789" does not appear anymore)

but again some GUI won't even show you the name of the "123456789." file

With my proposed patch, at least if getdents tells a user program that
there is a "123456789." entry, further references by open and friends
will access precisely that entry, not another one.  That at least needs to
be fixed, without any option that the user should give to mount.

Best regards

Philippe

Re: [PATCH vfat] IOMEGA network drive compatibility

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

> You can call that a hack, but it certainly is better than the current situation.
> IOMEGA network drive firmware allows to create two files in the same directory
> differing only by the trailing dots.  Suppose you have in the same directory
> a file called "123456789" and a file called "123456789." (same name + a
> trailing dot).  When afterwards connected via USB to a linux computer,
> ls or any GUI equivalent will show you two files, one called "123456789"
> and one called "123456789.".  If you compare those two files, linux/vfat
> will tell you that they are equal, because the vfat driver will wrongly
> access twice the same "123456789" file and never the "123456789." file.
>
> -rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789
> -rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789.
>
> If you then decide to remove the "123456789." file (the one with the
> trailing dot), because it is an useless copy of your "123456789" file,
> linux/vfat will silently remove the "123456789" file.  Afterwards,
> if you're lucky, you'll see ls complain :
>
> 	ls: cannot access a.: No such file or directory
>
> , but some GUI interfaces won't say anything.
>
> In the listing, ls will show :
>
> -????????? ? ?    ?        ?                ? 123456789.
> (The "123456789" does not appear anymore)
>
> but again some GUI won't even show you the name of the "123456789." file

Because it's wrong entry as fatfs. In this situation, user needs to
repair by tools (e.g. fsck), so it's not responsibility of fs driver.

> With my proposed patch, at least if getdents tells a user program that
> there is a "123456789." entry, further references by open and friends
> will access precisely that entry, not another one.  That at least needs to
> be fixed, without any option that the user should give to mount.

Now, you try to change that design. So, you need to change/think more
high level consistency/design, not just workaround. Otherwise, it'll be
unmaintainable/unfixable.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH vfat] IOMEGA network drive compatibility

[Philippe De Muyter]

On Sun, Mar 14, 2010 at 08:17:30PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> > You can call that a hack, but it certainly is better than the current situation.
> > IOMEGA network drive firmware allows to create two files in the same directory
> > differing only by the trailing dots.  Suppose you have in the same directory
> > a file called "123456789" and a file called "123456789." (same name + a
> > trailing dot).  When afterwards connected via USB to a linux computer,
> > ls or any GUI equivalent will show you two files, one called "123456789"
> > and one called "123456789.".  If you compare those two files, linux/vfat
> > will tell you that they are equal, because the vfat driver will wrongly
> > access twice the same "123456789" file and never the "123456789." file.
> >
> > -rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789
> > -rwxr-xr-x 1 phdm root     1 2009-10-15 23:44 123456789.
> >
> > If you then decide to remove the "123456789." file (the one with the
> > trailing dot), because it is an useless copy of your "123456789" file,
> > linux/vfat will silently remove the "123456789" file.  Afterwards,
> > if you're lucky, you'll see ls complain :
> >
> > 	ls: cannot access a.: No such file or directory
wrong editing, of course it was :
	ls: cannot access 123456789.: No such file or directory
> >
> > , but some GUI interfaces won't say anything.
> >
> > In the listing, ls will show :
> >
> > -????????? ? ?    ?        ?                ? 123456789.
> > (The "123456789" does not appear anymore)
> >
> > but again some GUI won't even show you the name of the "123456789." file
> 
> Because it's wrong entry as fatfs. In this situation, user needs to
> repair by tools (e.g. fsck), so it's not responsibility of fs driver.

I know that entries with trailing dots are "invalid" fatfs entries,
but 1) linux/vfat currently accept such entries silently without truncating
them and 2) IOMEGA disks present themselves as fatfs disks.  Who are we to
tell the opposite ?  And IOMEGA integrated scandisk (accessible via the
web interface) does not find any error in its filesystem.

> 
> > With my proposed patch, at least if getdents tells a user program that
> > there is a "123456789." entry, further references by open and friends
> > will access precisely that entry, not another one.  That at least needs to
> > be fixed, without any option that the user should give to mount.
> 
> Now, you try to change that design. So, you need to change/think more
> high level consistency/design, not just workaround. Otherwise, it'll be
> unmaintainable/unfixable.

I am sorry but I don't see anything unmaintainable/unfixable in my patch :
it is very small and localized and it only allows accessing existing
entries with trailing dots, nothing more.
It does not allow to create "invalid" entries, so it does not change the
behaviour for "strict" vfat disks, it only introduces a small time penalty
when a user tries to access a file using a different name that the one stored
in the directory, which happens very rarely : most linux users use GUI's or
bash tab-completion to access files, which implies they use the name given by
the file-system driver, not a variation of it.

Now if we are concerned about "editing" such files by creating a temporary
file and renaming, we can look at what is needed to make renaming to an
existing name containing trailing dots succeed.  I have not tested that yet.

For the rest, i.e. creating arbitrary names with trailing dots just like
what IOMEGA drives do, it can be with a mount option, or left as it is
now : impossible.

Philippe

Re: [PATCH vfat] IOMEGA network drive compatibility

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> Because it's wrong entry as fatfs. In this situation, user needs to
>> repair by tools (e.g. fsck), so it's not responsibility of fs driver.
>
> I know that entries with trailing dots are "invalid" fatfs entries,
> but 1) linux/vfat currently accept such entries silently without truncating
> them and 2) IOMEGA disks present themselves as fatfs disks.  Who are we to
> tell the opposite ?  And IOMEGA integrated scandisk (accessible via the
> web interface) does not find any error in its filesystem.

You can create many files points to one cluster chain. It is silently
accepted by current fatfs driver. But, it's a broken fs image like this.

>> Now, you try to change that design. So, you need to change/think more
>> high level consistency/design, not just workaround. Otherwise, it'll be
>> unmaintainable/unfixable.
>
> I am sorry but I don't see anything unmaintainable/unfixable in my patch :
> it is very small and localized and it only allows accessing existing
> entries with trailing dots, nothing more.
>
> It does not allow to create "invalid" entries, so it does not change the
> behaviour for "strict" vfat disks, it only introduces a small time penalty
> when a user tries to access a file using a different name that the one stored
> in the directory, which happens very rarely : most linux users use GUI's or
> bash tab-completion to access files, which implies they use the name given by
> the file-system driver, not a variation of it.
>
> Now if we are concerned about "editing" such files by creating a temporary
> file and renaming, we can look at what is needed to make renaming to an
> existing name containing trailing dots succeed.  I have not tested that yet.
>
> For the rest, i.e. creating arbitrary names with trailing dots just like
> what IOMEGA drives do, it can be with a mount option, or left as it is
> now : impossible.

I already reported the one of bugs at least, and it seems you already
can't fix it, or just ignored it.

You are always saying "I'm fine with this". But I disagree, and I know
this fs is not only for just desktop users.  If this is only for you
usage, I'm sorry, but please apply it yourself.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCH] parport netmos 9845 & 9855 1P4S fixes

[Philippe De Muyter]

Hello list,

--

netmos serial/parallel adapters come in different flavour differing only
by the number of parallel and serial ports, which are encoded in the
subdevice ID.

Last fix of Christian Pellegrin for 9855 2P2S broke support for 9855 1P4S,
and works only by side-effect for the first parallel port of a 2P2S,
as it is found by reading the second addr entry of
(struct parport_pc_pci) cards[netmos_9855], which is not initialized.

netmos_9xx5_combo entry in (struct parport_pc_pci) cards[], which is used
for a 9845 1P4S should also be fixed when there are 4 serial ports
because it currently gives 2 as BAR index for the parallel port,
while, in this case, BAR 2 is the 3rd serial port and the parallel port
is at BAR 4.

I fixed 9845 1P4S and 9855 1P4S support, while preserving 9855 2P2S support,
by creating and using a netmos_9855_2p entry for 9855 boards with 2 parallel
ports for 9855 2P2S and 9855 2P0S boards,
and by allowing netmos_parallel_init to change not only the number of
parallel ports (0 or 1), but also the BAR index of the parallel port
when the serial ports are before the parallel port.

PS: the netmos_9855_2p entry in
(struct pciserial_board) pci_parport_serial_boards[] is needed because
netmos_parallel_init has no clean way to replace FL_BASE2 by FL_BASE4
based on the number of parallel ports in the card.

Tested with a 9845 1P4S and a 9855 1P4S board.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -30,6 +30,7 @@ enum parport_pc_pci_cards {
 	titan_210l,
 	netmos_9xx5_combo,
 	netmos_9855,
+	netmos_9855_2p,
 	avlab_1s1p,
 	avlab_1s2p,
 	avlab_2s1p,
@@ -62,7 +63,7 @@ struct parport_pc_pci {
 				struct parport_pc_pci *card, int failed);
 };
 
-static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *card, int autoirq, int autodma)
+static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *par, int autoirq, int autodma)
 {
 	/* the rule described below doesn't hold for this device */
 	if (dev->device == PCI_DEVICE_ID_NETMOS_9835 &&
@@ -74,9 +75,17 @@ static int __devinit netmos_parallel_ini
 	 * and serial ports.  The form is 0x00PS, where <P> is the number of
 	 * parallel ports and <S> is the number of serial ports.
 	 */
-	card->numports = (dev->subsystem_device & 0xf0) >> 4;
-	if (card->numports > ARRAY_SIZE(card->addr))
-		card->numports = ARRAY_SIZE(card->addr);
+	par->numports = (dev->subsystem_device & 0xf0) >> 4;
+	if (par->numports > ARRAY_SIZE(par->addr))
+		par->numports = ARRAY_SIZE(par->addr);
+	/*
+	 * This function is currently only called for cards with up to
+	 * one parallel port.
+	 * Parallel port BAR is either before or after serial ports BARS;
+	 * hence, lo should be either 0 or equal to the number of serial ports.
+	 */
+	if (par->addr[0].lo != 0)
+		par->addr[0].lo = dev->subsystem_device & 0xf;
 	return 0;
 }
 
@@ -84,7 +93,8 @@ static struct parport_pc_pci cards[] __d
 	/* titan_110l */		{ 1, { { 3, -1 }, } },
 	/* titan_210l */		{ 1, { { 3, -1 }, } },
 	/* netmos_9xx5_combo */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
-	/* netmos_9855 */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
+	/* netmos_9855 */		{ 1, { { 0, -1 }, }, netmos_parallel_init },
+	/* netmos_9855_2p */		{ 2, { { 0, -1 }, { 2, -1 }, } },
 	/* avlab_1s1p     */		{ 1, { { 1, 2}, } },
 	/* avlab_1s2p     */		{ 2, { { 1, 2}, { 3, 4 },} },
 	/* avlab_2s1p     */		{ 1, { { 2, 3}, } },
@@ -109,6 +119,10 @@ static struct pci_device_id parport_seri
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9845,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0020, 0, 0, netmos_9855_2p },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0022, 0, 0, netmos_9855_2p },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9855 },
 	/* PCI_VENDOR_ID_AVLAB/Intek21 has another bunch of cards ...*/
@@ -192,6 +206,12 @@ static struct pciserial_board pci_parpor
 		.uart_offset	= 8,
 	},
 	[netmos_9855] = {
+		.flags		= FL_BASE2 | FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 115200,
+		.uart_offset	= 8,
+	},
+	[netmos_9855_2p] = {
 		.flags		= FL_BASE4 | FL_BASE_BARS,
 		.num_ports	= 1,
 		.base_baud	= 115200,

Re: [PATCH] parport netmos 9845 & 9855 1P4S fixes

[Philippe De Muyter]

Hello list,

Sorry for the resend.  I had forgotten to cc Christian Pellegrin,
and have rephrased somewhat the comment.

Christian, can you verify that the patch below works for your
netmos 9855 2P2S board ?

Philippe

--

netmos serial/parallel adapters come in different flavour differing only
by the number of parallel and serial ports, which are encoded in the
subdevice ID.

Last fix of Christian Pellegrin for 9855 2P2S broke support for 9855 1P4S,
and works only by side-effect for the first parallel port of a 2P2S,
as this first parallel port is found by reading the second addr entry of
(struct parport_pc_pci) cards[netmos_9855], which is not initialized, and
hence has value 0, which happens to be the BAR of the first parallel port.

netmos_9xx5_combo entry in (struct parport_pc_pci) cards[], which is used
for a 9845 1P4S must also be fixed for the parallel port support when there
are 4 serial ports because this entry currently gives 2 as BAR index for
the parallel port.  Actually, in this case, BAR 2 is the 3rd serial port
while the parallel port is at BAR 4.

I fixed 9845 1P4S and 9855 1P4S support, while preserving 9855 2P2S support,
- by creating a netmos_9855_2p entry and using it for 9855 boards with
2 parallel ports : 9855 2P2S and 9855 2P0S boards,
- and by allowing netmos_parallel_init to change not only the number of
parallel ports (0 or 1), but making it also change the BAR index of the
parallel port when the serial ports are before the parallel port.

PS: the netmos_9855_2p entry in
(struct pciserial_board) pci_parport_serial_boards[] is needed because
netmos_parallel_init has no clean way to replace FL_BASE2 by FL_BASE4
in the description of the serial ports in function of the number of
parallel ports on the card.

Tested with a 9845 1P4S and a 9855 1P4S board.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -30,6 +30,7 @@ enum parport_pc_pci_cards {
 	titan_210l,
 	netmos_9xx5_combo,
 	netmos_9855,
+	netmos_9855_2p,
 	avlab_1s1p,
 	avlab_1s2p,
 	avlab_2s1p,
@@ -62,7 +63,7 @@ struct parport_pc_pci {
 				struct parport_pc_pci *card, int failed);
 };
 
-static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *card, int autoirq, int autodma)
+static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *par, int autoirq, int autodma)
 {
 	/* the rule described below doesn't hold for this device */
 	if (dev->device == PCI_DEVICE_ID_NETMOS_9835 &&
@@ -74,9 +75,17 @@ static int __devinit netmos_parallel_ini
 	 * and serial ports.  The form is 0x00PS, where <P> is the number of
 	 * parallel ports and <S> is the number of serial ports.
 	 */
-	card->numports = (dev->subsystem_device & 0xf0) >> 4;
-	if (card->numports > ARRAY_SIZE(card->addr))
-		card->numports = ARRAY_SIZE(card->addr);
+	par->numports = (dev->subsystem_device & 0xf0) >> 4;
+	if (par->numports > ARRAY_SIZE(par->addr))
+		par->numports = ARRAY_SIZE(par->addr);
+	/*
+	 * This function is currently only called for cards with up to
+	 * one parallel port.
+	 * Parallel port BAR is either before or after serial ports BARS;
+	 * hence, lo should be either 0 or equal to the number of serial ports.
+	 */
+	if (par->addr[0].lo != 0)
+		par->addr[0].lo = dev->subsystem_device & 0xf;
 	return 0;
 }
 
@@ -84,7 +93,8 @@ static struct parport_pc_pci cards[] __d
 	/* titan_110l */		{ 1, { { 3, -1 }, } },
 	/* titan_210l */		{ 1, { { 3, -1 }, } },
 	/* netmos_9xx5_combo */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
-	/* netmos_9855 */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
+	/* netmos_9855 */		{ 1, { { 0, -1 }, }, netmos_parallel_init },
+	/* netmos_9855_2p */		{ 2, { { 0, -1 }, { 2, -1 }, } },
 	/* avlab_1s1p     */		{ 1, { { 1, 2}, } },
 	/* avlab_1s2p     */		{ 2, { { 1, 2}, { 3, 4 },} },
 	/* avlab_2s1p     */		{ 1, { { 2, 3}, } },
@@ -109,6 +119,10 @@ static struct pci_device_id parport_seri
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9845,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0020, 0, 0, netmos_9855_2p },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0022, 0, 0, netmos_9855_2p },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9855 },
 	/* PCI_VENDOR_ID_AVLAB/Intek21 has another bunch of cards ...*/
@@ -192,6 +206,12 @@ static struct pciserial_board pci_parpor
 		.uart_offset	= 8,
 	},
 	[netmos_9855] = {
+		.flags		= FL_BASE2 | FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 115200,
+		.uart_offset	= 8,
+	},
+	[netmos_9855_2p] = {
 		.flags		= FL_BASE4 | FL_BASE_BARS,
 		.num_ports	= 1,
 		.base_baud	= 115200,

[PATCH] parport netmos 9845 & 9855 1P4S fixes

[Philippe De Muyter]

Hello list,

The same patch, with an additional Tested-by, after the positive feedback
of Christian Pellegrin.

Philippe

--

netmos serial/parallel adapters come in different flavour differing only
by the number of parallel and serial ports, which are encoded in the
subdevice ID.

Last fix of Christian Pellegrin for 9855 2P2S broke support for 9855 1P4S,
and works only by side-effect for the first parallel port of a 2P2S,
as this first parallel port is found by reading the second addr entry of
(struct parport_pc_pci) cards[netmos_9855], which is not initialized, and
hence has value 0, which happens to be the BAR of the first parallel port.

netmos_9xx5_combo entry in (struct parport_pc_pci) cards[], which is used
for a 9845 1P4S must also be fixed for the parallel port support when there
are 4 serial ports because this entry currently gives 2 as BAR index for
the parallel port.  Actually, in this case, BAR 2 is the 3rd serial port
while the parallel port is at BAR 4.

I fixed 9845 1P4S and 9855 1P4S support, while preserving 9855 2P2S support,
- by creating a netmos_9855_2p entry and using it for 9855 boards with
2 parallel ports : 9855 2P2S and 9855 2P0S boards,
- and by allowing netmos_parallel_init to change not only the number of
parallel ports (0 or 1), but making it also change the BAR index of the
parallel port when the serial ports are before the parallel port.

PS: the netmos_9855_2p entry in
(struct pciserial_board) pci_parport_serial_boards[] is needed because
netmos_parallel_init has no clean way to replace FL_BASE2 by FL_BASE4
in the description of the serial ports in function of the number of
parallel ports on the card.

Tested with 9845 1P4S, 9855 1P4S and 9855 2P2S boards.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>
Tested-by: Christian Pellegrin <chripell@fsfe.org>

diff --git a/drivers/parport/parport_serial.c b/drivers/parport/parport_serial.c
--- a/drivers/parport/parport_serial.c
+++ b/drivers/parport/parport_serial.c
@@ -30,6 +30,7 @@ enum parport_pc_pci_cards {
 	titan_210l,
 	netmos_9xx5_combo,
 	netmos_9855,
+	netmos_9855_2p,
 	avlab_1s1p,
 	avlab_1s2p,
 	avlab_2s1p,
@@ -62,7 +63,7 @@ struct parport_pc_pci {
 				struct parport_pc_pci *card, int failed);
 };
 
-static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *card, int autoirq, int autodma)
+static int __devinit netmos_parallel_init(struct pci_dev *dev, struct parport_pc_pci *par, int autoirq, int autodma)
 {
 	/* the rule described below doesn't hold for this device */
 	if (dev->device == PCI_DEVICE_ID_NETMOS_9835 &&
@@ -74,9 +75,17 @@ static int __devinit netmos_parallel_ini
 	 * and serial ports.  The form is 0x00PS, where <P> is the number of
 	 * parallel ports and <S> is the number of serial ports.
 	 */
-	card->numports = (dev->subsystem_device & 0xf0) >> 4;
-	if (card->numports > ARRAY_SIZE(card->addr))
-		card->numports = ARRAY_SIZE(card->addr);
+	par->numports = (dev->subsystem_device & 0xf0) >> 4;
+	if (par->numports > ARRAY_SIZE(par->addr))
+		par->numports = ARRAY_SIZE(par->addr);
+	/*
+	 * This function is currently only called for cards with up to
+	 * one parallel port.
+	 * Parallel port BAR is either before or after serial ports BARS;
+	 * hence, lo should be either 0 or equal to the number of serial ports.
+	 */
+	if (par->addr[0].lo != 0)
+		par->addr[0].lo = dev->subsystem_device & 0xf;
 	return 0;
 }
 
@@ -84,7 +93,8 @@ static struct parport_pc_pci cards[] __d
 	/* titan_110l */		{ 1, { { 3, -1 }, } },
 	/* titan_210l */		{ 1, { { 3, -1 }, } },
 	/* netmos_9xx5_combo */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
-	/* netmos_9855 */		{ 1, { { 2, -1 }, }, netmos_parallel_init },
+	/* netmos_9855 */		{ 1, { { 0, -1 }, }, netmos_parallel_init },
+	/* netmos_9855_2p */		{ 2, { { 0, -1 }, { 2, -1 }, } },
 	/* avlab_1s1p     */		{ 1, { { 1, 2}, } },
 	/* avlab_1s2p     */		{ 2, { { 1, 2}, { 3, 4 },} },
 	/* avlab_2s1p     */		{ 1, { { 2, 3}, } },
@@ -109,6 +119,10 @@ static struct pci_device_id parport_seri
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9845,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9xx5_combo },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0020, 0, 0, netmos_9855_2p },
+	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
+	  0x1000, 0x0022, 0, 0, netmos_9855_2p },
 	{ PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9855,
 	  PCI_ANY_ID, PCI_ANY_ID, 0, 0, netmos_9855 },
 	/* PCI_VENDOR_ID_AVLAB/Intek21 has another bunch of cards ...*/
@@ -192,6 +206,12 @@ static struct pciserial_board pci_parpor
 		.uart_offset	= 8,
 	},
 	[netmos_9855] = {
+		.flags		= FL_BASE2 | FL_BASE_BARS,
+		.num_ports	= 1,
+		.base_baud	= 115200,
+		.uart_offset	= 8,
+	},
+	[netmos_9855_2p] = {
 		.flags		= FL_BASE4 | FL_BASE_BARS,
 		.num_ports	= 1,
 		.base_baud	= 115200,

-- 
Philippe De Muyter  phdm at macqel dot be  Tel +32 27029044
Macq Electronique SA  rue de l'Aeronef 2  B-1140 Bruxelles  Fax +32 27029077

[PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[Philippe De Muyter]

Hello vfat guru's

I have an IOMEGA home network hard disk that I can connect either by
ethernet (ftp or cifs) or by USB. because of the ftp server mode
firmware, I must keep it formatted as a vfat filesystem.

I have copied my music files on it using the ftp mode.  Using the same
ftp mode, I can also retrieve my music files without problem, and
when I list them, they have the exact names that they had on my linux
ext3 partition.

When I connected this disk via USB, now relying on the vfat module
of linux, there were some directories that I could not reread.  The
common factor of these directories names is that they end with one or
more dots, e.g.

	Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

If I issue the `ls' or `find' command, I get this strange message :

find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory

Adding printk's in `fat_search_long' revealed that on this disk, the
file/directory NAMES ENDING WITH DOTS ARE STORED WITH THEIR TRAILING DOTS.

Here is a patch squetch that make accessing my
 Simon_&_Garfunkel-Wednesday_Morning,_3_a.m. directory possible, but I
don't know if storing long filenames ending with dot's should not also
be fixed.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>

diff -r f2c5827a8d44 fs/fat/namei_vfat.c
--- a/fs/fat/namei_vfat.c	Mon Aug 31 17:44:05 2009 -1000
+++ b/fs/fat/namei_vfat.c	Fri Sep 25 21:30:36 2009 +0200
@@ -702,10 +702,7 @@
 static int vfat_find(struct inode *dir, struct qstr *qname,
 		     struct fat_slot_info *sinfo)
 {
-	unsigned int len = vfat_striptail_len(qname);
-	if (len == 0)
-		return -ENOENT;
-	return fat_search_long(dir, qname->name, len, sinfo);
+	return fat_search_long(dir, qname->name, qname->len, sinfo);
 }
 
 static struct dentry *vfat_lookup(struct inode *dir, struct dentry *dentry,

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

> I have an IOMEGA home network hard disk that I can connect either by
> ethernet (ftp or cifs) or by USB. because of the ftp server mode
> firmware, I must keep it formatted as a vfat filesystem.
>
> I have copied my music files on it using the ftp mode.  Using the same
> ftp mode, I can also retrieve my music files without problem, and
> when I list them, they have the exact names that they had on my linux
> ext3 partition.
>
> When I connected this disk via USB, now relying on the vfat module
> of linux, there were some directories that I could not reread.  The
> common factor of these directories names is that they end with one or
> more dots, e.g.
>
> 	Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.
>
> If I issue the `ls' or `find' command, I get this strange message :
>
> find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory
>
> Adding printk's in `fat_search_long' revealed that on this disk, the
> file/directory NAMES ENDING WITH DOTS ARE STORED WITH THEIR TRAILING DOTS.
>
> Here is a patch squetch that make accessing my
>  Simon_&_Garfunkel-Wednesday_Morning,_3_a.m. directory possible, but I
> don't know if storing long filenames ending with dot's should not also
> be fixed.

IIRC, Windows stripped the trailing ".". That filename can be read/used on
Windows?

If it can be read, I think we should consider to remove
vfat_striptail_len() (at least for lookup path), more investigation
would be needed though.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[Philippe De Muyter]

Hi Ogawa,

On Tue, Sep 29, 2009 at 07:05:27PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> > I have an IOMEGA home network hard disk that I can connect either by
> > ethernet (ftp or cifs) or by USB. Because of the ftp server mode
> > firmware, I must keep it formatted as a vfat filesystem.
> >
> > I have copied my music files on it using the ftp mode.  Using the same
> > ftp mode, I can also retrieve my music files without problem, and
> > when I list them, they have the exact names that they had on my linux
> > ext3 partition.
> >
> > When I connected this disk via USB, now relying on the vfat module
> > of linux, there were some directories that I could not reread.  The
> > common factor of these directories names is that they end with one or
> > more dots, e.g.
> >
> > 	Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.
> >
> > If I issue the `ls' or `find' command, I get this strange message :
> >
> > find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory
> >
> > Adding printk's in `fat_search_long' revealed that on this disk, the
> > file/directory NAMES ENDING WITH DOTS ARE STORED WITH THEIR TRAILING DOTS.
> >
> > Here is a patch sketch that makes accessing my
> >  Simon_&_Garfunkel-Wednesday_Morning,_3_a.m. directory possible, but I
> > don't know if storing long filenames ending with dot's should not also
> > be fixed.
> 
> IIRC, Windows stripped the trailing ".". That filename can be read/used on
> Windows?

Sorry I don't use MS-Windows.  I'll try to find someone who does.

But, risking repeating myself, the firmware of the IOMEGA disk (via
the ftp interface) presents the filenames with the trailing dots.

> If it can be read, I think we should consider to remove
> vfat_striptail_len() (at least for lookup path), more investigation
> would be needed though.

That's what my patch does actually.  Maybe it should depend of the length
of the filename, though.

Thanks

Philippe

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[Philippe De Muyter]

Hi Ogawa,
On Tue, Sep 29, 2009 at 12:25:59PM +0200, I wrote:
> Hi Ogawa,
> 
> On Tue, Sep 29, 2009 at 07:05:27PM +0900, OGAWA Hirofumi wrote:
> > Philippe De Muyter <phdm@macqel.be> writes:
> > 
> > > I have an IOMEGA home network hard disk that I can connect either by
> > > ethernet (ftp or cifs) or by USB. Because of the ftp server mode
> > > firmware, I must keep it formatted as a vfat filesystem.
> > >
> > > I have copied my music files on it using the ftp mode.  Using the same
> > > ftp mode, I can also retrieve my music files without problem, and
> > > when I list them, they have the exact names that they had on my linux
> > > ext3 partition.
> > >
> > > When I connected this disk via USB, now relying on the vfat module
> > > of linux, there were some directories that I could not reread.  The
> > > common factor of these directories names is that they end with one or
> > > more dots, e.g.
> > >
> > > 	Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.
> > >
> > > If I issue the `ls' or `find' command, I get this strange message :
> > >
> > > find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory
> > >
> > > Adding printk's in `fat_search_long' revealed that on this disk, the
> > > file/directory NAMES ENDING WITH DOTS ARE STORED WITH THEIR TRAILING DOTS.
> > >
> > > Here is a patch sketch that makes accessing my
> > >  Simon_&_Garfunkel-Wednesday_Morning,_3_a.m. directory possible, but I
> > > don't know if storing long filenames ending with dot's should not also
> > > be fixed.
> > 
> > IIRC, Windows stripped the trailing ".". That filename can be read/used on
> > Windows?
> 
> Sorry I don't use MS-Windows.  I'll try to find someone who does.

MS-Windows (XP SP3) lists correctly the names with their trailing dots.

Philippe

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

>> > IIRC, Windows stripped the trailing ".". That filename can be read/used on
>> > Windows?
>> 
>> Sorry I don't use MS-Windows.  I'll try to find someone who does.
>
> MS-Windows (XP SP3) lists correctly the names with their trailing dots.

Ok. Is it meaning you could read the file correctly? I guess so, and
I'll review deeply about dcache issues for it.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[Philippe De Muyter]

On Wed, Sep 30, 2009 at 08:02:01PM +0900, OGAWA Hirofumi wrote:
> Philippe De Muyter <phdm@macqel.be> writes:
> 
> >> > IIRC, Windows stripped the trailing ".". That filename can be read/used on
> >> > Windows?
> >> 
> >> Sorry I don't use MS-Windows.  I'll try to find someone who does.
> >
> > MS-Windows (XP SP3) lists correctly the names with their trailing dots.
> 
> Ok. Is it meaning you could read the file correctly? I guess so, and
> I'll review deeply about dcache issues for it.

Sorry. I did not test enough yesterday. It was late  :/

on MS-Windows XP SP#, the files and directories are listed with their
real name, including the trailing dots, but if one tries to open such a
file or go into such a directory, that fails with an error message.

With a "123456789." file :
Cannot find the E:\PUBLIC\trailing_dots\123456789. file.
Do you want to create a new file?
		Yes		No		Cancel

With a "directoryname." directory :
E\PUBLIC\trailing_dots\directoryname. refers to a location that is unavailable.
It could be on a hard drive on this computer, or on a network. Check to make
sure that the disk is properly inserted, or that you are connected to the
Internet or your network, and then try again.  If it still cannot be located,
the information might have been moved to a different location.
				OK

I don't think we want linux to remain compatible with this bug.  So perhaps
not removing the trailing dots for a lookup is all what is needed.

Best regards

Philippe

Re: [PATCH RFC] vfat and Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.

[OGAWA Hirofumi]

Philippe De Muyter <phdm@macqel.be> writes:

> Sorry. I did not test enough yesterday. It was late  :/
>
> on MS-Windows XP SP#, the files and directories are listed with their
> real name, including the trailing dots, but if one tries to open such a
> file or go into such a directory, that fails with an error message.
>
> With a "123456789." file :
> Cannot find the E:\PUBLIC\trailing_dots\123456789. file.
> Do you want to create a new file?
> 		Yes		No		Cancel
>
> With a "directoryname." directory :
> E\PUBLIC\trailing_dots\directoryname. refers to a location that is unavailable.
> It could be on a hard drive on this computer, or on a network. Check to make
> sure that the disk is properly inserted, or that you are connected to the
> Internet or your network, and then try again.  If it still cannot be located,
> the information might have been moved to a different location.
> 				OK
>
> I don't think we want linux to remain compatible with this bug.  So perhaps
> not removing the trailing dots for a lookup is all what is needed.

I think to make that filename readable is not crazy, however it would be
the part of design.

And I found one issue for now, fs has to check existant file to make new
file. But if lookup and creation was using different name, the check can
not be done with that patch correctly.

Thanks.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCH vfat] allow retrieving entries with trailing dots

[Philippe De Muyter]

This fixes accessing vfat entries with trailing dots created by an external
vfat driver (like the one in IOMEGA home network hard drives)

Philippe

--

Some vfat-formatted network disks that are also usb disk do not discard
trailing dots when creating files or directories via ftp.
When connected via usb to a linux machine, that leads to this problem :
if one issues the `ls' or `find' command, one gets this message :

find: ./Simon_&_Garfunkel-Wednesday_Morning,_3_a.m.: No such file or directory

Fix that by first trying to retrieve the entry with the full name, and only if
that fails and there are trailing dots in the searched name, try then to find
the truncated name.

Signed-off-by: Philippe De Muyter <phdm@macqel.be>

--- a/fs/fat/namei_vfat.c	2009-09-10 00:13:59.000000000 +0200
+++ b/fs/fat/namei_vfat.c	2010-02-08 02:28:37.010096903 +0100
@@ -702,9 +702,22 @@
 static int vfat_find(struct inode *dir, struct qstr *qname,
 		     struct fat_slot_info *sinfo)
 {
-	unsigned int len = vfat_striptail_len(qname);
+	int err;
+	unsigned int len;
+
+	/* Some combined ethernet + usb external hard drive do not
+	 * remove the trailing dots when creating entries in ethernet mode.
+	 * (e.g. Iomega Home Network Hard Drive)
+	 * Make accessing those entries possible.
+	 */
+	err = fat_search_long(dir, qname->name, qname->len, sinfo);
+	if (!err)
+		return err;
+	len = vfat_striptail_len(qname);
 	if (len == 0)
 		return -ENOENT;
+	if (len == qname->len)
+		return err;
 	return fat_search_long(dir, qname->name, len, sinfo);
 }