[PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Jesper Juhl]

Whitespace and CodingStyle cleanup for fs/hpfs/dir.c

While reading through fs/hpfs/dir.c I ran across lots of cases of multiple
statements on a single line that made some bits of the file quite confusing
to read. This patch cleans that up and also make sure labels are placed at
column 1, removes trailing whitespace and a few other minor CodingStyle nits.

There are some very long lines in this file that would be a lot more readable 
(IMHO) if broken up to fit within 80 cols. This patch does not do that, but
if wanted I can submit an aditional patch on top of this one to do that.


Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
---

 fs/hpfs/dir.c |   65 +++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 38 insertions(+), 27 deletions(-)

--- linux-2.6.14-rc4-orig/fs/hpfs/dir.c	2005-08-29 01:41:01.000000000 +0200
+++ linux-2.6.14-rc4/fs/hpfs/dir.c	2005-10-12 13:18:33.000000000 +0200
@@ -31,19 +31,23 @@ static loff_t hpfs_dir_lseek(struct file
 	lock_kernel();
 
 	/*printk("dir lseek\n");*/
-	if (new_off == 0 || new_off == 1 || new_off == 11 || new_off == 12 || new_off == 13) goto ok;
+	if (new_off == 0 || new_off == 1 || new_off == 11 || new_off == 12 || new_off == 13)
+		goto ok;
 	down(&i->i_sem);
-	pos = ((loff_t) hpfs_de_as_down_as_possible(s, hpfs_inode->i_dno) << 4) + 1;
+	pos = ((loff_t)hpfs_de_as_down_as_possible(s, hpfs_inode->i_dno) << 4) + 1;
 	while (pos != new_off) {
-		if (map_pos_dirent(i, &pos, &qbh)) hpfs_brelse4(&qbh);
-		else goto fail;
-		if (pos == 12) goto fail;
+		if (map_pos_dirent(i, &pos, &qbh))
+			hpfs_brelse4(&qbh);
+		else
+			goto fail;
+		if (pos == 12)
+			goto fail;
 	}
 	up(&i->i_sem);
-ok:
+ ok:
 	unlock_kernel();
 	return filp->f_pos = new_off;
-fail:
+ fail:
 	up(&i->i_sem);
 	/*printk("illegal lseek: %016llx\n", new_off);*/
 	unlock_kernel();
@@ -78,6 +82,7 @@ static int hpfs_readdir(struct file *fil
 		struct buffer_head *bh;
 		struct fnode *fno;
 		int e = 0;
+
 		if (!(fno = hpfs_map_fnode(inode->i_sb, inode->i_ino, &bh))) {
 			ret = -EIOERROR;
 			goto out;
@@ -105,7 +110,7 @@ static int hpfs_readdir(struct file *fil
 		ret = -ENOENT;
 		goto out;
 	}
-	
+
 	while (1) {
 		again:
 		/* This won't work when cycle is longer than number of dirents
@@ -119,7 +124,7 @@ static int hpfs_readdir(struct file *fil
 		if (filp->f_pos == 12)
 			goto out;
 		if (filp->f_pos == 3 || filp->f_pos == 4 || filp->f_pos == 5) {
-			printk("HPFS: warning: pos==%d\n",(int)filp->f_pos);
+			printk("HPFS: warning: pos==%d\n", (int)filp->f_pos);
 			goto out;
 		}
 		if (filp->f_pos == 0) {
@@ -144,8 +149,10 @@ static int hpfs_readdir(struct file *fil
 		}
 		if (de->first || de->last) {
 			if (hpfs_sb(inode->i_sb)->sb_chk) {
-				if (de->first && !de->last && (de->namelen != 2 || de ->name[0] != 1 || de->name[1] != 1)) hpfs_error(inode->i_sb, "hpfs_readdir: bad ^A^A entry; pos = %08x", old_pos);
-				if (de->last && (de->namelen != 1 || de ->name[0] != 255)) hpfs_error(inode->i_sb, "hpfs_readdir: bad \\377 entry; pos = %08x", old_pos);
+				if (de->first && !de->last && (de->namelen != 2 || de ->name[0] != 1 || de->name[1] != 1))
+					hpfs_error(inode->i_sb, "hpfs_readdir: bad ^A^A entry; pos = %08x", old_pos);
+				if (de->last && (de->namelen != 1 || de ->name[0] != 255))
+					hpfs_error(inode->i_sb, "hpfs_readdir: bad \\377 entry; pos = %08x", old_pos);
 			}
 			hpfs_brelse4(&qbh);
 			goto again;
@@ -153,14 +160,16 @@ static int hpfs_readdir(struct file *fil
 		tempname = hpfs_translate_name(inode->i_sb, de->name, de->namelen, lc, de->not_8x3);
 		if (filldir(dirent, tempname, de->namelen, old_pos, de->fnode, DT_UNKNOWN) < 0) {
 			filp->f_pos = old_pos;
-			if (tempname != (char *)de->name) kfree(tempname);
+			if (tempname != (char *)de->name)
+				kfree(tempname);
 			hpfs_brelse4(&qbh);
 			goto out;
 		}
-		if (tempname != (char *)de->name) kfree(tempname);
+		if (tempname != (char *)de->name)
+			kfree(tempname);
 		hpfs_brelse4(&qbh);
 	}
-out:
+ out:
 	unlock_kernel();
 	return ret;
 }
@@ -204,13 +213,14 @@ struct dentry *hpfs_lookup(struct inode 
 	 * '.' and '..' will never be passed here.
 	 */
 
-	de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *) name, len, NULL, &qbh);
+	de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name, len, NULL, &qbh);
 
 	/*
 	 * This is not really a bailout, just means file not found.
 	 */
 
-	if (!de) goto end;
+	if (!de)
+		goto end;
 
 	/*
 	 * Get inode number, what we're after.
@@ -243,14 +253,16 @@ struct dentry *hpfs_lookup(struct inode 
 		unlock_new_inode(result);
 	}
 	hpfs_result = hpfs_i(result);
-	if (!de->directory) hpfs_result->i_parent_dir = dir->i_ino;
+	if (!de->directory)
+		hpfs_result->i_parent_dir = dir->i_ino;
 
 	hpfs_decide_conv(result, (char *)name, len);
 
-	if (de->has_acl || de->has_xtd_perm) if (!(dir->i_sb->s_flags & MS_RDONLY)) {
-		hpfs_error(result->i_sb, "ACLs or XPERM found. This is probably HPFS386. This driver doesn't support it now. Send me some info on these structures");
-		goto bail1;
-	}
+	if (de->has_acl || de->has_xtd_perm)
+		if (!(dir->i_sb->s_flags & MS_RDONLY)) {
+			hpfs_error(result->i_sb, "ACLs or XPERM found. This is probably HPFS386. This driver doesn't support it now. Send me some info on these structures");
+			goto bail1;
+		}
 
 	/*
 	 * Fill in the info from the directory if this is a newly created
@@ -290,8 +302,8 @@ struct dentry *hpfs_lookup(struct inode 
 	 * Made it.
 	 */
 
-	end:
-	end_add:
+ end:
+ end_add:
 	hpfs_set_dentry_operations(dentry);
 	unlock_kernel();
 	d_add(dentry, result);
@@ -300,11 +312,10 @@ struct dentry *hpfs_lookup(struct inode 
 	/*
 	 * Didn't.
 	 */
-	bail1:
-	
+ bail1:
 	hpfs_brelse4(&qbh);
-	
-	/*bail:*/
+
+ /*bail:*/
 
 	unlock_kernel();
 	return ERR_PTR(-ENOENT);

Re: [PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Mikulas Patocka]

On Wed, 12 Oct 2005, Jesper Juhl wrote:

> Whitespace and CodingStyle cleanup for fs/hpfs/dir.c
>
> While reading through fs/hpfs/dir.c I ran across lots of cases of multiple
> statements on a single line that made some bits of the file quite confusing
> to read. This patch cleans that up and also make sure labels are placed at
> column 1, removes trailing whitespace and a few other minor CodingStyle nits.

I don't see anything wrong with
 	if (some_condition) do_some_action();
statements. Generally, if they consume less lines, you can see more code 
on a screen.

Mikulas

> There are some very long lines in this file that would be a lot more readable
> (IMHO) if broken up to fit within 80 cols. This patch does not do that, but
> if wanted I can submit an aditional patch on top of this one to do that.
>
>
> Signed-off-by: Jesper Juhl <jesper.juhl@gmail.com>
> ---
>
> fs/hpfs/dir.c |   65 +++++++++++++++++++++++++++++++++-------------------------
> 1 files changed, 38 insertions(+), 27 deletions(-)
>
> --- linux-2.6.14-rc4-orig/fs/hpfs/dir.c	2005-08-29 01:41:01.000000000 +0200
> +++ linux-2.6.14-rc4/fs/hpfs/dir.c	2005-10-12 13:18:33.000000000 +0200
> @@ -31,19 +31,23 @@ static loff_t hpfs_dir_lseek(struct file
> 	lock_kernel();
>
> 	/*printk("dir lseek\n");*/
> -	if (new_off == 0 || new_off == 1 || new_off == 11 || new_off == 12 || new_off == 13) goto ok;
> +	if (new_off == 0 || new_off == 1 || new_off == 11 || new_off == 12 || new_off == 13)
> +		goto ok;
> 	down(&i->i_sem);
> -	pos = ((loff_t) hpfs_de_as_down_as_possible(s, hpfs_inode->i_dno) << 4) + 1;
> +	pos = ((loff_t)hpfs_de_as_down_as_possible(s, hpfs_inode->i_dno) << 4) + 1;
> 	while (pos != new_off) {
> -		if (map_pos_dirent(i, &pos, &qbh)) hpfs_brelse4(&qbh);
> -		else goto fail;
> -		if (pos == 12) goto fail;
> +		if (map_pos_dirent(i, &pos, &qbh))
> +			hpfs_brelse4(&qbh);
> +		else
> +			goto fail;
> +		if (pos == 12)
> +			goto fail;
> 	}
> 	up(&i->i_sem);
> -ok:
> + ok:
> 	unlock_kernel();
> 	return filp->f_pos = new_off;
> -fail:
> + fail:
> 	up(&i->i_sem);
> 	/*printk("illegal lseek: %016llx\n", new_off);*/
> 	unlock_kernel();
> @@ -78,6 +82,7 @@ static int hpfs_readdir(struct file *fil
> 		struct buffer_head *bh;
> 		struct fnode *fno;
> 		int e = 0;
> +
> 		if (!(fno = hpfs_map_fnode(inode->i_sb, inode->i_ino, &bh))) {
> 			ret = -EIOERROR;
> 			goto out;
> @@ -105,7 +110,7 @@ static int hpfs_readdir(struct file *fil
> 		ret = -ENOENT;
> 		goto out;
> 	}
> -
> +
> 	while (1) {
> 		again:
> 		/* This won't work when cycle is longer than number of dirents
> @@ -119,7 +124,7 @@ static int hpfs_readdir(struct file *fil
> 		if (filp->f_pos == 12)
> 			goto out;
> 		if (filp->f_pos == 3 || filp->f_pos == 4 || filp->f_pos == 5) {
> -			printk("HPFS: warning: pos==%d\n",(int)filp->f_pos);
> +			printk("HPFS: warning: pos==%d\n", (int)filp->f_pos);
> 			goto out;
> 		}
> 		if (filp->f_pos == 0) {
> @@ -144,8 +149,10 @@ static int hpfs_readdir(struct file *fil
> 		}
> 		if (de->first || de->last) {
> 			if (hpfs_sb(inode->i_sb)->sb_chk) {
> -				if (de->first && !de->last && (de->namelen != 2 || de ->name[0] != 1 || de->name[1] != 1)) hpfs_error(inode->i_sb, "hpfs_readdir: bad ^A^A entry; pos = %08x", old_pos);
> -				if (de->last && (de->namelen != 1 || de ->name[0] != 255)) hpfs_error(inode->i_sb, "hpfs_readdir: bad \\377 entry; pos = %08x", old_pos);
> +				if (de->first && !de->last && (de->namelen != 2 || de ->name[0] != 1 || de->name[1] != 1))
> +					hpfs_error(inode->i_sb, "hpfs_readdir: bad ^A^A entry; pos = %08x", old_pos);
> +				if (de->last && (de->namelen != 1 || de ->name[0] != 255))
> +					hpfs_error(inode->i_sb, "hpfs_readdir: bad \\377 entry; pos = %08x", old_pos);
> 			}
> 			hpfs_brelse4(&qbh);
> 			goto again;
> @@ -153,14 +160,16 @@ static int hpfs_readdir(struct file *fil
> 		tempname = hpfs_translate_name(inode->i_sb, de->name, de->namelen, lc, de->not_8x3);
> 		if (filldir(dirent, tempname, de->namelen, old_pos, de->fnode, DT_UNKNOWN) < 0) {
> 			filp->f_pos = old_pos;
> -			if (tempname != (char *)de->name) kfree(tempname);
> +			if (tempname != (char *)de->name)
> +				kfree(tempname);
> 			hpfs_brelse4(&qbh);
> 			goto out;
> 		}
> -		if (tempname != (char *)de->name) kfree(tempname);
> +		if (tempname != (char *)de->name)
> +			kfree(tempname);
> 		hpfs_brelse4(&qbh);
> 	}
> -out:
> + out:
> 	unlock_kernel();
> 	return ret;
> }
> @@ -204,13 +213,14 @@ struct dentry *hpfs_lookup(struct inode
> 	 * '.' and '..' will never be passed here.
> 	 */
>
> -	de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *) name, len, NULL, &qbh);
> +	de = map_dirent(dir, hpfs_i(dir)->i_dno, (char *)name, len, NULL, &qbh);
>
> 	/*
> 	 * This is not really a bailout, just means file not found.
> 	 */
>
> -	if (!de) goto end;
> +	if (!de)
> +		goto end;
>
> 	/*
> 	 * Get inode number, what we're after.
> @@ -243,14 +253,16 @@ struct dentry *hpfs_lookup(struct inode
> 		unlock_new_inode(result);
> 	}
> 	hpfs_result = hpfs_i(result);
> -	if (!de->directory) hpfs_result->i_parent_dir = dir->i_ino;
> +	if (!de->directory)
> +		hpfs_result->i_parent_dir = dir->i_ino;
>
> 	hpfs_decide_conv(result, (char *)name, len);
>
> -	if (de->has_acl || de->has_xtd_perm) if (!(dir->i_sb->s_flags & MS_RDONLY)) {
> -		hpfs_error(result->i_sb, "ACLs or XPERM found. This is probably HPFS386. This driver doesn't support it now. Send me some info on these structures");
> -		goto bail1;
> -	}
> +	if (de->has_acl || de->has_xtd_perm)
> +		if (!(dir->i_sb->s_flags & MS_RDONLY)) {
> +			hpfs_error(result->i_sb, "ACLs or XPERM found. This is probably HPFS386. This driver doesn't support it now. Send me some info on these structures");
> +			goto bail1;
> +		}
>
> 	/*
> 	 * Fill in the info from the directory if this is a newly created
> @@ -290,8 +302,8 @@ struct dentry *hpfs_lookup(struct inode
> 	 * Made it.
> 	 */
>
> -	end:
> -	end_add:
> + end:
> + end_add:
> 	hpfs_set_dentry_operations(dentry);
> 	unlock_kernel();
> 	d_add(dentry, result);
> @@ -300,11 +312,10 @@ struct dentry *hpfs_lookup(struct inode
> 	/*
> 	 * Didn't.
> 	 */
> -	bail1:
> -
> + bail1:
> 	hpfs_brelse4(&qbh);
> -
> -	/*bail:*/
> +
> + /*bail:*/
>
> 	unlock_kernel();
> 	return ERR_PTR(-ENOENT);
>

Re: [PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Pekka Enberg]

Hi.

On 10/12/05, Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz> wrote:
> I don't see anything wrong with
>         if (some_condition) do_some_action();
> statements. Generally, if they consume less lines, you can see more code
> on a screen.

>From Documentation/CodingStyle:

"Don't put multiple statements on a single line unless you have
something to hide:"

                                Pekka

Re: [PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Jesper Juhl]

On 10/12/05, Mikulas Patocka <mikulas@artax.karlin.mff.cuni.cz> wrote:
>
>
> On Wed, 12 Oct 2005, Jesper Juhl wrote:
>
> > Whitespace and CodingStyle cleanup for fs/hpfs/dir.c
> >
> > While reading through fs/hpfs/dir.c I ran across lots of cases of multiple
> > statements on a single line that made some bits of the file quite confusing
> > to read. This patch cleans that up and also make sure labels are placed at
> > column 1, removes trailing whitespace and a few other minor CodingStyle nits.
>
> I don't see anything wrong with
>         if (some_condition) do_some_action();
> statements. Generally, if they consume less lines, you can see more code
> on a screen.
>
> Mikulas
>

Well, as Pekka Enberg also pointed out, Documentation/CodingStyle says
that's not the prefered way. But, it's your code, so if you don't like
the cleanups don't apply them.
i still think the patch is a good idea and makes the file more readable though.


> > There are some very long lines in this file that would be a lot more readable
> > (IMHO) if broken up to fit within 80 cols. This patch does not do that, but
> > if wanted I can submit an aditional patch on top of this one to do that.
> >
Same with this bit, IMHO a good idea, but I won't bother making a
patch if you don't want it.

--
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: [PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Pekka Enberg]

Hi,

On Tue, 2005-11-29 at 16:17 +0100, Jesper Juhl wrote:
> Well, as Pekka Enberg also pointed out, Documentation/CodingStyle says
> that's not the prefered way. But, it's your code, so if you don't like
> the cleanups don't apply them.
> i still think the patch is a good idea and makes the file more
> readable though.

I also think the patches are a good idea but it is up to the maintainer,
of course.

				Pekka

Re: [PATCH] hpfs: Whitespace and Codingstyle cleanup for dir.c

[Mikulas Patocka]

Hi

> Hi,
>
> On Tue, 2005-11-29 at 16:17 +0100, Jesper Juhl wrote:
>> Well, as Pekka Enberg also pointed out, Documentation/CodingStyle says
>> that's not the prefered way. But, it's your code, so if you don't like
>> the cleanups don't apply them.
>> i still think the patch is a good idea and makes the file more
>> readable though.

I think: if you are going to hack the hpfs code and coding style patches 
will improve your experience, apply them and push them into mainstream 
kernel. Otherwise don't --- they wouldn't help anybody and they would make 
patch merging between 2.0, 2.2, 2.4 and 2.6 version of this driver (yes 
--- I keep all these synchronised) harder.

Mikulas

> I also think the patches are a good idea but it is up to the maintainer,
> of course.
>
> 				Pekka
>