Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756307Ab2F2TIw (ORCPT ); Fri, 29 Jun 2012 15:08:52 -0400 Received: from mail.parknet.co.jp ([210.171.160.6]:39763 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756250Ab2F2TIu (ORCPT ); Fri, 29 Jun 2012 15:08:50 -0400 From: OGAWA Hirofumi To: "Steven J. Magnani" Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH] fat: Refactor shortname parsing References: <1340993528-12111-1-git-send-email-steve@digidescorp.com> Date: Sat, 30 Jun 2012 04:08:45 +0900 In-Reply-To: <1340993528-12111-1-git-send-email-steve@digidescorp.com> (Steven J. Magnani's message of "Fri, 29 Jun 2012 13:12:08 -0500") Message-ID: <87r4syrl4y.fsf@devron.myhome.or.jp> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1.50 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 9265 Lines: 328 "Steven J. Magnani" 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 > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/