Re: [PATCH] Added CONFIG_VFAT_FS_DUALNAMES option

[Eric W. Biederman]

tridge@xxxxxxxxx writes:

> This is a new patch for VFAT long filename support, replacing the one
> that I posted last month. It retains a lot more functionality then the
> previous patch.
>
> A FAQ will be posted immediately after this patch to answer the
> questions that were raised from the previous discussion.
>
> Cheers, Tridge
>
>
> ------------
>
> When VFAT_FS_DUALNAMES is disabled we avoid the creation of 8.3 short
> filenames for files on VFAT filesystems that require a long name. The
> patch uses a pattern of 11 bytes in the directory entry which contains
> invalid characters such that it cannot be considered to be a valid short
> filename.
>
> Signed-off-by: Andrew Tridgell <tridge@xxxxxxxxx>
> Acked-by: Dave Kleikamp <shaggy@xxxxxxxxxxxxxxxxxx>
> Acked-by: Paul E. McKenney <paulmck@xxxxxxxxxxxxxxxxxx>
> ---
>  fs/fat/Kconfig      |   20 +++++++++++++++++
>  fs/fat/dir.c        |   15 ++++++-------
>  fs/fat/namei_vfat.c |   59 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/fs/fat/Kconfig b/fs/fat/Kconfig
> index 182f9ff..907a5de 100644
> --- a/fs/fat/Kconfig
> +++ b/fs/fat/Kconfig
> @@ -74,6 +74,26 @@ config VFAT_FS
>  	  To compile this as a module, choose M here: the module will be called
>  	  vfat.
>  
> +config VFAT_FS_DUALNAMES
> +	bool "VFAT dual names support"
> +	depends on VFAT_FS
> +	help
> +	  This option provides support for dual filenames on VFAT filesystems.
> +	  If this option is disabled then file creation will either put
> +	  a short (8.3) name or a long name on the file, but never both.
> +	  The field where a shortname would normally go is filled with
> +	  invalid characters such that it cannot be considered a valid
> +	  short filename.
> +
> +	  That means that long filenames created with this option
> +	  disabled will not be accessible at all to operating systems
> +	  that do not understand the VFAT extensions.
> +
> +	  Users considering enabling this option should consider the implications
> +	  of any patents that may exist on dual filenames in VFAT.
> +
> +	  If unsure, say N
> +
>  config FAT_DEFAULT_CODEPAGE
>  	int "Default codepage for FAT"
>  	depends on MSDOS_FS || VFAT_FS
> diff --git a/fs/fat/dir.c b/fs/fat/dir.c
> index 38ff75a..cd5d3ec 100644
> --- a/fs/fat/dir.c
> +++ b/fs/fat/dir.c
> @@ -420,14 +420,13 @@ parse_record:
>  			}
>  			i += chl;
>  		}
> -		if (!last_u)
> -			continue;
> -
> -		/* Compare shortname */
> -		bufuname[last_u] = 0x0000;
> -		len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> -		if (fat_name_match(sbi, name, name_len, bufname, len))
> -			goto found;
> +		if (last_u) {
> +			/* Compare shortname */
> +			bufuname[last_u] = 0x0000;
> +			len = fat_uni_to_x8(sbi, bufuname, bufname, sizeof(bufname));
> +			if (fat_name_match(sbi, name, name_len, bufname, len))
> +				goto found;
> +		}
>  
>  		if (nr_slots) {
>  			void *longname = unicode + FAT_MAX_UNI_CHARS;

This hunk allowing the examination of the long name if there is not a
short name is something else entirely.

You don't describe it in your summary, and don't make the case that
this is the correct behavior.

This should probably be split out into a separate patch if it is the
right thing to do, or drop it if (as it looks) there is no point in
that change.

Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/