+Cc: Pali, who AFAIRC is interested in FAT labeling mess. On Wed, Jan 17, 2018 at 12:43 PM, ChenGuanqiao <chen.chencha...@foxmail.com> wrote:
Commit message? > Signed-off-by: ChenGuanqiao <chen.chencha...@foxmail.com> > #include <linux/fsnotify.h> > #include <linux/security.h> > #include <linux/falloc.h> > +#include <linux/ctype.h> It would be better to squeeze it to have order (to some extent) preserved. > +static int fat_check_d_characters(char *label, unsigned long len) > +{ > + int i; > + > + for (i = 0; i < len; ++i) { Oy vey, unsigned long len vs. int i. > + switch (label[i]) { > + case 'a' ... 'z': > + label[i] = __toupper(label[i]); > + case 'A' ... 'Z': > + case '0' ... '9': > + case '_': > + case 0x20: > + continue; I'm not sure this is best approach. And since there is no commit message I can't be constructive. > + default: > + return -EINVAL; > + } > + } > + > + return 0; > +} > +static int fat_ioctl_get_volume_label(struct inode *inode, > + u8 __user *vol_label) > +{ > + int err = 0; Redundant assignment. > + struct buffer_head *bh; > + struct msdos_dir_entry *de; > + struct super_block *sb = inode->i_sb; Moreover, perhaps reversed tree order for the definition block? > + > + inode = d_inode(sb->s_root); > + inode_lock_shared(inode); > + > + err = fat_get_volume_label_entry(inode, &bh, &de); > + if (err) > + goto out; > + > + if (copy_to_user(vol_label, de->name, MSDOS_NAME)) > + err = -EFAULT; > + > + brelse(bh); > +out: > + inode_unlock_shared(inode); > + > + return err; > +} > + > +static int fat_ioctl_set_volume_label(struct file *file, > + u8 __user *vol_label) Perhaps vol_label -> label, and fit one line? > +{ > + int err = 0; > + u8 label[MSDOS_NAME]; > + struct timespec ts; > + 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); > + > + inode = d_inode(sb->s_root); > + > + if (copy_from_user(label, vol_label, sizeof(label))) { > + err = -EFAULT; > + goto out; > + } > + > + err = fat_check_d_characters(label, sizeof(label)); > + if (err) > + goto out; > + > + err = mnt_want_write_file(file); > + if (err) > + goto out; > + > + down_write(&sb->s_umount); > + > + /* Updates root directory's vol_label */ > + err = fat_get_volume_label_entry(inode, &vol_bh, &de); > + ts = current_time(inode); > + if (err) { > + /* Create volume label entry */ > + err = fat_add_volume_label_entry(inode, label, &ts); > + if (err) > + goto out_vol_brelse; > + } else { > + /* Write to root directory */ > + memcpy(de->name, label, sizeof(de->name)); > + inode->i_ctime = inode->i_mtime = inode->i_atime = ts; > + > + mark_buffer_dirty(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; > + 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); > + > + /* Synchronize the data together */ > + err = sync_dirty_buffer(boot_bh); > + if (err) > + goto out_boot_brelse; > + > +out_boot_brelse: > + brelse(boot_bh); > +out_vol_brelse: > + brelse(vol_bh); > + > + up_write(&sb->s_umount); > + mnt_drop_write_file(file); > +out: > + return err; > +} -- With Best Regards, Andy Shevchenko