[PATCH v2 0/3] fs/efs: Follow kernel style guide

[PATCH v2 0/3] fs/efs: Follow kernel style guide

[Amy Parker]

As the EFS driver is old and non-maintained, many kernel style guide
rules have not been followed, and their violations have not been
noticed. This patchset corrects those violations.

v2:
 - Corrected commit message line breaking

v1:
 - Fixed brace styling
 - Corrected C keyword spacing
 - Corrected line breakage for C keywords

Amy Parker (3):
  fs/efs: Use correct brace styling for statements
  fs/efs: Correct spacing after C keywords
  fs/efs: Fix line breakage for C keywords

 fs/efs/inode.c | 36 ++++++++++++++++++++++--------------
 fs/efs/namei.c |  2 +-
 fs/efs/super.c | 25 +++++++++++--------------
 3 files changed, 34 insertions(+), 29 deletions(-)

-- 
2.29.2

[PATCH v2 1/3] fs/efs: Use correct brace styling for statements

[Amy Parker]

Many single-line statements have unnecessary braces, and some statement 
pairs have mismatched braces. This is a clear violation of the kernel 
style guide, which mandates that single line statements have no braces 
and that pairs with at least one multi-line block maintain their braces.

This patch fixes these style violations. Single-line statements that 
have braces have had their braces stripped. Pair single-line statements 
have been formatted per the style guide. Pair mixed-line statements have 
had their braces updated to conform.

Signed-off-by: Amy Parker <enbyamy@gmail.com>
---
 fs/efs/inode.c | 10 ++++++----
 fs/efs/super.c | 15 ++++++---------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/fs/efs/inode.c b/fs/efs/inode.c
index 89e73a6f0d36..36d6c45046e2 100644
--- a/fs/efs/inode.c
+++ b/fs/efs/inode.c
@@ -107,11 +107,11 @@ struct inode *efs_iget(struct super_block *super, unsigned long ino)
 	inode->i_atime.tv_nsec = inode->i_mtime.tv_nsec = inode->i_ctime.tv_nsec = 0;
 
 	/* this is the number of blocks in the file */
-	if (inode->i_size == 0) {
+	if (inode->i_size == 0)
 		inode->i_blocks = 0;
-	} else {
+	else
 		inode->i_blocks = ((inode->i_size - 1) >> EFS_BLOCKSIZE_BITS) + 1;
-	}
+
 
 	rdev = be16_to_cpu(efs_inode->di_u.di_dev.odev);
 	if (rdev == 0xffff) {
@@ -120,8 +120,10 @@ struct inode *efs_iget(struct super_block *super, unsigned long ino)
 			device = 0;
 		else
 			device = MKDEV(sysv_major(rdev), sysv_minor(rdev));
-	} else
+	}
+	else {
 		device = old_decode_dev(rdev);
+	}
 
 	/* get the number of extents for this object */
 	in->numextents = be16_to_cpu(efs_inode->di_numextents);
diff --git a/fs/efs/super.c b/fs/efs/super.c
index 62b155b9366b..874d82096b2f 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -160,14 +160,13 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 	struct pt_types	*pt_entry;
 	int		pt_type, slice = -1;
 
-	if (be32_to_cpu(vh->vh_magic) != VHMAGIC) {
+	if (be32_to_cpu(vh->vh_magic) != VHMAGIC)
 		/*
 		 * assume that we're dealing with a partition and allow
 		 * read_super() to try and detect a valid superblock
 		 * on the next block.
 		 */
 		return 0;
-	}
 
 	ui = ((__be32 *) (vh + 1)) - 1;
 	for(csum = 0; ui >= ((__be32 *) vh);) {
@@ -191,11 +190,11 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 		}
 		name[j] = (char) 0;
 
-		if (name[0]) {
+		if (name[0])
 			pr_debug("vh: %8s block: 0x%08x size: 0x%08x\n",
 				name, (int) be32_to_cpu(vh->vh_vd[i].vd_lbn),
 				(int) be32_to_cpu(vh->vh_vd[i].vd_nbytes));
-		}
+
 	}
 #endif
 
@@ -219,15 +218,14 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 		}
 	}
 
-	if (slice == -1) {
+	if (slice == -1)
 		pr_notice("partition table contained no EFS partitions\n");
 #ifdef DEBUG
-	} else {
+	else
 		pr_info("using slice %d (type %s, offset 0x%x)\n", slice,
 			(pt_entry->pt_name) ? pt_entry->pt_name : "unknown",
 			sblock);
 #endif
-	}
 	return sblock;
 }
 
@@ -284,9 +282,8 @@ 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) {
+	if (sb->fs_start == -1)
 		return -EINVAL;
-	}
 
 	bh = sb_bread(s, sb->fs_start + EFS_SUPER);
 	if (!bh) {
-- 
2.29.2

[PATCH v2 2/3] fs/efs: Correct spacing after C keywords

[Amy Parker]

In EFS code, some C keywords (most commonly 'for') do not have spaces 
before their instructions, such as for() vs for (). The kernel style 
guide indicates that these should be of the latter variant. This patch 
updates them accordingly.

Signed-off-by: Amy Parker <enbyamy@gmail.com>
---
 fs/efs/inode.c |  8 ++++----
 fs/efs/namei.c |  2 +-
 fs/efs/super.c | 10 +++++-----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/fs/efs/inode.c b/fs/efs/inode.c
index 36d6c45046e2..2cc55d514421 100644
--- a/fs/efs/inode.c
+++ b/fs/efs/inode.c
@@ -130,7 +130,7 @@ struct inode *efs_iget(struct super_block *super, unsigned long ino)
 	in->lastextent = 0;
 
 	/* copy the extents contained within the inode to memory */
-	for(i = 0; i < EFS_DIRECTEXTENTS; i++) {
+	for (i = 0; i < EFS_DIRECTEXTENTS; i++) {
 		extent_copy(&(efs_inode->di_u.di_extents[i]), &(in->extents[i]));
 		if (i < in->numextents && in->extents[i].cooked.ex_magic != 0) {
 			pr_warn("extent %d has bad magic number in inode %lu\n",
@@ -227,7 +227,7 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 		 * check the stored extents in the inode
 		 * start with next extent and check forwards
 		 */
-		for(dirext = 1; dirext < direxts; dirext++) {
+		for (dirext = 1; dirext < direxts; dirext++) {
 			cur = (last + dirext) % in->numextents;
 			if ((result = efs_extent_check(&in->extents[cur], block, sb))) {
 				in->lastextent = cur;
@@ -244,7 +244,7 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 	direxts = in->extents[0].cooked.ex_offset;
 	indexts = in->numextents;
 
-	for(indext = 0; indext < indexts; indext++) {
+	for (indext = 0; indext < indexts; indext++) {
 		cur = (last + indext) % indexts;
 
 		/*
@@ -255,7 +255,7 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 		 *
 		 */
 		ibase = 0;
-		for(dirext = 0; cur < ibase && dirext < direxts; dirext++) {
+		for (dirext = 0; cur < ibase && dirext < direxts; dirext++) {
 			ibase += in->extents[dirext].cooked.ex_length *
 				(EFS_BLOCKSIZE / sizeof(efs_extent));
 		}
diff --git a/fs/efs/namei.c b/fs/efs/namei.c
index 38961ee1d1af..65d9c7f4d0c0 100644
--- a/fs/efs/namei.c
+++ b/fs/efs/namei.c
@@ -28,7 +28,7 @@ static efs_ino_t efs_find_entry(struct inode *inode, const char *name, int len)
 		pr_warn("%s(): directory size not a multiple of EFS_DIRBSIZE\n",
 			__func__);
 
-	for(block = 0; block < inode->i_blocks; block++) {
+	for (block = 0; block < inode->i_blocks; block++) {
 
 		bh = sb_bread(inode->i_sb, efs_bmap(inode, block));
 		if (!bh) {
diff --git a/fs/efs/super.c b/fs/efs/super.c
index 874d82096b2f..dd97a071f971 100644
--- a/fs/efs/super.c
+++ b/fs/efs/super.c
@@ -169,7 +169,7 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 		return 0;
 
 	ui = ((__be32 *) (vh + 1)) - 1;
-	for(csum = 0; ui >= ((__be32 *) vh);) {
+	for (csum = 0; ui >= ((__be32 *) vh);) {
 		cs = *ui--;
 		csum += be32_to_cpu(cs);
 	}
@@ -181,11 +181,11 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 #ifdef DEBUG
 	pr_debug("bf: \"%16s\"\n", vh->vh_bootfile);
 
-	for(i = 0; i < NVDIR; i++) {
+	for (i = 0; i < NVDIR; i++) {
 		int	j;
 		char	name[VDNAMESIZE+1];
 
-		for(j = 0; j < VDNAMESIZE; j++) {
+		for (j = 0; j < VDNAMESIZE; j++) {
 			name[j] = vh->vh_vd[i].vd_name[j];
 		}
 		name[j] = (char) 0;
@@ -198,9 +198,9 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
 	}
 #endif
 
-	for(i = 0; i < NPARTAB; i++) {
+	for (i = 0; i < NPARTAB; i++) {
 		pt_type = (int) be32_to_cpu(vh->vh_pt[i].pt_type);
-		for(pt_entry = sgi_pt_types; pt_entry->pt_name; pt_entry++) {
+		for (pt_entry = sgi_pt_types; pt_entry->pt_name; pt_entry++) {
 			if (pt_type == pt_entry->pt_type) break;
 		}
 #ifdef DEBUG
-- 
2.29.2

[PATCH v2 3/3] fs/efs: Fix line breakage for C keywords

[Amy Parker]

Some statements - such as if statements - are not broken into their 
lines correctly. For example, some are expressed on a single line. 
Single line if statements are expressely prohibited by the style guide. 
This patch corrects these violations.

Signed-off-by: Amy Parker <enbyamy@gmail.com>
---
 fs/efs/inode.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/fs/efs/inode.c b/fs/efs/inode.c
index 2cc55d514421..0099e6ad529a 100644
--- a/fs/efs/inode.c
+++ b/fs/efs/inode.c
@@ -193,7 +193,8 @@ efs_extent_check(efs_extent *ptr, efs_block_t block, struct efs_sb_info *sb) {
 
 	if ((block >= offset) && (block < offset+length)) {
 		return(sb->fs_start + start + block - offset);
-	} else {
+	}
+	else {
 		return 0;
 	}
 }
@@ -264,7 +265,8 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 			/* should never happen */
 			pr_err("couldn't find direct extent for indirect extent %d (block %u)\n",
 			       cur, block);
-			if (bh) brelse(bh);
+			if (bh)
+				brelse(bh);
 			return 0;
 		}
 		
@@ -276,7 +278,8 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 			(EFS_BLOCKSIZE / sizeof(efs_extent));
 
 		if (first || lastblock != iblock) {
-			if (bh) brelse(bh);
+			if (bh)
+				brelse(bh);
 
 			bh = sb_bread(inode->i_sb, iblock);
 			if (!bh) {
@@ -297,17 +300,20 @@ efs_block_t efs_map_block(struct inode *inode, efs_block_t block) {
 		if (ext.cooked.ex_magic != 0) {
 			pr_err("extent %d has bad magic number in block %d\n",
 			       cur, iblock);
-			if (bh) brelse(bh);
+			if (bh)
+				brelse(bh);
 			return 0;
 		}
 
 		if ((result = efs_extent_check(&ext, block, sb))) {
-			if (bh) brelse(bh);
+			if (bh)
+				brelse(bh);
 			in->lastextent = cur;
 			return result;
 		}
 	}
-	if (bh) brelse(bh);
+	if (bh)
+		brelse(bh);
 	pr_err("%s() failed to map block %u (indir)\n", __func__, block);
 	return 0;
 }  
-- 
2.29.2

Re: [PATCH v2 1/3] fs/efs: Use correct brace styling for statements

[Joe Perches]

On Thu, 2021-02-04 at 21:14 -0800, Amy Parker wrote:
> Many single-line statements have unnecessary braces, and some statement 
> pairs have mismatched braces. This is a clear violation of the kernel 
> style guide, which mandates that single line statements have no braces 
> and that pairs with at least one multi-line block maintain their braces.
> 
> This patch fixes these style violations. Single-line statements that 
> have braces have had their braces stripped. Pair single-line statements 
> have been formatted per the style guide. Pair mixed-line statements have 
> had their braces updated to conform.
> 
> Signed-off-by: Amy Parker <enbyamy@gmail.com>
> ---
>  fs/efs/inode.c | 10 ++++++----
>  fs/efs/super.c | 15 ++++++---------
>  2 files changed, 12 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/efs/inode.c b/fs/efs/inode.c
> @@ -120,8 +120,10 @@ struct inode *efs_iget(struct super_block *super, unsigned long ino)
>  			device = 0;
>  		else
>  			device = MKDEV(sysv_major(rdev), sysv_minor(rdev));
> -	} else
> +	}
> +	else {

Not the kernel specified style.

	} else {

Try using checkpatch on your proposed patches.

Re: [PATCH v2 2/3] fs/efs: Correct spacing after C keywords

[Joe Perches]

On Thu, 2021-02-04 at 21:14 -0800, Amy Parker wrote:
> In EFS code, some C keywords (most commonly 'for') do not have spaces 
> before their instructions, such as for() vs for (). The kernel style 
> guide indicates that these should be of the latter variant. This patch 
> updates them accordingly.

ok but:

> diff --git a/fs/efs/super.c b/fs/efs/super.c
[]
> @@ -169,7 +169,7 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
>  		return 0;
>  
> 
>  	ui = ((__be32 *) (vh + 1)) - 1;
> -	for(csum = 0; ui >= ((__be32 *) vh);) {
> +	for (csum = 0; ui >= ((__be32 *) vh);) {
>  		cs = *ui--;
>  		csum += be32_to_cpu(cs);
>  	}

I think this loop is atypical.  More common would be:

	csum = 0;
	for (ui = ((__be32 *)(vh + 1)) - 1; ui >= (__be32 *)vh; ui--)
		csum += be32_to_cpu(*ui);


> @@ -198,9 +198,9 @@ static efs_block_t efs_validate_vh(struct volume_header *vh) {
>  	}
>  #endif
>  
> 
> -	for(i = 0; i < NPARTAB; i++) {
> +	for (i = 0; i < NPARTAB; i++) {
>  		pt_type = (int) be32_to_cpu(vh->vh_pt[i].pt_type);
> -		for(pt_entry = sgi_pt_types; pt_entry->pt_name; pt_entry++) {
> +		for (pt_entry = sgi_pt_types; pt_entry->pt_name; pt_entry++) {
>  			if (pt_type == pt_entry->pt_type) break;

Also atypical is the break location, it should be on a separate line.