Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754176AbeAIHvZ (ORCPT + 1 other); Tue, 9 Jan 2018 02:51:25 -0500 Received: from mail.parknet.co.jp ([210.171.160.6]:36066 "EHLO mail.parknet.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751465AbeAIHvX (ORCPT ); Tue, 9 Jan 2018 02:51:23 -0500 From: OGAWA Hirofumi To: ChenGuanqiao Cc: linux-kernel@vger.kernel.org Subject: Re: [PATCH v6 2/3] fs: fat: Add volume label entry method function References: <20180102064229.25202-1-chen.chenchacha@foxmail.com> <20180102064229.25202-3-chen.chenchacha@foxmail.com> Date: Tue, 09 Jan 2018 16:51:20 +0900 In-Reply-To: <20180102064229.25202-3-chen.chenchacha@foxmail.com> (ChenGuanqiao's message of "Tue, 2 Jan 2018 14:42:28 +0800") Message-ID: <871sizfp1j.fsf@mail.parknet.co.jp> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.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 Return-Path: ChenGuanqiao writes: > +static int fat_get_volume_label_entry(struct inode *dir, loff_t *pos, > + struct buffer_head **bh, > + struct msdos_dir_entry **de) > +{ > + while (fat_get_entry(dir, pos, bh, de) >= 0) { > + if (((*de)->attr & ATTR_VOLUME) && (*de)->attr != ATTR_EXT) It should check IS_FREE(). > + return 0; > + } > + return -ENOENT; > +} > + > +int fat_scan_volume_label(struct inode *dir, struct buffer_head **bh, > + struct msdos_dir_entry **de) > +{ > + loff_t offset = 0; > + > + if (dir->i_ino != MSDOS_ROOT_INO) > + return -EINVAL; > + > + *bh = NULL; > + *de = NULL; > + while (fat_get_volume_label_entry(dir, &offset, bh, de) >= 0) > + return 0; > + > + return -ENOENT; > +} > +EXPORT_SYMBOL_GPL(fat_scan_volume_label); No need this wrapper? Looks like better to merge into fat_get_volume_label_entry(). > +int fat_add_volume_label_entry(struct inode *dir, const unsigned char *name, > + struct timespec *ts) > +{ > + struct msdos_sb_info *sbi = MSDOS_SB(dir->i_sb); > + struct msdos_dir_entry de; > + struct fat_slot_info sinfo; > + __le16 time, date; > + int err; > + > + if (dir->i_ino != MSDOS_ROOT_INO) > + return -EINVAL; > + > + memcpy(de.name, name, MSDOS_NAME); > + de.attr = ATTR_VOLUME; > + de.lcase = 0; > + fat_time_unix2fat(sbi, ts, &time, &date, NULL); > + de.cdate = de.adate = 0; > + de.ctime = 0; > + de.ctime_cs = 0; > + de.time = time; > + de.date = date; > + fat_set_start(&de, 0); > + de.size = 0; > + > + err = fat_add_entries(dir, &de, 1, &sinfo); > + if (err) > + return err; Missing call of brelse(sinfo.bh) > + dir->i_ctime = dir->i_mtime = *ts; Probably, we should not update this for labal. > + return fat_sync_inode(dir); No inode for root. -- OGAWA Hirofumi