[PATCH 4/6] fs: fat: update parent dirs metadata on rename

[Gabriel D'Alimonte]

On Wed, Jan 22, 2025 at 4:00 AM Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 22.01.25 06:32, Gabriel Dalimonte wrote:
> > None of the existing fat code appears to update parent dir properties
> > however, the POSIX specification for rename() mentions that the
> > last modified timestamp should be updated. In addition, I believe
> > the ATTR_ARCH attribute should also be reset on the parent directories
> > as the content of those directories has changed.
>
> Could you, please, provide a link to the referenced specification.
>
> E.g. https://pubs.opengroup.org/onlinepubs/9799919799/functions/rename.html
>
> Here I find:
> "Upon successful completion, rename() shall mark for update the last
> data modification and last file status change timestamps of the parent
> directory of each file."

That was the line I was referring to in the message. My apologies for
not including a direct link in the commit message.

>
> Does this mean that if we move a directory we would have to update the
> fields of all moved non-empty sub-directories?

I interpreted that to mean only the immediate parent directories of
`old_path` and `new_path` are to be updated. With the rationale for
updating the timestamp and archive attribute on the moved file coming
from the application usage section stating:
"Some implementations mark for update the last file status change
timestamp of renamed files and some do not."

>
> On many boards we don't have a real time clock. Should we skip updating
> if we have no RTC?

That is a good question. I think skipping timestamp updates in the
absence of an RTC makes sense rather than having a timestamp jump
backwards by (likely) several years.

Thanks,

Gabriel

>
> If we want to update parent directories, we should do this for file
> creation, too. The update function could be called in the function where
> we create directory entries.
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Gabriel Dalimonte <gabriel.dalimonte at gmail.com>
> > ---
> >
> >   fs/fat/fat_write.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 42 insertions(+)
> >
> > diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c
> > index f9f7051e30..2559e38d5b 100644
> > --- a/fs/fat/fat_write.c
> > +++ b/fs/fat/fat_write.c
> > @@ -1912,6 +1912,42 @@ exit:
> >       return ret;
> >   }
> >
> > +/**
> > + * update_parent_dir_props - updates the modified time and resets the archive
> > + * attribute for the parent directory
> > + *
> > + * @itr:     iterator positioned anywhere in a directory whose parent should be
> > + *   updated
> > + * @Return:  0 for success
> > + */
> > +static int update_parent_dir_props(fat_itr *itr)
> > +{
> > +     int ret = 0;
> > +     __u32 target_clust = itr->start_clust;
> > +     /* For START macro */
> > +     fsdata *mydata = itr->fsdata;
> > +
> > +     if (!itr->is_root) {
> > +             ret = fat_itr_parent(itr);
> > +             if (ret)
> > +                     return ret;
> > +
> > +             while (fat_itr_next(itr)) {
> > +                     if (START(itr->dent) == target_clust)
> > +                             goto update;
> > +             }
> > +
> > +             /* dent not found */
> > +             return -EIO;
> > +update:
> > +             dentry_set_time(itr->dent);
> > +             itr->dent->attr |= ATTR_ARCH;
> > +             ret = flush_dir(itr);
> > +     }
> > +
> > +     return ret;
> > +}
> > +
> >   /**
> >    * fat_rename - rename/move a file or directory
> >    *
> > @@ -2086,7 +2122,12 @@ int fat_rename(const char *old_path, const char *new_path)
> >               ret = flush_dir(new_itr);
> >               if (ret)
> >                       goto exit;
> > +             /* restore directory location to update parent props below */
> > +             fat_itr_child(new_itr, new_itr);
> >       }
> > +     ret = update_parent_dir_props(new_itr);
> > +     if (ret)
> > +             goto exit;
> >
> >       /* refresh old in case write happened to the same block. */
> >       ret = fat_move_to_cluster(old_itr, old_itr->dent_clust);
> > @@ -2097,6 +2138,7 @@ int fat_rename(const char *old_path, const char *new_path)
> >       if (ret)
> >               goto exit;
> >
> > +     ret = update_parent_dir_props(old_itr);
> >   exit:
> >       free(new_datablock.fatbuf);
> >       free(old_datablock.fatbuf);
>