RE: RE: [PATCH v1] exfat: remove the code that sets FileAttributes when renaming
From: Sungjong Seo
Date: Wed Sep 14 2022 - 06:23:07 EST
> 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.
> >>
> >> *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
>
> >> - if (exfat_get_entry_type(epnew) == TYPE_FILE) {
> >> - epnew->dentry.file.attr |=
> cpu_to_le16(ATTR_ARCHIVE);
> No need to set again here.
>
> >> - ei->attr |= ATTR_ARCHIVE;
> ei->attr doesn't change and already set ATTR_ARCHIVE in
> exfat_fill_inode()
>
> >> - }
>
> Best regards,
> Rover
>
> ------------------ Original ------------------
> From: "Sungjong Seo" <sj1557.seo@xxxxxxxxxxx>;
> Date: Wed, Sep 14, 2022 04:48 PM
> To: "Rover"<739817562@xxxxxx>;"'linkinjeon'"<linkinjeon@xxxxxxxxxx>;
> Cc: "'linux-kernel'"<linux-kernel@xxxxxxxxxxxxxxx>;"'linux-
> fsdevel'"<linux-
> fsdevel@xxxxxxxxxxxxxxx>;"sj1557.seo"<sj1557.seo@xxxxxxxxxxx>;
> 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
>
> > When renaming, FileAttributes remain unchanged, do not need to be
>
> set, so the code that sets FileAttributes is unneeded, remove it.
> >
> > Signed-off-by: rover &lt;739817562@xxxxxx&gt; > ---
> > fs/exfat/namei.c | 12 ------------ > 1 file changed,
> 12 deletions(-) > > diff --git a/fs/exfat/namei.c b/fs/exfat/namei.c
> > index b617bebc3d0f..5ffaf553155e 100644 > --- a/fs/exfat/namei.c
> > +++ b/fs/exfat/namei.c > @@ -1031,10 +1031,6 @@ static int
> exfat_rename_file(struct inode *inode, > struct exfat_chain *p_dir,
> > return -EIO; > > *epnew = *epold; > -if
> (exfat_get_entry_type(epnew) == TYPE_FILE) { > - epnew-
> &gt;dentry.file.attr |= > cpu_to_le16(ATTR_ARCHIVE); > - ei-
> &gt;attr |= ATTR_ARCHIVE; > -} > exfat_update_bh(new_bh,
> sync); > brelse(old_bh); > brelse(new_bh); > @@ -
> 1063,10 +1059,6 @@ static int exfat_rename_file(struct inode *inode, >
> struct exfat_chain *p_dir, > ei-&gt;dir = *p_dir; >
> ei-&gt;entry = newentry; > } else { > -if
> (exfat_get_entry_type(epold) == TYPE_FILE) { > - epold-
> &gt;dentry.file.attr |= > cpu_to_le16(ATTR_ARCHIVE); > - ei-
> &gt;attr |= ATTR_ARCHIVE; > -} > exfat_update_bh(old_bh,
> sync); > brelse(old_bh); > ret =
> exfat_init_ext_entry(inode, p_dir, oldentry, > @@ -1112,10 +1104,6 @@
> static int exfat_move_file(struct inode *inode, > struct exfat_chain
> *p_olddir, > return -EIO; > > *epnew = *epmov; > -
> if (exfat_get_entry_type(epnew) == TYPE_FILE) { > -epnew-
> &gt;dentry.file.attr |= cpu_to_le16(ATTR_ARCHIVE); > -ei-
> &gt;attr |= ATTR_ARCHIVE; > - } > exfat_update_bh(new_bh,
> IS_DIRSYNC(inode)); > brelse(mov_bh); > brelse(new_bh);
> > -- > 2.25.1</sj1557.seo@xxxxxxxxxxx></linux-
> fsdevel@xxxxxxxxxxxxxxx></linux-
> kernel@xxxxxxxxxxxxxxx></linkinjeon@xxxxxxxxxx></sj1557.seo@xxxxxxxxxxx>