[PATCH] fat: Refactor shortname parsing

[PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

Nearly identical shortname parsing is performed in fat_search_long()
and __fat_readdir(). Extract this code into a function that may be
called by both.

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
--- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
+++ new/fs/fat/dir.c	2012-06-29 13:08:53.020677428 -0500
@@ -35,6 +35,11 @@
 #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
 #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
 
+static inline unsigned char fat_tolower(int allowed, unsigned char c)
+{
+	return (allowed && (c >= 'A') && (c <= 'Z')) ? c+32 : c;
+}
+
 static inline loff_t fat_make_i_pos(struct super_block *sb,
 				    struct buffer_head *bh,
 				    struct msdos_dir_entry *de)
@@ -333,6 +338,106 @@ parse_long:
 	return 0;
 }
 
+/**
+ * fat_parse_short - Parse MS-DOS (short) directory entry.
+ * @sb:         superblock
+ * @de:         directory entry to parse
+ * @name:       FAT_MAX_SHORT_SIZE array in which to place extracted name
+ * @dot_hidden  Nonzero == prepend '.' to names with ATTR_HIDDEN
+ *
+ * Returns the number of characters extracted into 'name'.
+ */
+static int fat_parse_short(struct super_block *sb,
+			   const struct msdos_dir_entry *de,
+			   unsigned char *name, int dot_hidden)
+{
+	const struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	struct nls_table *nls_disk = sbi->nls_disk;
+	unsigned short opt_shortname = sbi->options.shortname;
+	int nocase = sbi->options.nocase;
+	wchar_t uni_name[14];
+	unsigned char c, work[MSDOS_NAME];
+	unsigned char *ptname = name;
+	int chi, chl, i, j, k;
+	int dotoffset = 0;
+	int name_len = 0, uni_len = 0;
+
+	if (dot_hidden && (de->attr & ATTR_HIDDEN)) {
+		*ptname++ = '.';
+		dotoffset = 1;
+	}
+
+	memcpy(work, de->name, sizeof(work));
+	/* see namei.c, msdos_format_name */
+	if (work[0] == 0x05)
+		work[0] = 0xE5;
+
+	/* Filename */
+	for (i = 0, j = 0; i < 8;) {
+		c = work[i];
+		if (!c)
+			break;
+		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
+					&uni_name[j++], opt_shortname,
+					de->lcase & CASE_LOWER_BASE);
+		if (chl <= 1) {
+			ptname[i++] = fat_tolower(!nocase, c);
+			if (c != ' ') {
+				name_len = i;
+				uni_len  = j;
+			}
+		} else {
+			uni_len = j;
+			for (chi = 0; chi < chl && i < 8; chi++) {
+				ptname[i] = work[i];
+				i++;
+				name_len = i;
+			}
+		}
+	}
+
+	i = name_len;
+	j = uni_len;
+	fat_short2uni(nls_disk, ".", 1, &uni_name[j++]);
+	ptname[i++] = '.';
+
+	/* Extension */
+	for (k = 8; k < MSDOS_NAME;) {
+		c = work[k];
+		if (!c)
+			break;
+		chl = fat_shortname2uni(nls_disk, &work[k], MSDOS_NAME - k,
+					&uni_name[j++], opt_shortname,
+					de->lcase & CASE_LOWER_EXT);
+		if (chl <= 1) {
+			k++;
+			ptname[i++] = fat_tolower(!nocase, c);
+			if (c != ' ') {
+				name_len = i;
+				uni_len  = j;
+			}
+		} else {
+			uni_len = j;
+			for (chi = 0; chi < chl && k < MSDOS_NAME; chi++) {
+				ptname[i++] = work[k++];
+				name_len = i;
+			}
+		}
+	}
+
+	if (name_len > 0) {
+		name_len += dotoffset;
+
+		if (sbi->options.isvfat) {
+			uni_name[uni_len] = 0x0000;
+			name_len = fat_uni_to_x8(sb, uni_name, name,
+						 FAT_MAX_SHORT_SIZE);
+		}
+	}
+
+	return name_len;
+}
+
 /*
  * Return values: negative -> error, 0 -> not found, positive -> found,
  * value is the total amount of slots, including the shortname entry.
@@ -344,15 +449,11 @@ int fat_search_long(struct inode *inode,
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	struct buffer_head *bh = NULL;
 	struct msdos_dir_entry *de;
-	struct nls_table *nls_disk = sbi->nls_disk;
 	unsigned char nr_slots;
-	wchar_t bufuname[14];
 	wchar_t *unicode = NULL;
-	unsigned char work[MSDOS_NAME];
 	unsigned char bufname[FAT_MAX_SHORT_SIZE];
-	unsigned short opt_shortname = sbi->options.shortname;
 	loff_t cpos = 0;
-	int chl, i, j, last_u, err, len;
+	int err, len;
 
 	err = -ENOENT;
 	while (1) {
@@ -380,47 +481,16 @@ parse_record:
 				goto end_of_dir;
 		}
 
-		memcpy(work, de->name, sizeof(de->name));
-		/* see namei.c, msdos_format_name */
-		if (work[0] == 0x05)
-			work[0] = 0xE5;
-		for (i = 0, j = 0, last_u = 0; i < 8;) {
-			if (!work[i])
-				break;
-			chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
-						&bufuname[j++], opt_shortname,
-						de->lcase & CASE_LOWER_BASE);
-			if (chl <= 1) {
-				if (work[i] != ' ')
-					last_u = j;
-			} else {
-				last_u = j;
-			}
-			i += chl;
-		}
-		j = last_u;
-		fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
-		for (i = 8; i < MSDOS_NAME;) {
-			if (!work[i])
-				break;
-			chl = fat_shortname2uni(nls_disk, &work[i],
-						MSDOS_NAME - i,
-						&bufuname[j++], opt_shortname,
-						de->lcase & CASE_LOWER_EXT);
-			if (chl <= 1) {
-				if (work[i] != ' ')
-					last_u = j;
-			} else {
-				last_u = j;
-			}
-			i += chl;
-		}
-		if (!last_u)
+		/* Never prepend '.' to hidden files here.
+		 * That is done only for msdos mounts (and only when
+		 * 'dotsOK=yes'); if we are executing here, it is in the
+		 * context of a vfat mount.
+		 */
+		len = fat_parse_short(sb, de, bufname, 0);
+		if (len == 0)
 			continue;
 
 		/* Compare shortname */
-		bufuname[last_u] = 0x0000;
-		len = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
 		if (fat_name_match(sbi, name, name_len, bufname, len))
 			goto found;
 
@@ -469,20 +539,15 @@ static int __fat_readdir(struct inode *i
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	struct buffer_head *bh;
 	struct msdos_dir_entry *de;
-	struct nls_table *nls_disk = sbi->nls_disk;
 	unsigned char nr_slots;
-	wchar_t bufuname[14];
 	wchar_t *unicode = NULL;
-	unsigned char c, work[MSDOS_NAME];
-	unsigned char bufname[FAT_MAX_SHORT_SIZE], *ptname = bufname;
-	unsigned short opt_shortname = sbi->options.shortname;
+	unsigned char bufname[FAT_MAX_SHORT_SIZE];
 	int isvfat = sbi->options.isvfat;
-	int nocase = sbi->options.nocase;
 	const char *fill_name = NULL;
 	unsigned long inum;
 	unsigned long lpos, dummy, *furrfu = &lpos;
 	loff_t cpos;
-	int chi, chl, i, i2, j, last, last_u, dotoffset = 0, fill_len = 0;
+	int short_len = 0, fill_len = 0;
 	int ret = 0;
 
 	lock_super(sb);
@@ -556,74 +621,10 @@ parse_record:
 		}
 	}
 
-	if (sbi->options.dotsOK) {
-		ptname = bufname;
-		dotoffset = 0;
-		if (de->attr & ATTR_HIDDEN) {
-			*ptname++ = '.';
-			dotoffset = 1;
-		}
-	}
-
-	memcpy(work, de->name, sizeof(de->name));
-	/* see namei.c, msdos_format_name */
-	if (work[0] == 0x05)
-		work[0] = 0xE5;
-	for (i = 0, j = 0, last = 0, last_u = 0; i < 8;) {
-		if (!(c = work[i]))
-			break;
-		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
-					&bufuname[j++], opt_shortname,
-					de->lcase & CASE_LOWER_BASE);
-		if (chl <= 1) {
-			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
-			if (c != ' ') {
-				last = i;
-				last_u = j;
-			}
-		} else {
-			last_u = j;
-			for (chi = 0; chi < chl && i < 8; chi++) {
-				ptname[i] = work[i];
-				i++; last = i;
-			}
-		}
-	}
-	i = last;
-	j = last_u;
-	fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
-	ptname[i++] = '.';
-	for (i2 = 8; i2 < MSDOS_NAME;) {
-		if (!(c = work[i2]))
-			break;
-		chl = fat_shortname2uni(nls_disk, &work[i2], MSDOS_NAME - i2,
-					&bufuname[j++], opt_shortname,
-					de->lcase & CASE_LOWER_EXT);
-		if (chl <= 1) {
-			i2++;
-			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
-			if (c != ' ') {
-				last = i;
-				last_u = j;
-			}
-		} else {
-			last_u = j;
-			for (chi = 0; chi < chl && i2 < MSDOS_NAME; chi++) {
-				ptname[i++] = work[i2++];
-				last = i;
-			}
-		}
-	}
-	if (!last)
+	short_len = fat_parse_short(sb, de, bufname, sbi->options.dotsOK);
+	if (short_len == 0)
 		goto record_end;
 
-	i = last + dotoffset;
-	j = last_u;
-
-	if (isvfat) {
-		bufuname[j] = 0x0000;
-		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
-	}
 	if (nr_slots) {
 		/* hack for fat_ioctl_filldir() */
 		struct fat_ioctl_filldir_callback *p = dirent;
@@ -631,12 +632,12 @@ parse_record:
 		p->longname = fill_name;
 		p->long_len = fill_len;
 		p->shortname = bufname;
-		p->short_len = i;
+		p->short_len = short_len;
 		fill_name = NULL;
 		fill_len = 0;
 	} else {
 		fill_name = bufname;
-		fill_len = i;
+		fill_len = short_len;
 	}
 
 start_filldir:

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> Nearly identical shortname parsing is performed in fat_search_long()
> and __fat_readdir(). Extract this code into a function that may be
> called by both.

What is the difference, and why changes was needed?

> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
> ---
> diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
> --- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
> +++ new/fs/fat/dir.c	2012-06-29 13:08:53.020677428 -0500
> @@ -35,6 +35,11 @@
>  #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
>  #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
>  
> +static inline unsigned char fat_tolower(int allowed, unsigned char c)
> +{
> +	return (allowed && (c >= 'A') && (c <= 'Z')) ? c+32 : c;
> +}
> +
>  static inline loff_t fat_make_i_pos(struct super_block *sb,
>  				    struct buffer_head *bh,
>  				    struct msdos_dir_entry *de)
> @@ -333,6 +338,106 @@ parse_long:
>  	return 0;
>  }
>  
> +/**
> + * fat_parse_short - Parse MS-DOS (short) directory entry.
> + * @sb:         superblock
> + * @de:         directory entry to parse
> + * @name:       FAT_MAX_SHORT_SIZE array in which to place extracted name
> + * @dot_hidden  Nonzero == prepend '.' to names with ATTR_HIDDEN
> + *
> + * Returns the number of characters extracted into 'name'.
> + */
> +static int fat_parse_short(struct super_block *sb,
> +			   const struct msdos_dir_entry *de,
> +			   unsigned char *name, int dot_hidden)
> +{
> +	const struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +	struct nls_table *nls_disk = sbi->nls_disk;
> +	unsigned short opt_shortname = sbi->options.shortname;
> +	int nocase = sbi->options.nocase;
> +	wchar_t uni_name[14];
> +	unsigned char c, work[MSDOS_NAME];
> +	unsigned char *ptname = name;
> +	int chi, chl, i, j, k;
> +	int dotoffset = 0;
> +	int name_len = 0, uni_len = 0;
> +
> +	if (dot_hidden && (de->attr & ATTR_HIDDEN)) {
> +		*ptname++ = '.';
> +		dotoffset = 1;
> +	}
> +
> +	memcpy(work, de->name, sizeof(work));
> +	/* see namei.c, msdos_format_name */
> +	if (work[0] == 0x05)
> +		work[0] = 0xE5;
> +
> +	/* Filename */
> +	for (i = 0, j = 0; i < 8;) {
> +		c = work[i];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_BASE);
> +		if (chl <= 1) {
> +			ptname[i++] = fat_tolower(!nocase, c);
> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			for (chi = 0; chi < chl && i < 8; chi++) {
> +				ptname[i] = work[i];
> +				i++;
> +				name_len = i;
> +			}
> +		}
> +	}
> +
> +	i = name_len;
> +	j = uni_len;
> +	fat_short2uni(nls_disk, ".", 1, &uni_name[j++]);
> +	ptname[i++] = '.';
> +
> +	/* Extension */
> +	for (k = 8; k < MSDOS_NAME;) {
> +		c = work[k];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[k], MSDOS_NAME - k,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_EXT);
> +		if (chl <= 1) {
> +			k++;
> +			ptname[i++] = fat_tolower(!nocase, c);
> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			for (chi = 0; chi < chl && k < MSDOS_NAME; chi++) {
> +				ptname[i++] = work[k++];
> +				name_len = i;
> +			}
> +		}
> +	}
> +
> +	if (name_len > 0) {
> +		name_len += dotoffset;
> +
> +		if (sbi->options.isvfat) {
> +			uni_name[uni_len] = 0x0000;
> +			name_len = fat_uni_to_x8(sb, uni_name, name,
> +						 FAT_MAX_SHORT_SIZE);
> +		}
> +	}
> +
> +	return name_len;
> +}
> +
>  /*
>   * Return values: negative -> error, 0 -> not found, positive -> found,
>   * value is the total amount of slots, including the shortname entry.
> @@ -344,15 +449,11 @@ int fat_search_long(struct inode *inode,
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh = NULL;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char work[MSDOS_NAME];
>  	unsigned char bufname[FAT_MAX_SHORT_SIZE];
> -	unsigned short opt_shortname = sbi->options.shortname;
>  	loff_t cpos = 0;
> -	int chl, i, j, last_u, err, len;
> +	int err, len;
>  
>  	err = -ENOENT;
>  	while (1) {
> @@ -380,47 +481,16 @@ parse_record:
>  				goto end_of_dir;
>  		}
>  
> -		memcpy(work, de->name, sizeof(de->name));
> -		/* see namei.c, msdos_format_name */
> -		if (work[0] == 0x05)
> -			work[0] = 0xE5;
> -		for (i = 0, j = 0, last_u = 0; i < 8;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_BASE);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		j = last_u;
> -		fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -		for (i = 8; i < MSDOS_NAME;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i],
> -						MSDOS_NAME - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_EXT);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		if (!last_u)
> +		/* Never prepend '.' to hidden files here.
> +		 * That is done only for msdos mounts (and only when
> +		 * 'dotsOK=yes'); if we are executing here, it is in the
> +		 * context of a vfat mount.
> +		 */
> +		len = fat_parse_short(sb, de, bufname, 0);
> +		if (len == 0)
>  			continue;
>  
>  		/* Compare shortname */
> -		bufuname[last_u] = 0x0000;
> -		len = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
>  		if (fat_name_match(sbi, name, name_len, bufname, len))
>  			goto found;
>  
> @@ -469,20 +539,15 @@ static int __fat_readdir(struct inode *i
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char c, work[MSDOS_NAME];
> -	unsigned char bufname[FAT_MAX_SHORT_SIZE], *ptname = bufname;
> -	unsigned short opt_shortname = sbi->options.shortname;
> +	unsigned char bufname[FAT_MAX_SHORT_SIZE];
>  	int isvfat = sbi->options.isvfat;
> -	int nocase = sbi->options.nocase;
>  	const char *fill_name = NULL;
>  	unsigned long inum;
>  	unsigned long lpos, dummy, *furrfu = &lpos;
>  	loff_t cpos;
> -	int chi, chl, i, i2, j, last, last_u, dotoffset = 0, fill_len = 0;
> +	int short_len = 0, fill_len = 0;
>  	int ret = 0;
>  
>  	lock_super(sb);
> @@ -556,74 +621,10 @@ parse_record:
>  		}
>  	}
>  
> -	if (sbi->options.dotsOK) {
> -		ptname = bufname;
> -		dotoffset = 0;
> -		if (de->attr & ATTR_HIDDEN) {
> -			*ptname++ = '.';
> -			dotoffset = 1;
> -		}
> -	}
> -
> -	memcpy(work, de->name, sizeof(de->name));
> -	/* see namei.c, msdos_format_name */
> -	if (work[0] == 0x05)
> -		work[0] = 0xE5;
> -	for (i = 0, j = 0, last = 0, last_u = 0; i < 8;) {
> -		if (!(c = work[i]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_BASE);
> -		if (chl <= 1) {
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i < 8; chi++) {
> -				ptname[i] = work[i];
> -				i++; last = i;
> -			}
> -		}
> -	}
> -	i = last;
> -	j = last_u;
> -	fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -	ptname[i++] = '.';
> -	for (i2 = 8; i2 < MSDOS_NAME;) {
> -		if (!(c = work[i2]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i2], MSDOS_NAME - i2,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_EXT);
> -		if (chl <= 1) {
> -			i2++;
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i2 < MSDOS_NAME; chi++) {
> -				ptname[i++] = work[i2++];
> -				last = i;
> -			}
> -		}
> -	}
> -	if (!last)
> +	short_len = fat_parse_short(sb, de, bufname, sbi->options.dotsOK);
> +	if (short_len == 0)
>  		goto record_end;
>  
> -	i = last + dotoffset;
> -	j = last_u;
> -
> -	if (isvfat) {
> -		bufuname[j] = 0x0000;
> -		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
> -	}
>  	if (nr_slots) {
>  		/* hack for fat_ioctl_filldir() */
>  		struct fat_ioctl_filldir_callback *p = dirent;
> @@ -631,12 +632,12 @@ parse_record:
>  		p->longname = fill_name;
>  		p->long_len = fill_len;
>  		p->shortname = bufname;
> -		p->short_len = i;
> +		p->short_len = short_len;
>  		fill_name = NULL;
>  		fill_len = 0;
>  	} else {
>  		fill_name = bufname;
> -		fill_len = i;
> +		fill_len = short_len;
>  	}
>  
>  start_filldir:
>

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

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Sat, 2012-06-30 at 04:08 +0900, OGAWA Hirofumi wrote: 
> "Steven J. Magnani" <steve@digidescorp.com> writes:
> 
> > Nearly identical shortname parsing is performed in fat_search_long()
> > and __fat_readdir(). Extract this code into a function that may be
> > called by both.
> 
> What is the difference, and why changes was needed?

The only difference I could see was in the "dot_hidden" functionality.  
fat_search_long() never does it; __fat_readdir() does it only when the
'dotsOK' option is active.

The changes are not 'needed'; they are purely to simplify future
maintenance of the code.

Regards,
Steve

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> On Sat, 2012-06-30 at 04:08 +0900, OGAWA Hirofumi wrote: 
>> "Steven J. Magnani" <steve@digidescorp.com> writes:
>> 
>> > Nearly identical shortname parsing is performed in fat_search_long()
>> > and __fat_readdir(). Extract this code into a function that may be
>> > called by both.
>> 
>> What is the difference, and why changes was needed?
>
> The only difference I could see was in the "dot_hidden" functionality.  
> fat_search_long() never does it; __fat_readdir() does it only when the
> 'dotsOK' option is active.
>
> The changes are not 'needed'; they are purely to simplify future
> maintenance of the code.

Ah, I misread your email. I thought you made the nearly identical
parser. So, this patch is no logic change, right?

OK, looks like good though. If we added to check vfat and run, it might
be more readable. E.g. the following?

> +	if (dot_hidden && (de->attr & ATTR_HIDDEN)) {
> +		*ptname++ = '.';
> +		dotoffset = 1;
> +	}
> +
> +	memcpy(work, de->name, sizeof(work));
> +	/* see namei.c, msdos_format_name */
> +	if (work[0] == 0x05)
> +		work[0] = 0xE5;
> +
> +	/* Filename */
> +	for (i = 0, j = 0; i < 8;) {
> +		c = work[i];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_BASE);
> +		if (chl <= 1) {
> +			ptname[i++] = fat_tolower(!nocase, c);

			if (is_vfat)
				ptname[i++] = fat_tolower(!nocase, c);

> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			for (chi = 0; chi < chl && i < 8; chi++) {
> +				ptname[i] = work[i];

				if (is_vfat)
					ptname[i] = work[i];

> +				i++;
> +				name_len = i;
> +			}
> +		}
> +	}
> +
> +	i = name_len;
> +	j = uni_len;
> +	fat_short2uni(nls_disk, ".", 1, &uni_name[j++]);
> +	ptname[i++] = '.';
	if (is_vfat)
		ptname[i++] = '.';
> +
> +	/* Extension */
> +	for (k = 8; k < MSDOS_NAME;) {
> +		c = work[k];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[k], MSDOS_NAME - k,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_EXT);
> +		if (chl <= 1) {
> +			k++;
> +			ptname[i++] = fat_tolower(!nocase, c);

  		        if (is_vat)
				ptname[i++] = fat_tolower(!nocase, c);

> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			for (chi = 0; chi < chl && k < MSDOS_NAME; chi++) {
> +				ptname[i++] = work[k++];
> +				name_len = i;
> +			}
> +		}
> +	}
> +
> +	if (name_len > 0) {
> +		name_len += dotoffset;
> +
> +		if (sbi->options.isvfat) {
> +			uni_name[uni_len] = 0x0000;
> +			name_len = fat_uni_to_x8(sb, uni_name, name,
> +						 FAT_MAX_SHORT_SIZE);
> +		}
> +	}
> +
> +	return name_len;
> +}
> +
>  /*
>   * Return values: negative -> error, 0 -> not found, positive -> found,
>   * value is the total amount of slots, including the shortname entry.
> @@ -344,15 +449,11 @@ int fat_search_long(struct inode *inode,
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh = NULL;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char work[MSDOS_NAME];
>  	unsigned char bufname[FAT_MAX_SHORT_SIZE];
> -	unsigned short opt_shortname = sbi->options.shortname;
>  	loff_t cpos = 0;
> -	int chl, i, j, last_u, err, len;
> +	int err, len;
>  
>  	err = -ENOENT;
>  	while (1) {
> @@ -380,47 +481,16 @@ parse_record:
>  				goto end_of_dir;
>  		}
>  
> -		memcpy(work, de->name, sizeof(de->name));
> -		/* see namei.c, msdos_format_name */
> -		if (work[0] == 0x05)
> -			work[0] = 0xE5;
> -		for (i = 0, j = 0, last_u = 0; i < 8;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_BASE);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		j = last_u;
> -		fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -		for (i = 8; i < MSDOS_NAME;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i],
> -						MSDOS_NAME - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_EXT);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		if (!last_u)
> +		/* Never prepend '.' to hidden files here.
> +		 * That is done only for msdos mounts (and only when
> +		 * 'dotsOK=yes'); if we are executing here, it is in the
> +		 * context of a vfat mount.
> +		 */
> +		len = fat_parse_short(sb, de, bufname, 0);
> +		if (len == 0)
>  			continue;
>  
>  		/* Compare shortname */
> -		bufuname[last_u] = 0x0000;
> -		len = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
>  		if (fat_name_match(sbi, name, name_len, bufname, len))
>  			goto found;
>  
> @@ -469,20 +539,15 @@ static int __fat_readdir(struct inode *i
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char c, work[MSDOS_NAME];
> -	unsigned char bufname[FAT_MAX_SHORT_SIZE], *ptname = bufname;
> -	unsigned short opt_shortname = sbi->options.shortname;
> +	unsigned char bufname[FAT_MAX_SHORT_SIZE];
>  	int isvfat = sbi->options.isvfat;
> -	int nocase = sbi->options.nocase;
>  	const char *fill_name = NULL;
>  	unsigned long inum;
>  	unsigned long lpos, dummy, *furrfu = &lpos;
>  	loff_t cpos;
> -	int chi, chl, i, i2, j, last, last_u, dotoffset = 0, fill_len = 0;
> +	int short_len = 0, fill_len = 0;
>  	int ret = 0;
>  
>  	lock_super(sb);
> @@ -556,74 +621,10 @@ parse_record:
>  		}
>  	}
>  
> -	if (sbi->options.dotsOK) {
> -		ptname = bufname;
> -		dotoffset = 0;
> -		if (de->attr & ATTR_HIDDEN) {
> -			*ptname++ = '.';
> -			dotoffset = 1;
> -		}
> -	}
> -
> -	memcpy(work, de->name, sizeof(de->name));
> -	/* see namei.c, msdos_format_name */
> -	if (work[0] == 0x05)
> -		work[0] = 0xE5;
> -	for (i = 0, j = 0, last = 0, last_u = 0; i < 8;) {
> -		if (!(c = work[i]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_BASE);
> -		if (chl <= 1) {
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i < 8; chi++) {
> -				ptname[i] = work[i];
> -				i++; last = i;
> -			}
> -		}
> -	}
> -	i = last;
> -	j = last_u;
> -	fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -	ptname[i++] = '.';
> -	for (i2 = 8; i2 < MSDOS_NAME;) {
> -		if (!(c = work[i2]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i2], MSDOS_NAME - i2,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_EXT);
> -		if (chl <= 1) {
> -			i2++;
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i2 < MSDOS_NAME; chi++) {
> -				ptname[i++] = work[i2++];
> -				last = i;
> -			}
> -		}
> -	}
> -	if (!last)
> +	short_len = fat_parse_short(sb, de, bufname, sbi->options.dotsOK);
> +	if (short_len == 0)
>  		goto record_end;
>  
> -	i = last + dotoffset;
> -	j = last_u;
> -
> -	if (isvfat) {
> -		bufuname[j] = 0x0000;
> -		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
> -	}
>  	if (nr_slots) {
>  		/* hack for fat_ioctl_filldir() */
>  		struct fat_ioctl_filldir_callback *p = dirent;
> @@ -631,12 +632,12 @@ parse_record:
>  		p->longname = fill_name;
>  		p->long_len = fill_len;
>  		p->shortname = bufname;
> -		p->short_len = i;
> +		p->short_len = short_len;
>  		fill_name = NULL;
>  		fill_len = 0;
>  	} else {
>  		fill_name = bufname;
> -		fill_len = i;
> +		fill_len = short_len;
>  	}
>  
>  start_filldir:
>
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:

>
> 			if (is_vfat)
> 				ptname[i++] = fat_tolower(!nocase, c);

Of course, if (!is_vfat). Sorry.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Sat, 2012-06-30 at 05:09 +0900, OGAWA Hirofumi wrote: 
> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
> 
> >
> > 			if (is_vfat)
> > 				ptname[i++] = fat_tolower(!nocase, c);
> 
> Of course, if (!is_vfat). Sorry.

I agree that the nocase logic is confusing, but I'm pretty sure this
change would break the code. 

'nocase' is always zero for vfat, which does not recognize that option.
For msdos, it is zero by default, and 1 if the 'nocase' option was
specified.
In all cases it is necessary to copy *something* to ptname.

What could be done is something like this:

if (nocase)
ptname[i++] = c;
else
ptname[i++] = fat_tolower(c);

or, if you don't mind trigraphs:
ptname[i++] = nocase ? c : fat_tolower(c);

Let me know what you prefer.

Thanks,
Steve

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> On Sat, 2012-06-30 at 05:09 +0900, OGAWA Hirofumi wrote: 
>> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
>> 
>> >
>> > 			if (is_vfat)
>> > 				ptname[i++] = fat_tolower(!nocase, c);
>> 
>> Of course, if (!is_vfat). Sorry.
>
> I agree that the nocase logic is confusing, but I'm pretty sure this
> change would break the code. 

I might be wrong here though, which place is broken with it?

> 'nocase' is always zero for vfat, which does not recognize that option.
> For msdos, it is zero by default, and 1 if the 'nocase' option was
> specified.
> In all cases it is necessary to copy *something* to ptname.

Looks like, we overwrite bufname by converted bufuname in is_vfat case?

	if (isvfat) {
		bufuname[j] = 0x0000;
		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
	}

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

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Mon, 2012-07-02 at 22:40 +0900, OGAWA Hirofumi wrote: 
> "Steven J. Magnani" <steve@digidescorp.com> writes:
> 
> > On Sat, 2012-06-30 at 05:09 +0900, OGAWA Hirofumi wrote: 
> >> OGAWA Hirofumi <hirofumi@mail.parknet.co.jp> writes:
> >> 
> >> >
> >> > 			if (is_vfat)
> >> > 				ptname[i++] = fat_tolower(!nocase, c);
> >> 
> >> Of course, if (!is_vfat). Sorry.
> >
> > I agree that the nocase logic is confusing, but I'm pretty sure this
> > change would break the code. 
> 
> I might be wrong here though, which place is broken with it?
> 
> > 'nocase' is always zero for vfat, which does not recognize that option.
> > For msdos, it is zero by default, and 1 if the 'nocase' option was
> > specified.
> > In all cases it is necessary to copy *something* to ptname.
> 
> Looks like, we overwrite bufname by converted bufuname in is_vfat case?
> 
> 	if (isvfat) {
> 		bufuname[j] = 0x0000;
> 		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
> 	}

True, but with the change you suggest we lose the incrementing of 'i',
which likely would cause an infinite loop for 1:1 Unicode conversions.

Steve

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> True, but with the change you suggest we lose the incrementing of 'i',
> which likely would cause an infinite loop for 1:1 Unicode conversions.

You meant, we just have to do

	if (!is_vfat)
        	ptname[i] = ...;
	i++;

or something? I still feel this looks better to indicate, we don't use
ptname in the case of vfat.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Mon, 2012-07-02 at 23:36 +0900, OGAWA Hirofumi wrote: 
> "Steven J. Magnani" <steve@digidescorp.com> writes:
> 
> > True, but with the change you suggest we lose the incrementing of 'i',
> > which likely would cause an infinite loop for 1:1 Unicode conversions.
> 
> You meant, we just have to do
> 
> 	if (!is_vfat)
>         	ptname[i] = ...;
> 	i++;
> 
> or something? I still feel this looks better to indicate, we don't use
> ptname in the case of vfat.

I can change it, but there are other places in that function where
ptname is used that are not qualified with !is_vfat, so I don't know
whether this improves clarity or reduces it.

I do think fat_tolower() should not be making decisions. IMHO the
trigraph and a comment, perhaps before the vfat-only reassignment of
'name', would be clearer. 

Steve

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> On Mon, 2012-07-02 at 23:36 +0900, OGAWA Hirofumi wrote: 
>> "Steven J. Magnani" <steve@digidescorp.com> writes:
>> 
>> > True, but with the change you suggest we lose the incrementing of 'i',
>> > which likely would cause an infinite loop for 1:1 Unicode conversions.
>> 
>> You meant, we just have to do
>> 
>> 	if (!is_vfat)
>>         	ptname[i] = ...;
>> 	i++;
>> 
>> or something? I still feel this looks better to indicate, we don't use
>> ptname in the case of vfat.
>
> I can change it, but there are other places in that function where
> ptname is used that are not qualified with !is_vfat, so I don't know
> whether this improves clarity or reduces it.
>
> I do think fat_tolower() should not be making decisions. IMHO the
> trigraph and a comment, perhaps before the vfat-only reassignment of
> 'name', would be clearer. 

Hm, the primary case is vfat. fat_tolower()/hidden is required only for
msdos, and ptname too. So, my suggestion is trying to keep vfat case
clean.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Tue, 2012-07-03 at 00:11 +0900, OGAWA Hirofumi wrote: 
> "Steven J. Magnani" <steve@digidescorp.com> writes:
> 
> > On Mon, 2012-07-02 at 23:36 +0900, OGAWA Hirofumi wrote: 
> >> "Steven J. Magnani" <steve@digidescorp.com> writes:
> >> 
> >> > True, but with the change you suggest we lose the incrementing of 'i',
> >> > which likely would cause an infinite loop for 1:1 Unicode conversions.
> >> 
> >> You meant, we just have to do
> >> 
> >> 	if (!is_vfat)
> >>         	ptname[i] = ...;
> >> 	i++;
> >> 
> >> or something? I still feel this looks better to indicate, we don't use
> >> ptname in the case of vfat.
> >
> > I can change it, but there are other places in that function where
> > ptname is used that are not qualified with !is_vfat, so I don't know
> > whether this improves clarity or reduces it.
> >
> > I do think fat_tolower() should not be making decisions. IMHO the
> > trigraph and a comment, perhaps before the vfat-only reassignment of
> > 'name', would be clearer. 
> 
> Hm, the primary case is vfat. fat_tolower()/hidden is required only for
> msdos, and ptname too. So, my suggestion is trying to keep vfat case
> clean.

It's not clear to me where you want to go with this. 

1. Split fat_parse_short() into msdos and vfat versions. This may
improve clarity, but there would be some replication of code.

2. Sprinkle "if (!isvfat)" throughout the already-proposed version of
fat_parse_short()
  A. Everywhere
  B. Only in the places you've proposed

3. Retain the already-proposed version of fat_parse_short(), but add a
comment that the uni_name overrides the msdos "ptname", and separate out
the decision to lowercase from fat_tolower() [i.e., the trigraph].

4. Other options?

The NFS changes I would like to post depend on fat_parse_short() in some
form.

Steve

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

>> Hm, the primary case is vfat. fat_tolower()/hidden is required only for
>> msdos, and ptname too. So, my suggestion is trying to keep vfat case
>> clean.
>
> It's not clear to me where you want to go with this. 
>
> 1. Split fat_parse_short() into msdos and vfat versions. This may
> improve clarity, but there would be some replication of code.

This is not a option. We are better to not change.

> 2. Sprinkle "if (!isvfat)" throughout the already-proposed version of
> fat_parse_short()
>   A. Everywhere
>   B. Only in the places you've proposed

I'm not sure what (A) means though. Probably, I will not care.

> 3. Retain the already-proposed version of fat_parse_short(), but add a
> comment that the uni_name overrides the msdos "ptname", and separate out
> the decision to lowercase from fat_tolower() [i.e., the trigraph].

The trigraph change doesn't cleanup anything. Because it is using
needless ptname on vfat path.

vfat always uses the uni_name and doesn't use ptname, right?  And msdos
path doesn't use the uni_name and use ptname always.  So, I'm saying,
let's make clear those difference and usage, by annotating with isvfat.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

[PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

Nearly identical shortname parsing is performed in fat_search_long()
and __fat_readdir(). Extract this code into a function that may be
called by both.

v2: Attempt to clarify difference between vfat and msdos parsing.
    Remove decision-making from fat_tolower() for clarity.

Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
---
diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
--- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
+++ new/fs/fat/dir.c	2012-07-03 06:10:36.066283411 -0500
@@ -35,6 +35,11 @@
 #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
 #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
 
+static inline unsigned char fat_tolower(unsigned char c)
+{
+	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
+}
+
 static inline loff_t fat_make_i_pos(struct super_block *sb,
 				    struct buffer_head *bh,
 				    struct msdos_dir_entry *de)
@@ -333,6 +338,124 @@ parse_long:
 	return 0;
 }
 
+/**
+ * fat_parse_short - Parse MS-DOS (short) directory entry.
+ * @sb:		superblock
+ * @de:		directory entry to parse
+ * @name:	FAT_MAX_SHORT_SIZE array in which to place extracted name
+ * @dot_hidden:	Nonzero == prepend '.' to names with ATTR_HIDDEN
+ *
+ * Returns the number of characters extracted into 'name'.
+ */
+static int fat_parse_short(struct super_block *sb,
+			   const struct msdos_dir_entry *de,
+			   unsigned char *name, int dot_hidden)
+{
+	const struct msdos_sb_info *sbi = MSDOS_SB(sb);
+	int isvfat = sbi->options.isvfat;
+	int nocase = sbi->options.nocase;
+	unsigned short opt_shortname = sbi->options.shortname;
+	struct nls_table *nls_disk = sbi->nls_disk;
+	wchar_t uni_name[14];
+	unsigned char c, work[MSDOS_NAME];
+	unsigned char *ptname = name;
+	int chi, chl, i, j, k;
+	int dotoffset = 0;
+	int name_len = 0, uni_len = 0;
+
+	if (!isvfat && dot_hidden && (de->attr & ATTR_HIDDEN)) {
+		*ptname++ = '.';
+		dotoffset = 1;
+	}
+
+	memcpy(work, de->name, sizeof(work));
+	/* see namei.c, msdos_format_name */
+	if (work[0] == 0x05)
+		work[0] = 0xE5;
+
+	/* Filename */
+	for (i = 0, j = 0; i < 8;) {
+		c = work[i];
+		if (!c)
+			break;
+		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
+					&uni_name[j++], opt_shortname,
+					de->lcase & CASE_LOWER_BASE);
+		if (chl <= 1) {
+			if (!isvfat)
+				ptname[i] = nocase ? c : fat_tolower(c);
+			i++;
+			if (c != ' ') {
+				name_len = i;
+				uni_len  = j;
+			}
+		} else {
+			uni_len = j;
+			if (isvfat)
+				i += min(chl, 8-i);
+			else {
+				for (chi = 0; chi < chl && i < 8; chi++, i++)
+					ptname[i] = work[i];
+			}
+			if (chl)
+				name_len = i;
+		}
+	}
+
+	i = name_len;
+	j = uni_len;
+	fat_short2uni(nls_disk, ".", 1, &uni_name[j++]);
+	if (!isvfat)
+		ptname[i] = '.';
+	i++;
+
+	/* Extension */
+	for (k = 8; k < MSDOS_NAME;) {
+		c = work[k];
+		if (!c)
+			break;
+		chl = fat_shortname2uni(nls_disk, &work[k], MSDOS_NAME - k,
+					&uni_name[j++], opt_shortname,
+					de->lcase & CASE_LOWER_EXT);
+		if (chl <= 1) {
+			k++;
+			if (!isvfat)
+				ptname[i] = nocase ? c : fat_tolower(c);
+			i++;
+			if (c != ' ') {
+				name_len = i;
+				uni_len  = j;
+			}
+		} else {
+			uni_len = j;
+			if (isvfat) {
+				int offset = min(chl, MSDOS_NAME-k);
+				k += offset;
+				i += offset;
+			} else {
+				for (chi = 0; chi < chl && k < MSDOS_NAME;
+				     chi++, i++, k++) {
+						ptname[i] = work[k];
+				}
+			}
+			if (chl)
+				name_len = i;
+		}
+	}
+
+	if (name_len > 0) {
+		name_len += dotoffset;
+
+		if (sbi->options.isvfat) {
+			uni_name[uni_len] = 0x0000;
+			name_len = fat_uni_to_x8(sb, uni_name, name,
+						 FAT_MAX_SHORT_SIZE);
+		}
+	}
+
+	return name_len;
+}
+
 /*
  * Return values: negative -> error, 0 -> not found, positive -> found,
  * value is the total amount of slots, including the shortname entry.
@@ -344,15 +467,11 @@ int fat_search_long(struct inode *inode,
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	struct buffer_head *bh = NULL;
 	struct msdos_dir_entry *de;
-	struct nls_table *nls_disk = sbi->nls_disk;
 	unsigned char nr_slots;
-	wchar_t bufuname[14];
 	wchar_t *unicode = NULL;
-	unsigned char work[MSDOS_NAME];
 	unsigned char bufname[FAT_MAX_SHORT_SIZE];
-	unsigned short opt_shortname = sbi->options.shortname;
 	loff_t cpos = 0;
-	int chl, i, j, last_u, err, len;
+	int err, len;
 
 	err = -ENOENT;
 	while (1) {
@@ -380,47 +499,16 @@ parse_record:
 				goto end_of_dir;
 		}
 
-		memcpy(work, de->name, sizeof(de->name));
-		/* see namei.c, msdos_format_name */
-		if (work[0] == 0x05)
-			work[0] = 0xE5;
-		for (i = 0, j = 0, last_u = 0; i < 8;) {
-			if (!work[i])
-				break;
-			chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
-						&bufuname[j++], opt_shortname,
-						de->lcase & CASE_LOWER_BASE);
-			if (chl <= 1) {
-				if (work[i] != ' ')
-					last_u = j;
-			} else {
-				last_u = j;
-			}
-			i += chl;
-		}
-		j = last_u;
-		fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
-		for (i = 8; i < MSDOS_NAME;) {
-			if (!work[i])
-				break;
-			chl = fat_shortname2uni(nls_disk, &work[i],
-						MSDOS_NAME - i,
-						&bufuname[j++], opt_shortname,
-						de->lcase & CASE_LOWER_EXT);
-			if (chl <= 1) {
-				if (work[i] != ' ')
-					last_u = j;
-			} else {
-				last_u = j;
-			}
-			i += chl;
-		}
-		if (!last_u)
+		/* Never prepend '.' to hidden files here.
+		 * That is done only for msdos mounts (and only when
+		 * 'dotsOK=yes'); if we are executing here, it is in the
+		 * context of a vfat mount.
+		 */
+		len = fat_parse_short(sb, de, bufname, 0);
+		if (len == 0)
 			continue;
 
 		/* Compare shortname */
-		bufuname[last_u] = 0x0000;
-		len = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
 		if (fat_name_match(sbi, name, name_len, bufname, len))
 			goto found;
 
@@ -469,20 +557,15 @@ static int __fat_readdir(struct inode *i
 	struct msdos_sb_info *sbi = MSDOS_SB(sb);
 	struct buffer_head *bh;
 	struct msdos_dir_entry *de;
-	struct nls_table *nls_disk = sbi->nls_disk;
 	unsigned char nr_slots;
-	wchar_t bufuname[14];
 	wchar_t *unicode = NULL;
-	unsigned char c, work[MSDOS_NAME];
-	unsigned char bufname[FAT_MAX_SHORT_SIZE], *ptname = bufname;
-	unsigned short opt_shortname = sbi->options.shortname;
+	unsigned char bufname[FAT_MAX_SHORT_SIZE];
 	int isvfat = sbi->options.isvfat;
-	int nocase = sbi->options.nocase;
 	const char *fill_name = NULL;
 	unsigned long inum;
 	unsigned long lpos, dummy, *furrfu = &lpos;
 	loff_t cpos;
-	int chi, chl, i, i2, j, last, last_u, dotoffset = 0, fill_len = 0;
+	int short_len = 0, fill_len = 0;
 	int ret = 0;
 
 	lock_super(sb);
@@ -556,74 +639,10 @@ parse_record:
 		}
 	}
 
-	if (sbi->options.dotsOK) {
-		ptname = bufname;
-		dotoffset = 0;
-		if (de->attr & ATTR_HIDDEN) {
-			*ptname++ = '.';
-			dotoffset = 1;
-		}
-	}
-
-	memcpy(work, de->name, sizeof(de->name));
-	/* see namei.c, msdos_format_name */
-	if (work[0] == 0x05)
-		work[0] = 0xE5;
-	for (i = 0, j = 0, last = 0, last_u = 0; i < 8;) {
-		if (!(c = work[i]))
-			break;
-		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
-					&bufuname[j++], opt_shortname,
-					de->lcase & CASE_LOWER_BASE);
-		if (chl <= 1) {
-			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
-			if (c != ' ') {
-				last = i;
-				last_u = j;
-			}
-		} else {
-			last_u = j;
-			for (chi = 0; chi < chl && i < 8; chi++) {
-				ptname[i] = work[i];
-				i++; last = i;
-			}
-		}
-	}
-	i = last;
-	j = last_u;
-	fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
-	ptname[i++] = '.';
-	for (i2 = 8; i2 < MSDOS_NAME;) {
-		if (!(c = work[i2]))
-			break;
-		chl = fat_shortname2uni(nls_disk, &work[i2], MSDOS_NAME - i2,
-					&bufuname[j++], opt_shortname,
-					de->lcase & CASE_LOWER_EXT);
-		if (chl <= 1) {
-			i2++;
-			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
-			if (c != ' ') {
-				last = i;
-				last_u = j;
-			}
-		} else {
-			last_u = j;
-			for (chi = 0; chi < chl && i2 < MSDOS_NAME; chi++) {
-				ptname[i++] = work[i2++];
-				last = i;
-			}
-		}
-	}
-	if (!last)
+	short_len = fat_parse_short(sb, de, bufname, sbi->options.dotsOK);
+	if (short_len == 0)
 		goto record_end;
 
-	i = last + dotoffset;
-	j = last_u;
-
-	if (isvfat) {
-		bufuname[j] = 0x0000;
-		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
-	}
 	if (nr_slots) {
 		/* hack for fat_ioctl_filldir() */
 		struct fat_ioctl_filldir_callback *p = dirent;
@@ -631,12 +650,12 @@ parse_record:
 		p->longname = fill_name;
 		p->long_len = fill_len;
 		p->shortname = bufname;
-		p->short_len = i;
+		p->short_len = short_len;
 		fill_name = NULL;
 		fill_len = 0;
 	} else {
 		fill_name = bufname;
-		fill_len = i;
+		fill_len = short_len;
 	}
 
 start_filldir:

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

"Steven J. Magnani" <steve@digidescorp.com> writes:

> Nearly identical shortname parsing is performed in fat_search_long()
> and __fat_readdir(). Extract this code into a function that may be
> called by both.
>
> v2: Attempt to clarify difference between vfat and msdos parsing.
>     Remove decision-making from fat_tolower() for clarity.

Looks good. Thanks.

Acked-by: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

> Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
> ---
> diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
> --- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
> +++ new/fs/fat/dir.c	2012-07-03 06:10:36.066283411 -0500
> @@ -35,6 +35,11 @@
>  #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
>  #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
>  
> +static inline unsigned char fat_tolower(unsigned char c)
> +{
> +	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
> +}
> +
>  static inline loff_t fat_make_i_pos(struct super_block *sb,
>  				    struct buffer_head *bh,
>  				    struct msdos_dir_entry *de)
> @@ -333,6 +338,124 @@ parse_long:
>  	return 0;
>  }
>  
> +/**
> + * fat_parse_short - Parse MS-DOS (short) directory entry.
> + * @sb:		superblock
> + * @de:		directory entry to parse
> + * @name:	FAT_MAX_SHORT_SIZE array in which to place extracted name
> + * @dot_hidden:	Nonzero == prepend '.' to names with ATTR_HIDDEN
> + *
> + * Returns the number of characters extracted into 'name'.
> + */
> +static int fat_parse_short(struct super_block *sb,
> +			   const struct msdos_dir_entry *de,
> +			   unsigned char *name, int dot_hidden)
> +{
> +	const struct msdos_sb_info *sbi = MSDOS_SB(sb);
> +	int isvfat = sbi->options.isvfat;
> +	int nocase = sbi->options.nocase;
> +	unsigned short opt_shortname = sbi->options.shortname;
> +	struct nls_table *nls_disk = sbi->nls_disk;
> +	wchar_t uni_name[14];
> +	unsigned char c, work[MSDOS_NAME];
> +	unsigned char *ptname = name;
> +	int chi, chl, i, j, k;
> +	int dotoffset = 0;
> +	int name_len = 0, uni_len = 0;
> +
> +	if (!isvfat && dot_hidden && (de->attr & ATTR_HIDDEN)) {
> +		*ptname++ = '.';
> +		dotoffset = 1;
> +	}
> +
> +	memcpy(work, de->name, sizeof(work));
> +	/* see namei.c, msdos_format_name */
> +	if (work[0] == 0x05)
> +		work[0] = 0xE5;
> +
> +	/* Filename */
> +	for (i = 0, j = 0; i < 8;) {
> +		c = work[i];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_BASE);
> +		if (chl <= 1) {
> +			if (!isvfat)
> +				ptname[i] = nocase ? c : fat_tolower(c);
> +			i++;
> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			if (isvfat)
> +				i += min(chl, 8-i);
> +			else {
> +				for (chi = 0; chi < chl && i < 8; chi++, i++)
> +					ptname[i] = work[i];
> +			}
> +			if (chl)
> +				name_len = i;
> +		}
> +	}
> +
> +	i = name_len;
> +	j = uni_len;
> +	fat_short2uni(nls_disk, ".", 1, &uni_name[j++]);
> +	if (!isvfat)
> +		ptname[i] = '.';
> +	i++;
> +
> +	/* Extension */
> +	for (k = 8; k < MSDOS_NAME;) {
> +		c = work[k];
> +		if (!c)
> +			break;
> +		chl = fat_shortname2uni(nls_disk, &work[k], MSDOS_NAME - k,
> +					&uni_name[j++], opt_shortname,
> +					de->lcase & CASE_LOWER_EXT);
> +		if (chl <= 1) {
> +			k++;
> +			if (!isvfat)
> +				ptname[i] = nocase ? c : fat_tolower(c);
> +			i++;
> +			if (c != ' ') {
> +				name_len = i;
> +				uni_len  = j;
> +			}
> +		} else {
> +			uni_len = j;
> +			if (isvfat) {
> +				int offset = min(chl, MSDOS_NAME-k);
> +				k += offset;
> +				i += offset;
> +			} else {
> +				for (chi = 0; chi < chl && k < MSDOS_NAME;
> +				     chi++, i++, k++) {
> +						ptname[i] = work[k];
> +				}
> +			}
> +			if (chl)
> +				name_len = i;
> +		}
> +	}
> +
> +	if (name_len > 0) {
> +		name_len += dotoffset;
> +
> +		if (sbi->options.isvfat) {
> +			uni_name[uni_len] = 0x0000;
> +			name_len = fat_uni_to_x8(sb, uni_name, name,
> +						 FAT_MAX_SHORT_SIZE);
> +		}
> +	}
> +
> +	return name_len;
> +}
> +
>  /*
>   * Return values: negative -> error, 0 -> not found, positive -> found,
>   * value is the total amount of slots, including the shortname entry.
> @@ -344,15 +467,11 @@ int fat_search_long(struct inode *inode,
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh = NULL;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char work[MSDOS_NAME];
>  	unsigned char bufname[FAT_MAX_SHORT_SIZE];
> -	unsigned short opt_shortname = sbi->options.shortname;
>  	loff_t cpos = 0;
> -	int chl, i, j, last_u, err, len;
> +	int err, len;
>  
>  	err = -ENOENT;
>  	while (1) {
> @@ -380,47 +499,16 @@ parse_record:
>  				goto end_of_dir;
>  		}
>  
> -		memcpy(work, de->name, sizeof(de->name));
> -		/* see namei.c, msdos_format_name */
> -		if (work[0] == 0x05)
> -			work[0] = 0xE5;
> -		for (i = 0, j = 0, last_u = 0; i < 8;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_BASE);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		j = last_u;
> -		fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -		for (i = 8; i < MSDOS_NAME;) {
> -			if (!work[i])
> -				break;
> -			chl = fat_shortname2uni(nls_disk, &work[i],
> -						MSDOS_NAME - i,
> -						&bufuname[j++], opt_shortname,
> -						de->lcase & CASE_LOWER_EXT);
> -			if (chl <= 1) {
> -				if (work[i] != ' ')
> -					last_u = j;
> -			} else {
> -				last_u = j;
> -			}
> -			i += chl;
> -		}
> -		if (!last_u)
> +		/* Never prepend '.' to hidden files here.
> +		 * That is done only for msdos mounts (and only when
> +		 * 'dotsOK=yes'); if we are executing here, it is in the
> +		 * context of a vfat mount.
> +		 */
> +		len = fat_parse_short(sb, de, bufname, 0);
> +		if (len == 0)
>  			continue;
>  
>  		/* Compare shortname */
> -		bufuname[last_u] = 0x0000;
> -		len = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
>  		if (fat_name_match(sbi, name, name_len, bufname, len))
>  			goto found;
>  
> @@ -469,20 +557,15 @@ static int __fat_readdir(struct inode *i
>  	struct msdos_sb_info *sbi = MSDOS_SB(sb);
>  	struct buffer_head *bh;
>  	struct msdos_dir_entry *de;
> -	struct nls_table *nls_disk = sbi->nls_disk;
>  	unsigned char nr_slots;
> -	wchar_t bufuname[14];
>  	wchar_t *unicode = NULL;
> -	unsigned char c, work[MSDOS_NAME];
> -	unsigned char bufname[FAT_MAX_SHORT_SIZE], *ptname = bufname;
> -	unsigned short opt_shortname = sbi->options.shortname;
> +	unsigned char bufname[FAT_MAX_SHORT_SIZE];
>  	int isvfat = sbi->options.isvfat;
> -	int nocase = sbi->options.nocase;
>  	const char *fill_name = NULL;
>  	unsigned long inum;
>  	unsigned long lpos, dummy, *furrfu = &lpos;
>  	loff_t cpos;
> -	int chi, chl, i, i2, j, last, last_u, dotoffset = 0, fill_len = 0;
> +	int short_len = 0, fill_len = 0;
>  	int ret = 0;
>  
>  	lock_super(sb);
> @@ -556,74 +639,10 @@ parse_record:
>  		}
>  	}
>  
> -	if (sbi->options.dotsOK) {
> -		ptname = bufname;
> -		dotoffset = 0;
> -		if (de->attr & ATTR_HIDDEN) {
> -			*ptname++ = '.';
> -			dotoffset = 1;
> -		}
> -	}
> -
> -	memcpy(work, de->name, sizeof(de->name));
> -	/* see namei.c, msdos_format_name */
> -	if (work[0] == 0x05)
> -		work[0] = 0xE5;
> -	for (i = 0, j = 0, last = 0, last_u = 0; i < 8;) {
> -		if (!(c = work[i]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i], 8 - i,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_BASE);
> -		if (chl <= 1) {
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i < 8; chi++) {
> -				ptname[i] = work[i];
> -				i++; last = i;
> -			}
> -		}
> -	}
> -	i = last;
> -	j = last_u;
> -	fat_short2uni(nls_disk, ".", 1, &bufuname[j++]);
> -	ptname[i++] = '.';
> -	for (i2 = 8; i2 < MSDOS_NAME;) {
> -		if (!(c = work[i2]))
> -			break;
> -		chl = fat_shortname2uni(nls_disk, &work[i2], MSDOS_NAME - i2,
> -					&bufuname[j++], opt_shortname,
> -					de->lcase & CASE_LOWER_EXT);
> -		if (chl <= 1) {
> -			i2++;
> -			ptname[i++] = (!nocase && c>='A' && c<='Z') ? c+32 : c;
> -			if (c != ' ') {
> -				last = i;
> -				last_u = j;
> -			}
> -		} else {
> -			last_u = j;
> -			for (chi = 0; chi < chl && i2 < MSDOS_NAME; chi++) {
> -				ptname[i++] = work[i2++];
> -				last = i;
> -			}
> -		}
> -	}
> -	if (!last)
> +	short_len = fat_parse_short(sb, de, bufname, sbi->options.dotsOK);
> +	if (short_len == 0)
>  		goto record_end;
>  
> -	i = last + dotoffset;
> -	j = last_u;
> -
> -	if (isvfat) {
> -		bufuname[j] = 0x0000;
> -		i = fat_uni_to_x8(sb, bufuname, bufname, sizeof(bufname));
> -	}
>  	if (nr_slots) {
>  		/* hack for fat_ioctl_filldir() */
>  		struct fat_ioctl_filldir_callback *p = dirent;
> @@ -631,12 +650,12 @@ parse_record:
>  		p->longname = fill_name;
>  		p->long_len = fill_len;
>  		p->shortname = bufname;
> -		p->short_len = i;
> +		p->short_len = short_len;
>  		fill_name = NULL;
>  		fill_len = 0;
>  	} else {
>  		fill_name = bufname;
> -		fill_len = i;
> +		fill_len = short_len;
>  	}
>  
>  start_filldir:
>

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

Re: [PATCH] fat: Refactor shortname parsing

[Jan Engelhardt]

On Tuesday 2012-07-03 13:14, Steven J. Magnani wrote:

>Nearly identical shortname parsing is performed in fat_search_long()
>and __fat_readdir(). Extract this code into a function that may be
>called by both.
>
>v2: Attempt to clarify difference between vfat and msdos parsing.
>    Remove decision-making from fat_tolower() for clarity.
>
>Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
>---
>diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
>--- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
>+++ new/fs/fat/dir.c	2012-07-03 06:10:36.066283411 -0500
>@@ -35,6 +35,11 @@
> #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
> #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
> 
>+static inline unsigned char fat_tolower(unsigned char c)
>+{
>+	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
>+}
>+

The kernel already has a tolower() function, can that not be used?

Re: [PATCH] fat: Refactor shortname parsing

[OGAWA Hirofumi]

Jan Engelhardt <jengelh@inai.de> writes:

> On Tuesday 2012-07-03 13:14, Steven J. Magnani wrote:
>
>>Nearly identical shortname parsing is performed in fat_search_long()
>>and __fat_readdir(). Extract this code into a function that may be
>>called by both.
>>
>>v2: Attempt to clarify difference between vfat and msdos parsing.
>>    Remove decision-making from fat_tolower() for clarity.
>>
>>Signed-off-by: Steven J. Magnani <steve@digidescorp.com>
>>---
>>diff -uprN linux-3.5-rc4/fs/fat/dir.c new/fs/fat/dir.c
>>--- linux-3.5-rc4/fs/fat/dir.c	2012-06-29 11:20:12.766348728 -0500
>>+++ new/fs/fat/dir.c	2012-07-03 06:10:36.066283411 -0500
>>@@ -35,6 +35,11 @@
>> #define FAT_MAX_UNI_CHARS	((MSDOS_SLOTS - 1) * 13 + 1)
>> #define FAT_MAX_UNI_SIZE	(FAT_MAX_UNI_CHARS * sizeof(wchar_t))
>> 
>>+static inline unsigned char fat_tolower(unsigned char c)
>>+{
>>+	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
>>+}
>>+
>
> The kernel already has a tolower() function, can that not be used?

tolower() is not exactly same, right? e.g. tolower(0xc0). Otherwise,
tolower() is fine.
-- 
OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>

Re: [PATCH] fat: Refactor shortname parsing

[Jan Engelhardt]

On Friday 2012-08-03 17:06, OGAWA Hirofumi wrote:
>>>+static inline unsigned char fat_tolower(unsigned char c)
>>>+{
>>>+	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
>>>+}
>>>+
>>
>> The kernel already has a tolower() function, can that not be used?
>
>tolower() is not exactly same, right? e.g. tolower(0xc0). Otherwise,
>tolower() is fine.

Yes, but you can still

	return (c >= 'A' && c <= 'Z') ? tolower(c) : c;

Re: [PATCH] fat: Refactor shortname parsing

[Steven J. Magnani]

On Fri, 2012-08-03 at 17:58 +0200, Jan Engelhardt wrote: 
> On Friday 2012-08-03 17:06, OGAWA Hirofumi wrote:
> >>>+static inline unsigned char fat_tolower(unsigned char c)
> >>>+{
> >>>+	return ((c >= 'A') && (c <= 'Z')) ? c+32 : c;
> >>>+}
> >>>+
> >>
> >> The kernel already has a tolower() function, can that not be used?
> >
> >tolower() is not exactly same, right? e.g. tolower(0xc0). Otherwise,
> >tolower() is fine.
> 
> Yes, but you can still
> 
> 	return (c >= 'A' && c <= 'Z') ? tolower(c) : c;

But now it's less efficient because tolower() does an unnecessary lookup
to see if it's supposed to change the value. _tolower() wouldn't have
that issue, but it's marked "Do not use in your code". 

------------------------------------------------------------------------
Steven J. Magnani               "I claim this network for MARS!
www.digidescorp.com              Earthling, return my space modulator!"

#include <standard.disclaimer>