[PATCH 1/5] fs: efs: fix trailing and leading space

[PATCH 1/5] fs: efs: fix trailing and leading space

[Sudip Mukherjee]

fixed the trailing and leading whitespace errors in the code.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---

Hi Andrew,
   since there is no maintainer so sending to you.

 fs/efs/super.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/efs/super.c b/fs/efs/super.c
index 7fca462..c2f105f 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -248,8 +248,8 @@ static int efs_validate_super(struct efs_sb_info *sb, struct efs_super *super) {
 	sb->inode_free   = be32_to_cpu(super->fs_tinode);
 	sb->inode_blocks = be16_to_cpu(super->fs_cgisize);
 	sb->total_groups = be16_to_cpu(super->fs_ncg);
-    
-	return 0;    
+
+	return 0;
 }
 
 static int efs_fill_super(struct super_block *s, void *d, int silent)
@@ -258,18 +258,18 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	struct buffer_head *bh;
 	struct inode *root;
 
- 	sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL);
+	sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL);
 	if (!sb)
 		return -ENOMEM;
 	s->s_fs_info = sb;
- 
+
 	s->s_magic		= EFS_SUPER_MAGIC;
 	if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) {
 		pr_err("device does not support %d byte blocks\n",
 			EFS_BLOCKSIZE);
 		return -EINVAL;
 	}
-  
+
 	/* read the vh (volume header) block */
 	bh = sb_bread(s, 0);
 
@@ -295,7 +295,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 		pr_err("cannot read superblock\n");
 		return -EINVAL;
 	}
-		
+
 	if (efs_validate_super(sb, (struct efs_super *) bh->b_data)) {
 #ifdef DEBUG
 		pr_warn("invalid superblock at block %u\n",
-- 
1.8.1.2

[PATCH 2/5] fs: efs: fix forward declarations

[Sudip Mukherjee]

rearrange the functions to get rid of the forward declarations.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 fs/efs/super.c | 130 ++++++++++++++++++++++++++++-----------------------------
 1 file changed, 65 insertions(+), 65 deletions(-)

diff --git a/fs/efs/super.c b/fs/efs/super.c
index c2f105f..9aaae09 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -17,31 +17,6 @@
 #include <linux/efs_vh.h>
 #include <linux/efs_fs_sb.h>
 
-static int efs_statfs(struct dentry *dentry, struct kstatfs *buf);
-static int efs_fill_super(struct super_block *s, void *d, int silent);
-
-static struct dentry *efs_mount(struct file_system_type *fs_type,
-	int flags, const char *dev_name, void *data)
-{
-	return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super);
-}
-
-static void efs_kill_sb(struct super_block *s)
-{
-	struct efs_sb_info *sbi = SUPER_INFO(s);
-	kill_block_super(s);
-	kfree(sbi);
-}
-
-static struct file_system_type efs_fs_type = {
-	.owner		= THIS_MODULE,
-	.name		= "efs",
-	.mount		= efs_mount,
-	.kill_sb	= efs_kill_sb,
-	.fs_flags	= FS_REQUIRES_DEV,
-};
-MODULE_ALIAS_FS("efs");
-
 static struct pt_types sgi_pt_types[] = {
 	{0x00,		"SGI vh"},
 	{0x01,		"SGI trkrepl"},
@@ -112,6 +87,30 @@ static void destroy_inodecache(void)
 	kmem_cache_destroy(efs_inode_cachep);
 }
 
+static int efs_statfs(struct dentry *dentry, struct kstatfs *buf)
+{
+	struct super_block *sb = dentry->d_sb;
+	struct efs_sb_info *sbi = SUPER_INFO(sb);
+	u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
+
+	buf->f_type    = EFS_SUPER_MAGIC;	/* efs magic number */
+	buf->f_bsize   = EFS_BLOCKSIZE;		/* blocksize */
+	buf->f_blocks  = sbi->total_groups *	/* total data blocks */
+			(sbi->group_size - sbi->inode_blocks);
+	buf->f_bfree   = sbi->data_free;	/* free data blocks */
+	buf->f_bavail  = sbi->data_free;	/* free blocks for non-root */
+	buf->f_files   = sbi->total_groups *	/* total inodes */
+			sbi->inode_blocks *
+			(EFS_BLOCKSIZE / sizeof(struct efs_dinode));
+	buf->f_ffree   = sbi->inode_free;	/* free inodes */
+	buf->f_fsid.val[0] = (u32)id;
+	buf->f_fsid.val[1] = (u32)(id >> 32);
+	buf->f_namelen = EFS_MAXNAMELEN;	/* max filename length */
+
+	return 0;
+}
+
+
 static int efs_remount(struct super_block *sb, int *flags, char *data)
 {
 	sync_filesystem(sb);
@@ -132,29 +131,6 @@ static const struct export_operations efs_export_ops = {
 	.get_parent	= efs_get_parent,
 };
 
-static int __init init_efs_fs(void) {
-	int err;
-	pr_info(EFS_VERSION" - http://aeschi.ch.eu.org/efs/\n");
-	err = init_inodecache();
-	if (err)
-		goto out1;
-	err = register_filesystem(&efs_fs_type);
-	if (err)
-		goto out;
-	return 0;
-out:
-	destroy_inodecache();
-out1:
-	return err;
-}
-
-static void __exit exit_efs_fs(void) {
-	unregister_filesystem(&efs_fs_type);
-	destroy_inodecache();
-}
-
-module_init(init_efs_fs)
-module_exit(exit_efs_fs)
 
 static efs_block_t efs_validate_vh(struct volume_header *vh) {
 	int		i;
@@ -329,25 +305,49 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	return 0;
 }
 
-static int efs_statfs(struct dentry *dentry, struct kstatfs *buf) {
-	struct super_block *sb = dentry->d_sb;
-	struct efs_sb_info *sbi = SUPER_INFO(sb);
-	u64 id = huge_encode_dev(sb->s_bdev->bd_dev);
+static struct dentry *efs_mount(struct file_system_type *fs_type,
+	int flags, const char *dev_name, void *data)
+{
+	return mount_bdev(fs_type, flags, dev_name, data, efs_fill_super);
+}
 
-	buf->f_type    = EFS_SUPER_MAGIC;	/* efs magic number */
-	buf->f_bsize   = EFS_BLOCKSIZE;		/* blocksize */
-	buf->f_blocks  = sbi->total_groups *	/* total data blocks */
-			(sbi->group_size - sbi->inode_blocks);
-	buf->f_bfree   = sbi->data_free;	/* free data blocks */
-	buf->f_bavail  = sbi->data_free;	/* free blocks for non-root */
-	buf->f_files   = sbi->total_groups *	/* total inodes */
-			sbi->inode_blocks *
-			(EFS_BLOCKSIZE / sizeof(struct efs_dinode));
-	buf->f_ffree   = sbi->inode_free;	/* free inodes */
-	buf->f_fsid.val[0] = (u32)id;
-	buf->f_fsid.val[1] = (u32)(id >> 32);
-	buf->f_namelen = EFS_MAXNAMELEN;	/* max filename length */
+static void efs_kill_sb(struct super_block *s)
+{
+	struct efs_sb_info *sbi = SUPER_INFO(s);
+	kill_block_super(s);
+	kfree(sbi);
+}
+
+static struct file_system_type efs_fs_type = {
+	.owner		= THIS_MODULE,
+	.name		= "efs",
+	.mount		= efs_mount,
+	.kill_sb	= efs_kill_sb,
+	.fs_flags	= FS_REQUIRES_DEV,
+};
+MODULE_ALIAS_FS("efs");
 
+
+static int __init init_efs_fs(void) {
+	int err;
+	pr_info(EFS_VERSION" - http://aeschi.ch.eu.org/efs/\n");
+	err = init_inodecache();
+	if (err)
+		goto out1;
+	err = register_filesystem(&efs_fs_type);
+	if (err)
+		goto out;
 	return 0;
+out:
+	destroy_inodecache();
+out1:
+	return err;
 }
 
+static void __exit exit_efs_fs(void) {
+	unregister_filesystem(&efs_fs_type);
+	destroy_inodecache();
+}
+
+module_init(init_efs_fs)
+module_exit(exit_efs_fs)
-- 
1.8.1.2

[PATCH 3/5] fs: efs: remove nonexistant site

[Sudip Mukherjee]

remove the not-working website address from code and Kconfig file.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 fs/efs/Kconfig | 3 +--
 fs/efs/super.c | 2 +-
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/fs/efs/Kconfig b/fs/efs/Kconfig
index d020e3c..d5d9c8a 100644
--- a/fs/efs/Kconfig
+++ b/fs/efs/Kconfig
@@ -7,8 +7,7 @@ config EFS_FS
 	  uses the XFS file system for hard disk partitions however).
 
 	  This implementation only offers read-only access. If you don't know
-	  what all this is about, it's safe to say N. For more information
-	  about EFS see its home page at <http://aeschi.ch.eu.org/efs/>.
+	  what all this is about, it's safe to say N.
 
 	  To compile the EFS file system support as a module, choose M here: the
 	  module will be called efs.
diff --git a/fs/efs/super.c b/fs/efs/super.c
index 9aaae09..6026bd4 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -330,7 +330,7 @@ MODULE_ALIAS_FS("efs");
 
 static int __init init_efs_fs(void) {
 	int err;
-	pr_info(EFS_VERSION" - http://aeschi.ch.eu.org/efs/\n");
+	pr_info(EFS_VERSION "\n");
 	err = init_inodecache();
 	if (err)
 		goto out1;
-- 
1.8.1.2

[PATCH 4/5] fs: efs: fix possible memory leak

[Sudip Mukherjee]

we are allocating memory for struct efs_sb_info, but afterwards when
we are returning with error we are not releasing the memory.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 fs/efs/super.c | 24 +++++++++++++++---------
 1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/fs/efs/super.c b/fs/efs/super.c
index 6026bd4..e1037aa 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -233,6 +233,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	struct efs_sb_info *sb;
 	struct buffer_head *bh;
 	struct inode *root;
+	int ret = -EINVAL;
 
 	sb = kzalloc(sizeof(struct efs_sb_info), GFP_KERNEL);
 	if (!sb)
@@ -243,7 +244,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	if (!sb_set_blocksize(s, EFS_BLOCKSIZE)) {
 		pr_err("device does not support %d byte blocks\n",
 			EFS_BLOCKSIZE);
-		return -EINVAL;
+		goto err_out;
 	}
 
 	/* read the vh (volume header) block */
@@ -251,7 +252,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 
 	if (!bh) {
 		pr_err("cannot read volume header\n");
-		return -EINVAL;
+		goto err_out;
 	}
 
 	/*
@@ -262,14 +263,13 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	sb->fs_start = efs_validate_vh((struct volume_header *) bh->b_data);
 	brelse(bh);
 
-	if (sb->fs_start == -1) {
-		return -EINVAL;
-	}
+	if (sb->fs_start == -1)
+		goto err_out;
 
 	bh = sb_bread(s, sb->fs_start + EFS_SUPER);
 	if (!bh) {
 		pr_err("cannot read superblock\n");
-		return -EINVAL;
+		goto err_out;
 	}
 
 	if (efs_validate_super(sb, (struct efs_super *) bh->b_data)) {
@@ -278,7 +278,7 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 			sb->fs_start + EFS_SUPER);
 #endif
 		brelse(bh);
-		return -EINVAL;
+		goto err_out;
 	}
 	brelse(bh);
 
@@ -293,16 +293,22 @@ static int efs_fill_super(struct super_block *s, void *d, int silent)
 	root = efs_iget(s, EFS_ROOTINODE);
 	if (IS_ERR(root)) {
 		pr_err("get root inode failed\n");
-		return PTR_ERR(root);
+		ret = PTR_ERR(root);
+		goto err_out;
 	}
 
 	s->s_root = d_make_root(root);
 	if (!(s->s_root)) {
 		pr_err("get root dentry failed\n");
-		return -ENOMEM;
+		ret = -ENOMEM;
+		goto err_out;
 	}
 
 	return 0;
+
+err_out:
+	kfree(sb);
+	return ret;
 }
 
 static struct dentry *efs_mount(struct file_system_type *fs_type,
-- 
1.8.1.2

[PATCH 5/5] MAINTAINERS: remove website

[Sudip Mukherjee]

the website mentioned for efs file system does not exist

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
 MAINTAINERS | 1 -
 1 file changed, 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index fb0d901..bd1a5cb 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3731,7 +3731,6 @@ S:	Maintained
 F:	drivers/video/fbdev/efifb.c
 
 EFS FILESYSTEM
-W:	http://aeschi.ch.eu.org/efs/
 S:	Orphan
 F:	fs/efs/
 
-- 
1.8.1.2

Re: [PATCH 4/5] fs: efs: fix possible memory leak

[Al Viro]

On Thu, Feb 12, 2015 at 02:32:21PM +0530, Sudip Mukherjee wrote:
> we are allocating memory for struct efs_sb_info, but afterwards when
> we are returning with error we are not releasing the memory.

The hell we are not - unlike ->put_super(), ->kill_sb() is *always*
called, even when fill_super() fails halfway through.  Exactly because
it makes for simpler cleanup requirements on failure exits in said
fill_super().  And we have
static void efs_kill_sb(struct super_block *s)
{
        struct efs_sb_info *sbi = SUPER_INFO(s);
        kill_block_super(s);
        kfree(sbi);
}
for ->kill_sb() there, so sbi will *not* leak.

NAK.  This patch not only complicates efs_fill_super() for no good reason,
it ends up with double kfree() on those failure exits - ->s_fs_info is
left pointing to freed memory and efs_kill_sb() does kfree() again.

Re: [PATCH 4/5] fs: efs: fix possible memory leak

[Sudip Mukherjee]

On Tue, Feb 17, 2015 at 07:31:01PM +0000, Al Viro wrote:
> On Thu, Feb 12, 2015 at 02:32:21PM +0530, Sudip Mukherjee wrote:
> 
> The hell we are not - unlike ->put_super(), ->kill_sb() is *always*
> called, even when fill_super() fails halfway through.  Exactly because
> it makes for simpler cleanup requirements on failure exits in said
> fill_super().  And we have
> static void efs_kill_sb(struct super_block *s)
> {
>         struct efs_sb_info *sbi = SUPER_INFO(s);
>         kill_block_super(s);
>         kfree(sbi);
> }
> for ->kill_sb() there, so sbi will *not* leak.
thanks for explaining this. I was seeing the code in the fat and was trying to figure out why this in efs was not released.
i was have one more doubt about efs_iget() but that also is actually cleared with this.

thanks
sudip

> 
> NAK.  This patch not only complicates efs_fill_super() for no good reason,
> it ends up with double kfree() on those failure exits - ->s_fs_info is
> left pointing to freed memory and efs_kill_sb() does kfree() again.