> …
> > +++ b/fs/exfat/super.c
> …
> > +static int exfat_show_options(struct seq_file *m, struct dentry *root)
> > +{
> …
> > + seq_printf(m, ",fmask=%04o", opts->fs_fmask);
> > + seq_printf(m, ",dmask=%04o", opts->fs_dmask);
>
> How do you think about to combine these two function calls into a single one?
>
>
> > +static int __exfat_fill_super(struct super_block *sb)
> > +{
> …
> > + exfat_msg(sb, KERN_ERR, "unable to read boot sector");
> > + ret = -EIO;
> > + goto out;
> …
>
> Would you like to simplify this place?
>
> + return -EIO;
>
>
> …
> > + exfat_msg(sb, KERN_ERR, "failed to load upcase table");
> > + goto out;
>
> Would you like to omit this label?
>
> + return ret;
>
>
> > +static int exfat_fill_super(struct super_block *sb, struct fs_context *fc)
> > +{
> …
> > + exfat_msg(sb, KERN_ERR, "failed to recognize exfat type");
> > + goto failed_mount;
>
> The local variable “root_inode” contains still a null pointer at this place.
>
> * Thus I would find a jump target like “reset_s_root” more appropriate.
>
> * Can the corresponding pointer initialisation be omitted then?
>
>
> …
> > +failed_mount:
> > + if (root_inode)
> > + iput(root_inode);
> …
>
> I am informed in the way that this function tolerates the passing
> of null pointers.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/i
> node.c?id=1d4c79ed324ad780cfc3ad38364ba1fd585dd2a8#n1567
> https://protect2.fireeye.com/url?k=34e5568f-697957ef-34e4ddc0-0cc47a31307c-
> 7f9b30869a6ffaa4&u=https://elixir.bootlin.com/linux/v5.4-
> rc7/source/fs/inode.c#L1567
>
> Thus I suggest to omit the extra pointer check also at this place.
>
>
> > +static int __init init_exfat_fs(void)
> > +{
> …
> + err = exfat_cache_init();
> + if (err)
> + goto error;
>
> Can it be nicer to return directly?
>
>
> …
> > + if (!exfat_inode_cachep)
> > + goto error;
>
> Can an other jump target like “shutdown_cache” be more appropriate?
>
>
> > + err = register_filesystem(&exfat_fs_type);
> > + if (err)
> > + goto error;
> …
>
> Can the label “destroy_cache” be more appropriate?
>
>
I checked your all points, Will fix them on V2.
Thanks for your review!
> Regards,
> Markus
