ChenGuanqiao <chen.chencha...@foxmail.com> writes: > +static int fat_check_d_characters(char *label, unsigned long len) > +{ > + int i; > + > + for (i=0; i<len; ++i) {
coding style. "i=0" to "i = 0", etc. > + switch (label[i]) { > + case '0' ... '9': > + case 'A' ... 'Z': > + case '_': > + continue; > + case 0x20: > + return 0; Hm, stop check at ' '? What if "aaa b.%%%"? > +static int fat_ioctl_get_volume_label(struct inode *inode, > + u8 __user *vol_label) > +{ > + int err = 0; > + struct buffer_head *bh; > + struct msdos_dir_entry *de; Hm, user has to care to open rootdir for this ioctl? Isn't it better to force use the root inode? > + inode_lock_shared(inode); > + err = fat_scan_volume_label(inode, &bh, &de); > + if (err) > + goto out; > + > + if (copy_to_user(vol_label, de->name, MSDOS_NAME)) > + err = -EFAULT; Better to copy to user buffer outside locking. > +static int fat_ioctl_set_volume_label(struct file *file, > + u8 __user *vol_label) > +{ > + int err = 0; > + u8 label[MSDOS_NAME]; > + struct buffer_head *boot_bh; > + struct buffer_head *vol_bh; > + struct msdos_dir_entry *de; > + struct fat_boot_sector *b; > + struct inode *inode = file_inode(file); > + struct super_block *sb = inode->i_sb; > + struct msdos_sb_info *sbi = MSDOS_SB(sb); > + > + if (inode->i_ino != MSDOS_ROOT_INO) { Same with above, better to force use the root inode. > + down_write(&sb->s_umount); > + inode_lock(inode); > + > + /* Updates root directory's vol_label */ > + err = fat_scan_volume_label(inode, &vol_bh, &de); > + if (err) { > + /* Create volume label entry */ > + struct timespec ts; > + > + mutex_lock(&sbi->s_lock); No need sbi->s_lock? > + ts = current_time(inode); > + err = fat_add_volume_label_entry(inode, label, &ts); > + mutex_unlock(&sbi->s_lock); > + > + if (err) > + goto out_vol_brelse; > + } else { > + /* Write to root directory */ > + lock_buffer(vol_bh); No need lock_buffer()? > + memcpy(de->name, label, sizeof(de->name)); Probably, update timestamp? > + mark_buffer_dirty(vol_bh); > + unlock_buffer(vol_bh); > + } > + > + /* Update sector's vol_label */ > + boot_bh = sb_bread(sb, 0); > + if (boot_bh == NULL) { > + fat_msg(sb, KERN_ERR, > + "unable to read boot sector to write volume label"); > + err = -EIO; > + goto out_boot_brelse; > + } > + > + b = (struct fat_boot_sector *)boot_bh->b_data; > + lock_buffer(boot_bh); No need lock_buffer()? > + if (sbi->fat_bits == 32) > + memcpy(b->fat32.vol_label, label, sizeof(label)); > + else > + memcpy(b->fat16.vol_label, label, sizeof(label)); > + > + mark_buffer_dirty(boot_bh); > + unlock_buffer(boot_bh); -- OGAWA Hirofumi <hirof...@mail.parknet.co.jp>