[PATCH v2 0/9] bfs: assorted cleanups

[PATCH v2 0/9] bfs: assorted cleanups

[Dmitri Vorobiev]

Hi Adrian,

Please drop the previous version of the BFS patches I sent
to the trivial patches yesterday. I am now sending a new
version, where part #5 has been redone addressing the
comments I received.

Please note that the driver maintainer (I'm Cc:in him now)
has approved the changes in a private email to me:

> If you don't mind --- please proceed the way you were,
> but you may add whatever is the appropriate keyword --- 
> "Approved-by:" or "CC'd to:" or whatever they do nowadays
> to indicate that the official maintainer has seen and not
> objected to the patches.
>
> Kind regards
> Tigran

Thanks,

Dmitri

[PATCH v2 1/9] bfs: remove a useless variable

[Dmitri Vorobiev]

In the bfs_fill_super() routine, the "sblock" variable is declared
and assigned a value. However, this value is never used. This trivial
patch removes this useless variable.

This was compile-tested by building the bfs driver both as a module
and as a part of the kernel proper. Both build finished successfully.

Run time test was performed using a BFS image and verifying that the
filesystem remained functional after the change.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/inode.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index a64a71d..2284657 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -368,7 +368,7 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
 		struct bfs_inode *di;
 		int block = (i - BFS_ROOT_INO) / BFS_INODES_PER_BLOCK + 1;
 		int off = (i - BFS_ROOT_INO) % BFS_INODES_PER_BLOCK;
-		unsigned long sblock, eblock;
+		unsigned long eblock;
 
 		if (!off) {
 			brelse(bh);
@@ -387,7 +387,6 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
 		set_bit(i, info->si_imap);
 		info->si_freeb -= BFS_FILEBLOCKS(di);
 
-		sblock =  le32_to_cpu(di->i_sblock);
 		eblock =  le32_to_cpu(di->i_eblock);
 		if (eblock > info->si_lf_eblk)
 			info->si_lf_eblk = eblock;
-- 
1.5.3

[PATCH v2 2/9] bfs: coding style cleanup in fs/bfs/inode.c

[Dmitri Vorobiev]

This patch cleans up errors found by checkpatch.pl.

Before the patch:

$ ./scripts/checkpatch.pl --file fs/bfs/inode.c  | grep total
total: 11 errors, 0 warnings, 445 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file fs/bfs/inode.c  | grep total
total: 0 errors, 0 warnings, 446 lines checked

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/inode.c |   23 ++++++++++++-----------
 1 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 2284657..5191990 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -90,12 +90,12 @@ static void bfs_read_inode(struct inode *inode)
 static int bfs_write_inode(struct inode *inode, int unused)
 {
 	unsigned int ino = (u16)inode->i_ino;
-        unsigned long i_sblock;
+	unsigned long i_sblock;
 	struct bfs_inode *di;
 	struct buffer_head *bh;
 	int block, off;
 
-        dprintf("ino=%08x\n", ino);
+	dprintf("ino=%08x\n", ino);
 
 	if ((ino < BFS_ROOT_INO) || (ino > BFS_SB(inode->i_sb)->si_lasti)) {
 		printf("Bad inode number %s:%08x\n", inode->i_sb->s_id, ino);
@@ -128,7 +128,7 @@ static int bfs_write_inode(struct inode *inode, int unused)
 	di->i_atime = cpu_to_le32(inode->i_atime.tv_sec);
 	di->i_mtime = cpu_to_le32(inode->i_mtime.tv_sec);
 	di->i_ctime = cpu_to_le32(inode->i_ctime.tv_sec);
-        i_sblock = BFS_I(inode)->i_sblock;
+	i_sblock = BFS_I(inode)->i_sblock;
 	di->i_sblock = cpu_to_le32(i_sblock);
 	di->i_eblock = cpu_to_le32(BFS_I(inode)->i_eblock);
 	di->i_eoffset = cpu_to_le32(i_sblock * BFS_BSIZE + inode->i_size - 1);
@@ -157,7 +157,7 @@ static void bfs_delete_inode(struct inode *inode)
 		printf("invalid ino=%08lx\n", ino);
 		return;
 	}
-	
+
 	inode->i_size = 0;
 	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
 	lock_kernel();
@@ -177,13 +177,13 @@ static void bfs_delete_inode(struct inode *inode)
 	mark_buffer_dirty(bh);
 	brelse(bh);
 
-        if (bi->i_dsk_ino) {
+	if (bi->i_dsk_ino) {
 		if (bi->i_sblock)
 			info->si_freeb += bi->i_eblock + 1 - bi->i_sblock;
 		info->si_freei++;
 		clear_bit(ino, info->si_imap);
 		dump_imap("delete_inode", s);
-        }
+	}
 
 	/*
 	 * If this was the last file, make the previous block
@@ -293,7 +293,8 @@ void dump_imap(const char *prefix, struct super_block *s)
 	if (!tmpbuf)
 		return;
 	for (i = BFS_SB(s)->si_lasti; i >= 0; i--) {
-		if (i > PAGE_SIZE - 100) break;
+		if (i > PAGE_SIZE - 100)
+			break;
 		if (test_bit(i, BFS_SB(s)->si_imap))
 			strcat(tmpbuf, "1");
 		else
@@ -321,12 +322,12 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
 	sb_set_blocksize(s, BFS_BSIZE);
 
 	bh = sb_bread(s, 0);
-	if(!bh)
+	if (!bh)
 		goto out;
 	bfs_sb = (struct bfs_super_block *)bh->b_data;
 	if (le32_to_cpu(bfs_sb->s_magic) != BFS_MAGIC) {
 		if (!silent)
-			printf("No BFS filesystem on %s (magic=%08x)\n", 
+			printf("No BFS filesystem on %s (magic=%08x)\n",
 				s->s_id,  le32_to_cpu(bfs_sb->s_magic));
 		goto out;
 	}
@@ -395,7 +396,7 @@ static int bfs_fill_super(struct super_block *s, void *data, int silent)
 	if (!(s->s_flags & MS_RDONLY)) {
 		mark_buffer_dirty(info->si_sbh);
 		s->s_dirt = 1;
-	} 
+	}
 	dump_imap("read_super", s);
 	return 0;
 
@@ -425,7 +426,7 @@ static int __init init_bfs_fs(void)
 	int err = init_inodecache();
 	if (err)
 		goto out1;
-        err = register_filesystem(&bfs_fs_type);
+	err = register_filesystem(&bfs_fs_type);
 	if (err)
 		goto out;
 	return 0;
-- 
1.5.3

[PATCH v2 3/9] bfs: coding style cleanup in fs/bfs/bfs.h

[Dmitri Vorobiev]

Clean up errors found by checkpatch.pl.

Before the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/bfs.h | grep total
total: 2 errors, 0 warnings, 57 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/bfs.h | grep total
total: 0 errors, 0 warnings, 57 lines checked

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/bfs.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/bfs/bfs.h b/fs/bfs/bfs.h
index ac7a8b1..090b96e 100644
--- a/fs/bfs/bfs.h
+++ b/fs/bfs/bfs.h
@@ -16,8 +16,8 @@ struct bfs_sb_info {
 	unsigned long si_freei;
 	unsigned long si_lf_eblk;
 	unsigned long si_lasti;
-	unsigned long * si_imap;
-	struct buffer_head * si_sbh;		/* buffer header w/superblock */
+	unsigned long *si_imap;
+	struct buffer_head *si_sbh;		/* buffer header w/superblock */
 };
 
 /*
-- 
1.5.3

[PATCH v2 4/9] bfs: coding style cleanup in fs/bfs/dir.c

[Dmitri Vorobiev]

Clean up errors found by checkpatch.pl.

Before the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/dir.c | grep total
total: 7 errors, 1 warnings, 370 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/dir.c | grep total
total: 0 errors, 1 warnings, 370 lines checked

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/dir.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 1fd056d..5462a5b 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -72,7 +72,7 @@ static int bfs_readdir(struct file *f, void *dirent, filldir_t filldir)
 	}
 
 	unlock_kernel();
-	return 0;	
+	return 0;
 }
 
 const struct file_operations bfs_dir_operations = {
@@ -117,7 +117,7 @@ static int bfs_create(struct inode *dir, struct dentry *dentry, int mode,
 	BFS_I(inode)->i_sblock = 0;
 	BFS_I(inode)->i_eblock = 0;
 	insert_inode_hash(inode);
-        mark_inode_dirty(inode);
+	mark_inode_dirty(inode);
 	dump_imap("create", s);
 
 	err = bfs_add_entry(dir, dentry->d_name.name, dentry->d_name.len,
@@ -228,8 +228,8 @@ static int bfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		return -EINVAL;
 
 	lock_kernel();
-	old_bh = bfs_find_entry(old_dir, 
-				old_dentry->d_name.name, 
+	old_bh = bfs_find_entry(old_dir,
+				old_dentry->d_name.name,
 				old_dentry->d_name.len, &old_de);
 
 	if (!old_bh || (le16_to_cpu(old_de->ino) != old_inode->i_ino))
@@ -237,8 +237,8 @@ static int bfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 
 	error = -EPERM;
 	new_inode = new_dentry->d_inode;
-	new_bh = bfs_find_entry(new_dir, 
-				new_dentry->d_name.name, 
+	new_bh = bfs_find_entry(new_dir,
+				new_dentry->d_name.name,
 				new_dentry->d_name.len, &new_de);
 
 	if (new_bh && !new_inode) {
@@ -246,7 +246,7 @@ static int bfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 		new_bh = NULL;
 	}
 	if (!new_bh) {
-		error = bfs_add_entry(new_dir, 
+		error = bfs_add_entry(new_dir,
 					new_dentry->d_name.name,
 					new_dentry->d_name.len,
 					old_inode->i_ino);
-- 
1.5.3

[PATCH v2 5/9] bfs: move function prototype to the proper header file

[Dmitri Vorobiev]

The dump_imap() routine is defined in bs/bfs/inode.c and used both in
the same file and in fs/bfs/dir.c. This patch adds an extern function
declaration to the private bfs.h header file.

The effect is that one warning issued by checkpatch.pl is gone.

Before the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/dir.c | grep total
total: 0 errors, 1 warnings, 370 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/dir.c | grep total
total: 0 errors, 0 warnings, 368 lines checked

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/bfs.h   |    3 +++
 fs/bfs/dir.c   |    2 --
 fs/bfs/inode.c |    2 --
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/bfs/bfs.h b/fs/bfs/bfs.h
index 090b96e..ecc74bb 100644
--- a/fs/bfs/bfs.h
+++ b/fs/bfs/bfs.h
@@ -54,4 +54,7 @@ extern const struct address_space_operations bfs_aops;
 extern const struct inode_operations bfs_dir_inops;
 extern const struct file_operations bfs_dir_operations;
 
+/* inode.c */
+void dump_imap(const char *, struct super_block *);
+
 #endif /* _FS_BFS_BFS_H */
diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 5462a5b..2964505 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -81,8 +81,6 @@ const struct file_operations bfs_dir_operations = {
 	.fsync		= file_fsync,
 };
 
-extern void dump_imap(const char *, struct super_block *);
-
 static int bfs_create(struct inode *dir, struct dentry *dentry, int mode,
 						struct nameidata *nd)
 {
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 5191990..91d5686 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -30,8 +30,6 @@ MODULE_LICENSE("GPL");
 #define dprintf(x...)
 #endif
 
-void dump_imap(const char *prefix, struct super_block *s);
-
 static void bfs_read_inode(struct inode *inode)
 {
 	unsigned long ino = inode->i_ino;
-- 
1.5.3

[PATCH v2 6/9] bfs: coding style cleanup in fs/bfs/file.c

[Dmitri Vorobiev]

Clean up errors found by checkpatch.pl.

Before the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/file.c | grep total
total: 6 errors, 0 warnings, 191 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file  fs/bfs/file.c | grep total
total: 0 errors, 0 warnings, 191 lines checked

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/file.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index b11e63e..f32b2a2 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -55,7 +55,7 @@ static int bfs_move_blocks(struct super_block *sb, unsigned long start,
 
 	dprintf("%08lx-%08lx->%08lx\n", start, end, where);
 	for (i = start; i <= end; i++)
-		if(bfs_move_block(i, where + i, sb)) {
+		if (bfs_move_block(i, where + i, sb)) {
 			dprintf("failed to move block %08lx -> %08lx\n", i,
 								where + i);
 			return -EIO;
@@ -77,7 +77,7 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 	if (!create) {
 		if (phys <= bi->i_eblock) {
 			dprintf("c=%d, b=%08lx, phys=%09lx (granted)\n",
-                                create, (unsigned long)block, phys);
+					create, (unsigned long)block, phys);
 			map_bh(bh_result, sb, phys);
 		}
 		return 0;
@@ -88,7 +88,7 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 	 * range of blocks allocated for this file, we can grant it.
 	 */
 	if (bi->i_sblock && (phys <= bi->i_eblock)) {
-		dprintf("c=%d, b=%08lx, phys=%08lx (interim block granted)\n", 
+		dprintf("c=%d, b=%08lx, phys=%08lx (interim block granted)\n",
 				create, (unsigned long)block, phys);
 		map_bh(bh_result, sb, phys);
 		return 0;
@@ -107,7 +107,7 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 	 * anywhere.
 	 */
 	if (bi->i_eblock == info->si_lf_eblk) {
-		dprintf("c=%d, b=%08lx, phys=%08lx (simple extension)\n", 
+		dprintf("c=%d, b=%08lx, phys=%08lx (simple extension)\n",
 				create, (unsigned long)block, phys);
 		map_bh(bh_result, sb, phys);
 		info->si_freeb -= phys - bi->i_eblock;
@@ -126,7 +126,7 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 	}
 
 	if (bi->i_sblock) {
-		err = bfs_move_blocks(inode->i_sb, bi->i_sblock, 
+		err = bfs_move_blocks(inode->i_sb, bi->i_sblock,
 						bi->i_eblock, phys);
 		if (err) {
 			dprintf("failed to move ino=%08lx -> fs corruption\n",
@@ -137,7 +137,7 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 		err = 0;
 
 	dprintf("c=%d, b=%08lx, phys=%08lx (moved)\n",
-                create, (unsigned long)block, phys);
+			create, (unsigned long)block, phys);
 	bi->i_sblock = phys;
 	phys += block;
 	info->si_lf_eblk = bi->i_eblock = phys;
-- 
1.5.3

[PATCH v2 7/9] bfs: coding style cleanup in include/linux/bfs_fs.h

[Dmitri Vorobiev]

Clean up errors found by checkpatch.pl.

Before the patch:

$ ./scripts/checkpatch.pl --file  include/linux/bfs_fs.h | grep total
total: 5 errors, 3 warnings, 80 lines checked

After the patch:

$ ./scripts/checkpatch.pl --file  include/linux/bfs_fs.h | grep total
total: 0 errors, 0 warnings, 83 lines checked

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 include/linux/bfs_fs.h |   17 ++++++++++-------
 1 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/include/linux/bfs_fs.h b/include/linux/bfs_fs.h
index 8ed6dfd..d7b11a6 100644
--- a/include/linux/bfs_fs.h
+++ b/include/linux/bfs_fs.h
@@ -36,7 +36,7 @@ struct bfs_inode {
 	__u32 i_padding[4];
 };
 
-#define BFS_NAMELEN		14	
+#define BFS_NAMELEN		14
 #define BFS_DIRENT_SIZE		16
 #define BFS_DIRS_PER_BLOCK	32
 
@@ -61,20 +61,23 @@ struct bfs_super_block {
 
 
 #define BFS_OFF2INO(offset) \
-        ((((offset) - BFS_BSIZE) / sizeof(struct bfs_inode)) + BFS_ROOT_INO)
+	((((offset) - BFS_BSIZE) / sizeof(struct bfs_inode)) + BFS_ROOT_INO)
 
 #define BFS_INO2OFF(ino) \
 	((__u32)(((ino) - BFS_ROOT_INO) * sizeof(struct bfs_inode)) + BFS_BSIZE)
 #define BFS_NZFILESIZE(ip) \
-        ((le32_to_cpu((ip)->i_eoffset) + 1) -  le32_to_cpu((ip)->i_sblock) * BFS_BSIZE)
+	((le32_to_cpu((ip)->i_eoffset) + 1) - \
+	 le32_to_cpu((ip)->i_sblock) * BFS_BSIZE)
 
 #define BFS_FILESIZE(ip) \
-        ((ip)->i_sblock == 0 ? 0 : BFS_NZFILESIZE(ip))
+	((ip)->i_sblock == 0 ? 0 : BFS_NZFILESIZE(ip))
 
 #define BFS_FILEBLOCKS(ip) \
-        ((ip)->i_sblock == 0 ? 0 : (le32_to_cpu((ip)->i_eblock) + 1) -  le32_to_cpu((ip)->i_sblock))
+	((ip)->i_sblock == 0 ? 0 : (le32_to_cpu((ip)->i_eblock) + 1) - \
+	 le32_to_cpu((ip)->i_sblock))
 #define BFS_UNCLEAN(bfs_sb, sb)	\
-	((le32_to_cpu(bfs_sb->s_from) != -1) && (le32_to_cpu(bfs_sb->s_to) != -1) && !(sb->s_flags & MS_RDONLY))
-
+	((le32_to_cpu(bfs_sb->s_from) != -1) && \
+	(le32_to_cpu(bfs_sb->s_to) != -1) && \
+	!(sb->s_flags & MS_RDONLY))
 
 #endif	/* _LINUX_BFS_FS_H */
-- 
1.5.3

[PATCH v2 8/9] bfs: remove multiple assignments

[Dmitri Vorobiev]

The checkpatch.pl reported several warnings about multiple variable
assignments in the BFS driver sources. This trivial patch fixes these
warnings by giving each assignment its own line.

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/dir.c   |   13 +++++++++----
 fs/bfs/file.c  |    6 ++++--
 fs/bfs/inode.c |    7 +++++--
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/fs/bfs/dir.c b/fs/bfs/dir.c
index 2964505..94fed7a 100644
--- a/fs/bfs/dir.c
+++ b/fs/bfs/dir.c
@@ -104,7 +104,9 @@ static int bfs_create(struct inode *dir, struct dentry *dentry, int mode,
 	info->si_freei--;
 	inode->i_uid = current->fsuid;
 	inode->i_gid = (dir->i_mode & S_ISGID) ? dir->i_gid : current->fsgid;
-	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_mtime = CURRENT_TIME_SEC;
+	inode->i_atime = CURRENT_TIME_SEC;
+	inode->i_ctime = CURRENT_TIME_SEC;
 	inode->i_blocks = 0;
 	inode->i_op = &bfs_file_inops;
 	inode->i_fop = &bfs_file_operations;
@@ -200,7 +202,8 @@ static int bfs_unlink(struct inode *dir, struct dentry *dentry)
 	}
 	de->ino = 0;
 	mark_buffer_dirty(bh);
-	dir->i_ctime = dir->i_mtime = CURRENT_TIME_SEC;
+	dir->i_ctime = CURRENT_TIME_SEC;
+	dir->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(dir);
 	inode->i_ctime = dir->i_ctime;
 	inode_dec_link_count(inode);
@@ -220,7 +223,8 @@ static int bfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 	struct bfs_dirent *old_de, *new_de;
 	int error = -ENOENT;
 
-	old_bh = new_bh = NULL;
+	old_bh = NULL;
+	new_bh = NULL;
 	old_inode = old_dentry->d_inode;
 	if (S_ISDIR(old_inode->i_mode))
 		return -EINVAL;
@@ -252,7 +256,8 @@ static int bfs_rename(struct inode *old_dir, struct dentry *old_dentry,
 			goto end_rename;
 	}
 	old_de->ino = 0;
-	old_dir->i_ctime = old_dir->i_mtime = CURRENT_TIME_SEC;
+	old_dir->i_ctime = CURRENT_TIME_SEC;
+	old_dir->i_mtime = CURRENT_TIME_SEC;
 	mark_inode_dirty(old_dir);
 	if (new_inode) {
 		new_inode->i_ctime = CURRENT_TIME_SEC;
diff --git a/fs/bfs/file.c b/fs/bfs/file.c
index f32b2a2..ab2fa66 100644
--- a/fs/bfs/file.c
+++ b/fs/bfs/file.c
@@ -111,7 +111,8 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 				create, (unsigned long)block, phys);
 		map_bh(bh_result, sb, phys);
 		info->si_freeb -= phys - bi->i_eblock;
-		info->si_lf_eblk = bi->i_eblock = phys;
+		info->si_lf_eblk = phys;
+		bi->i_eblock = phys;
 		mark_inode_dirty(inode);
 		mark_buffer_dirty(sbh);
 		err = 0;
@@ -140,7 +141,8 @@ static int bfs_get_block(struct inode *inode, sector_t block,
 			create, (unsigned long)block, phys);
 	bi->i_sblock = phys;
 	phys += block;
-	info->si_lf_eblk = bi->i_eblock = phys;
+	info->si_lf_eblk = phys;
+	bi->i_eblock = phys;
 
 	/*
 	 * This assumes nothing can write the inode back while we are here
diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 91d5686..7eefafb 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -157,7 +157,9 @@ static void bfs_delete_inode(struct inode *inode)
 	}
 
 	inode->i_size = 0;
-	inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
+	inode->i_atime = CURRENT_TIME_SEC;
+	inode->i_mtime = CURRENT_TIME_SEC;
+	inode->i_ctime = CURRENT_TIME_SEC;
 	lock_kernel();
 	mark_inode_dirty(inode);
 
@@ -213,7 +215,8 @@ static int bfs_statfs(struct dentry *dentry, struct kstatfs *buf)
 	buf->f_type = BFS_MAGIC;
 	buf->f_bsize = s->s_blocksize;
 	buf->f_blocks = info->si_blocks;
-	buf->f_bfree = buf->f_bavail = info->si_freeb;
+	buf->f_bfree = info->si_freeb;
+	buf->f_bavail = info->si_freeb;
 	buf->f_files = info->si_lasti + 1 - BFS_ROOT_INO;
 	buf->f_ffree = info->si_freei;
 	buf->f_fsid.val[0] = (u32)id;
-- 
1.5.3

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Tigran Aivazian]

On Sat, 26 Jan 2008, Dmitri Vorobiev wrote:
> -	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
> +	inode->i_mtime = CURRENT_TIME_SEC;
> +	inode->i_atime = CURRENT_TIME_SEC;
> +	inode->i_ctime = CURRENT_TIME_SEC;

multiple assignments like "x = y = z = value;" can potentially (depending 
on the compiler and arch) be faster than "x = value; y = value; z=value;"

I am surprized that this script complains about them as it is a perfectly 
valid thing to do in C.

But this is not a "big deal", so I don't mind.

Kind regards
Tigran

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Dmitri Vorobiev]

Tigran Aivazian wrote:
> On Sat, 26 Jan 2008, Dmitri Vorobiev wrote:
>> -    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>> +    inode->i_mtime = CURRENT_TIME_SEC;
>> +    inode->i_atime = CURRENT_TIME_SEC;
>> +    inode->i_ctime = CURRENT_TIME_SEC;
> 
> multiple assignments like "x = y = z = value;" can potentially
> (depending on the compiler and arch) be faster than "x = value; y =
> value; z=value;"
> 
> I am surprized that this script complains about them as it is a
> perfectly valid thing to do in C.
>

Thanks for your comment.

My goal was to make the driver "checkpatch.pl-clean", and that was the
only reason for this change. I just assumed that what the script, which is
recommended for use to check the kernel coding style, insists upon,
gives an idea of what "the right way" is.

>From the viewpoint of code clarity, it seems to be a matter of personal
opinion whether multiple assignments should be preferred over giving a
separate line to each variable. I personally prefer separated assignments,
but in this particular case I would never touch these lines had the
script not complained about it.
 
> But this is not a "big deal", so I don't mind.
> 

OK. Let's wait for Adrian to pick up these patches and push these into
his tree.

Thanks,

Dmitri

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Dmitri Vorobiev]

Tigran Aivazian wrote:
> On Sat, 26 Jan 2008, Dmitri Vorobiev wrote:
>> -    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>> +    inode->i_mtime = CURRENT_TIME_SEC;
>> +    inode->i_atime = CURRENT_TIME_SEC;
>> +    inode->i_ctime = CURRENT_TIME_SEC;
> 
> multiple assignments like "x = y = z = value;" can potentially
> (depending on the compiler and arch) be faster than "x = value; y =
> value; z=value;"
> 
> I am surprized that this script complains about them as it is a
> perfectly valid thing to do in C.

I think it seems wise to ask the maintainers of checkpatch.pl to
comment on that. I'm Cc:ing them now.

Thanks,

Dmitri

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Joel Schopp]

>>> -    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>>> +    inode->i_mtime = CURRENT_TIME_SEC;
>>> +    inode->i_atime = CURRENT_TIME_SEC;
>>> +    inode->i_ctime = CURRENT_TIME_SEC;
>> multiple assignments like "x = y = z = value;" can potentially
>> (depending on the compiler and arch) be faster than "x = value; y =
>> value; z=value;"
>>
>> I am surprized that this script complains about them as it is a
>> perfectly valid thing to do in C.
> 
> I think it seems wise to ask the maintainers of checkpatch.pl to
> comment on that. I'm Cc:ing them now.
> 

There are plenty of things that are valid to do in C that don't make for 
maintainable code.  These scripts are designed to make your code easier for 
real people to review and maintain.

As for if this can be faster we don't deal in the realm of "can".  Please 
show a concrete example of gcc making Linux kernel code faster with 
multiple assignments per line.  If you can do that I'm willing to change my 
mind and I'll lead the charge for mutliple assignments per line.

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Al Viro]

On Mon, Jan 28, 2008 at 01:02:03AM -0600, Joel Schopp wrote:
> >>>-    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
> >>>+    inode->i_mtime = CURRENT_TIME_SEC;
> >>>+    inode->i_atime = CURRENT_TIME_SEC;
> >>>+    inode->i_ctime = CURRENT_TIME_SEC;
> >>multiple assignments like "x = y = z = value;" can potentially
> >>(depending on the compiler and arch) be faster than "x = value; y =
> >>value; z=value;"
> >>
> >>I am surprized that this script complains about them as it is a
> >>perfectly valid thing to do in C.
> >
> >I think it seems wise to ask the maintainers of checkpatch.pl to
> >comment on that. I'm Cc:ing them now.
> >
> 
> There are plenty of things that are valid to do in C that don't make for 
> maintainable code.  These scripts are designed to make your code easier for 
> real people to review and maintain.

Except that in this case the new variant is not equivalent to the old one...

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Dmitri Vorobiev]

Al Viro wrote:
> On Mon, Jan 28, 2008 at 01:02:03AM -0600, Joel Schopp wrote:
>>>>> -    inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>>>>> +    inode->i_mtime = CURRENT_TIME_SEC;
>>>>> +    inode->i_atime = CURRENT_TIME_SEC;
>>>>> +    inode->i_ctime = CURRENT_TIME_SEC;
>>>> multiple assignments like "x = y = z = value;" can potentially
>>>> (depending on the compiler and arch) be faster than "x = value; y =
>>>> value; z=value;"
>>>>
>>>> I am surprized that this script complains about them as it is a
>>>> perfectly valid thing to do in C.
>>> I think it seems wise to ask the maintainers of checkpatch.pl to
>>> comment on that. I'm Cc:ing them now.
>>>
>> There are plenty of things that are valid to do in C that don't make for 
>> maintainable code.  These scripts are designed to make your code easier for 
>> real people to review and maintain.
> 
> Except that in this case the new variant is not equivalent to the old one...

Yes, you're right. In fact, I felt like sending yet another version
of these patches, but this gets preempted all the time by "the other things".

Dmitri

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Adrian Bunk]

On Sat, Jan 26, 2008 at 06:35:41PM +0000, Tigran Aivazian wrote:
> On Sat, 26 Jan 2008, Dmitri Vorobiev wrote:
>> -	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>> +	inode->i_mtime = CURRENT_TIME_SEC;
>> +	inode->i_atime = CURRENT_TIME_SEC;
>> +	inode->i_ctime = CURRENT_TIME_SEC;
>
> multiple assignments like "x = y = z = value;" can potentially (depending 
> on the compiler and arch) be faster than "x = value; y = value; z=value;"

Only depending on the compiler, and recent gcc versions are quite good 
at optimizing code.

> I am surprized that this script complains about them as it is a perfectly 
> valid thing to do in C.

Checkpatch warns about the something already documented in 
Documentation/CodingStyle:

<--  snip  -->

Don't put multiple assignments on a single line either.  Kernel coding style
is super simple.  Avoid tricky expressions.

<--  snip  -->

Nobody claims it wasn't perfectly valid C code, the point is that 
multiple assignments on a single line are harder to read.

> But this is not a "big deal", so I don't mind.
>
> Kind regards
> Tigran

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [PATCH v2 8/9] bfs: remove multiple assignments

[Dmitri Vorobiev]

Adrian Bunk пишет:
> On Sat, Jan 26, 2008 at 06:35:41PM +0000, Tigran Aivazian wrote:
>> On Sat, 26 Jan 2008, Dmitri Vorobiev wrote:
>>> -	inode->i_mtime = inode->i_atime = inode->i_ctime = CURRENT_TIME_SEC;
>>> +	inode->i_mtime = CURRENT_TIME_SEC;
>>> +	inode->i_atime = CURRENT_TIME_SEC;
>>> +	inode->i_ctime = CURRENT_TIME_SEC;
>> multiple assignments like "x = y = z = value;" can potentially (depending 
>> on the compiler and arch) be faster than "x = value; y = value; z=value;"
> 
> Only depending on the compiler, and recent gcc versions are quite good 
> at optimizing code.
> 
>> I am surprized that this script complains about them as it is a perfectly 
>> valid thing to do in C.
> 
> Checkpatch warns about the something already documented in 
> Documentation/CodingStyle:
> 
> <--  snip  -->
> 
> Don't put multiple assignments on a single line either.  Kernel coding style
> is super simple.  Avoid tricky expressions.
> 
> <--  snip  -->
> 
> Nobody claims it wasn't perfectly valid C code, the point is that 
> multiple assignments on a single line are harder to read.

Thanks for your explanation. So, it was just a matter of readability,
not language lawyering. In fact, this is how I interpreted that complaint
from the checkpatch.pl script.

Dmitri

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 9/9] bfs: use the proper header file for inclusion

[Dmitri Vorobiev]

The checkpatch.pl reported the following warning:

$ ./scripts/checkpatch.pl --strict --file fs/bfs/inode.c
CHECK: Use #include <linux/uaccess.h> instead of <asm/uaccess.h>
+#include <asm/uaccess.h>

This patch fixes this warning.

No functional changes introduced.

This patch was compile-tested by building the BFS driver both
as a module and as a part of the kernel proper.

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Acked-by: Tigran Aivazian <tigran@aivazian.fsnet.co.uk>
---
 fs/bfs/inode.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/fs/bfs/inode.c b/fs/bfs/inode.c
index 7eefafb..00c54fa 100644
--- a/fs/bfs/inode.c
+++ b/fs/bfs/inode.c
@@ -15,7 +15,7 @@
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <linux/vfs.h>
-#include <asm/uaccess.h>
+#include <linux/uaccess.h>
 #include "bfs.h"
 
 MODULE_AUTHOR("Tigran Aivazian <tigran@aivazian.fsnet.co.uk>");
-- 
1.5.3