Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Sungjong Seo" <sj1557.seo@samsung.com>
To:     "'Rover'" <739817562@qq.com>,
        "'linkinjeon'" <linkinjeon@kernel.org>
Cc:     "'linux-kernel'" <linux-kernel@vger.kernel.org>,
        "'linux-fsdevel'" <linux-fsdevel@vger.kernel.org>,
        <sj1557.seo@samsung.com>
In-Reply-To: <tencent_52978C540CBFA2747B715C811F0D99530E07@qq.com>
Subject: RE: RE: [PATCH v1] exfat: remove the code that sets FileAttributes
 when renaming
Date:   Wed, 14 Sep 2022 19:22:43 +0900
Message-ID: <08bb01d8c823$ed1e0dd0$c75a2970$@samsung.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Thread-Index: AQLiLS1ckQU2fx+m2qoL2gnRLP1zpQJkLg6Wq7jCpzA=
Content-Language: ko
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 101P
DLP-Filter: Pass
References: <CGME20220914092543epcas1p1526e62ff4587f70d09bcefab3a97cbe2@epcas1p1.samsung.com>
        <tencent_52978C540CBFA2747B715C811F0D99530E07@qq.com>
Precedence: bulk

> Hi Sungjong,
> 
> This patch is to remove the duplicate setting ATTR_ARCHIVE when renaming.
> 
> We know that a file will not become a directory after being renamed, and a
> directory will not become a file after being renamed.

Sure, rename-op does not change any types of files.

> &gt;&gt;
> &gt;&gt; 		*epnew = *epold;
> So ATTR_ARCHIVE is already set here if rename file.

Do you think ATTR_ARCHIVE represents the file type? If so, you probably
are misunderstanding ATTR_ARCHIVE. It's just like a flag indicating that
there was a change. For example, in case of rename, it's just like a flag
indicating that its name has been changed.

The current linux exfat implementation does not yet provide an IOCTL
to control this attribute, but keep in mind that the ATTR_ARCHIVE may
be removed even if it is a file via another exfat driver.

B.R.
Sungjong Seo

> 
> &gt;&gt; -		if (exfat_get_entry_type(epnew) == TYPE_FILE) {
> &gt;&gt; -			epnew-&gt;dentry.file.attr |=
> cpu_to_le16(ATTR_ARCHIVE);
> No need to set again here.
> 
> &gt;&gt; -			ei-&gt;attr |= ATTR_ARCHIVE;
> ei-&gt;attr doesn't change and already set ATTR_ARCHIVE in
> exfat_fill_inode()
> 
> &gt;&gt; -		}
> 
> Best regards,
> Rover
> 
> ------------------ Original ------------------
> From: "Sungjong Seo" <sj1557.seo@samsung.com>;
> Date: Wed, Sep 14, 2022 04:48 PM
> To: "Rover"&lt;739817562@qq.com&gt;;"'linkinjeon'"<linkinjeon@kernel.org>;
> Cc: "'linux-kernel'"<linux-kernel@vger.kernel.org>;"'linux-
> fsdevel'"<linux-
> fsdevel@vger.kernel.org>;"sj1557.seo"<sj1557.seo@samsung.com>;
> Subject: RE: [PATCH v1] exfat: remove the code that sets FileAttributes
> when renaming
> 
> 
> Hi, Rover,
> 
> This patch seems to violate the exFAT specification below.
> Please refer to the description for ATTR_ARCHIVE in FAT32 Spec.
> 
> * Archive
> This field is mandatory and conforms to the MS-DOS definition.
> 
> * ATTR_ARCHIVE
> This attribute supports backup utilities. This bit is set by the FAT file
> system driver when a file is created, renamed, or written to. Backup
> utilities may use this attribute to indicate which files on the volume
> have been modified since the last time that a backup was performed.
> 
> Thanks.
> B.R.
> 
> Sungjong Seo
> 
> &gt; When renaming, FileAttributes remain unchanged, do not need to be
&gt;
> set, so the code that sets FileAttributes is unneeded, remove it.
> &gt;
> &gt; Signed-off-by: rover &amp;lt;739817562@qq.com&amp;gt; &gt; ---
> &gt;&nbsp; fs/exfat/namei.c | 12 ------------ &gt;&nbsp; 1 file changed,
> 12 deletions(-) &gt; &gt; diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
> &gt; index b617bebc3d0f..5ffaf553155e 100644 &gt; --- a/fs/exfat/namei.c
> &gt; +++ b/fs/exfat/namei.c &gt; @@ -1031,10 +1031,6 @@ static int
> exfat_rename_file(struct inode *inode, &gt; struct exfat_chain *p_dir,
> &gt;&nbsp;return -EIO; &gt; &gt;&nbsp; *epnew = *epold; &gt; -if
> (exfat_get_entry_type(epnew) == TYPE_FILE) { &gt; - epnew-
> &amp;gt;dentry.file.attr |= &gt; cpu_to_le16(ATTR_ARCHIVE); &gt; - ei-
> &amp;gt;attr |= ATTR_ARCHIVE; &gt; -} &gt;&nbsp; exfat_update_bh(new_bh,
> sync); &gt;&nbsp; brelse(old_bh); &gt;&nbsp; brelse(new_bh); &gt; @@ -
> 1063,10 +1059,6 @@ static int exfat_rename_file(struct inode *inode, &gt;
> struct exfat_chain *p_dir, &gt;&nbsp; ei-&amp;gt;dir = *p_dir; &gt;&nbsp;
> ei-&amp;gt;entry = newentry; &gt;&nbsp;} else { &gt; -if
> (exfat_get_entry_type(epold) == TYPE_FILE) { &gt; - epold-
> &amp;gt;dentry.file.attr |= &gt; cpu_to_le16(ATTR_ARCHIVE); &gt; - ei-
> &amp;gt;attr |= ATTR_ARCHIVE; &gt; -} &gt;&nbsp; exfat_update_bh(old_bh,
> sync); &gt;&nbsp; brelse(old_bh); &gt;&nbsp; ret =
> exfat_init_ext_entry(inode, p_dir, oldentry, &gt; @@ -1112,10 +1104,6 @@
> static int exfat_move_file(struct inode *inode, &gt; struct exfat_chain
> *p_olddir, &gt;&nbsp; return -EIO; &gt; &gt;&nbsp;*epnew = *epmov; &gt; -
> if (exfat_get_entry_type(epnew) == TYPE_FILE) { &gt; -epnew-
> &amp;gt;dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE); &gt; -ei-
> &amp;gt;attr |= ATTR_ARCHIVE; &gt; - } &gt;&nbsp;exfat_update_bh(new_bh,
> IS_DIRSYNC(inode)); &gt;&nbsp;brelse(mov_bh); &gt;&nbsp;brelse(new_bh);
> &gt; -- &gt; 2.25.1</sj1557.seo@samsung.com></linux-
> fsdevel@vger.kernel.org></linux-
> kernel@vger.kernel.org></linkinjeon@kernel.org></sj1557.seo@samsung.com>