lkml.org 
[lkml]   [2019]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 02/13] exfat: add super block operations
From
Date

> +++ 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/inode.c?id=1d4c79ed324ad780cfc3ad38364ba1fd585dd2a8#n1567
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?


Regards,
Markus

\
 
 \ /
  Last update: 2019-11-17 14:33    [W:0.135 / U:1.128 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site