Re: [PATCH v2 5/6] exfat: introduce exfat_chain_advance helper

[Chi Zhiling]

On 4/3/26 11:33 AM, Yuezhang.Mo@xxxxxxxx wrote:
> > On 4/3/26 10:34 AM, Yuezhang.Mo@xxxxxxxx wrote:
> > > > From: Chi Zhiling <chizhiling@xxxxxxxxxx>
> > > >
> > > > Introduce exfat_chain_advance() to walk a exfat_chain structure by a
> > > > given step, updating both ->dir and ->size fields atomically. This
> > > > helper handles both ALLOC_NO_FAT_CHAIN and ALLOC_FAT_CHAIN modes with
> > > > proper boundary checking.
> > > >
> > > > Signed-off-by: Chi Zhiling <chizhiling@xxxxxxxxxx>
> > > > Tested-by: syzbot@xxxxxxxxxxxxxxxxxxxxxxxxx
> > > >
> > > > ---
> > > > fs/exfat/exfat_fs.h | 17 +++++++++++++++++
> > > >    1 file changed, 17 insertions(+)
> > > >
> > > > diff --git a/fs/exfat/exfat_fs.h b/fs/exfat/exfat_fs.h
> > > > index 530459ab9acc..1035d20ba563 100644
> > > > --- a/fs/exfat/exfat_fs.h
> > > > +++ b/fs/exfat/exfat_fs.h
> > > > @@ -552,6 +552,23 @@ int exfat_read_volume_label(struct super_block *sb,
> > > > int exfat_write_volume_label(struct super_block *sb,
> > > >                                struct exfat_uni_name *label);
> > > >
> > > > +static inline int exfat_chain_advance(struct super_block *sb,
> > > > +               struct exfat_chain *chain, unsigned int step)
> > > > +{
> > > > +       if (chain->size >= step)
> > > > +               chain->size -= step;
> > >
> > > It would be better to not change *chain if return an error.
> >
> > Okay,
> >
> > v3:
> >
> > static inline int exfat_chain_advance(struct super_block *sb,
> >                   struct exfat_chain *chain, unsigned int step)
> > {
> >           if (chain->size < step)
> >                   return -EIO;
> >
> >           chain->size -= step;
> >
> >           if (exfat_fat_walk(sb, &chain->dir, step, chain->flags))
> >                   return -EIO;
> >
> >           if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
> >                   chain->dir = EXFAT_EOF_CLUSTER;
> >
> >           return 0;
> > }
>
> chain->dir should also not be changed.
>
> static inline int exfat_chain_advance(struct super_block *sb,
>                   struct exfat_chain *chain, unsigned int step)
> {
>           unsigned int clu = chain->dir;
>
>           if (chain->size < step)
>                   return -EIO;
>
>           if (exfat_fat_walk(sb, &clu, step, chain->flags))
>                   return -EIO;
>
>           chain->size -= step;
>
>           if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
>                   chain->dir = EXFAT_EOF_CLUSTER;
>           else
>                   chain->dir = clu;
>
>           return 0;
> }

Alright, I got it, this is great.

I’ll update it in v3.


Thanks,

> > Thanks,
> > > > +       else
> > > > +               return -EIO;
> > > > +
> > > > +       if (exfat_fat_walk(sb, &chain->dir, step, chain->flags))
> > > > +               return -EIO;
> > > > +
> > > > +       if (chain->size == 0 && chain->flags == ALLOC_NO_FAT_CHAIN)
> > > > +               chain->dir = EXFAT_EOF_CLUSTER;
> > > > +
> > > > +       return 0;
> > > > +}
> > > > +
> > > >    /* inode.c */
> > > >    extern const struct inode_operations exfat_file_inode_operations;
> > > >    void exfat_sync_inode(struct inode *inode);
> > > > --
> > > > 2.43.0