UML - Fix hostfs style

PATCH In review Jeff Dike 3 participants 5 messages Aug 18, 2007 – Aug 24, 2007 User-mode Linux (UML)

lore

Share & tools

lore

Jeff Dike

author

2007-08-18 00:42 UTC #0 lore raw

PATCH 6/6 UML - Fix hostfs style

Style fixes in hostfs.

Signed-off-by Jeff Dike <[email protected]>

 fs/hostfs/hostfs.h      |    9 +
 fs/hostfs/hostfs_kern.c |  226 ++++++++++++++++++++++++------------------------
 fs/hostfs/hostfs_user.c |  139 +++++++++++++++++------------
 3 files changed, 202 insertions(+), 172 deletions(-)

fs/hostfs/hostfs.h +6 -3

Index: linux-2.6.22/fs/hostfs/hostfs.h
===================================================================
@@ -3,7 +3,8 @@
 
 #include "os.h"
 
-/* These are exactly the same definitions as in fs.h, but the names are
+/*
+ * These are exactly the same definitions as in fs.h, but the names are
  * changed so that this file can be included in both kernel and user files.
  */

@@ -21,7 +22,8 @@
 #define HOSTFS_ATTR_FORCE	512	/* Not a change, but a change it */
 #define HOSTFS_ATTR_ATTR_FLAG	1024
 
-/* If you are very careful, you'll notice that these two are missing:
+/*
+ * If you are very careful, you'll notice that these two are missing:
  *
  * #define ATTR_KILL_SUID	2048
  * #define ATTR_KILL_SGID	4096

@@ -76,7 +78,8 @@ extern int make_symlink(const char *from
 extern int unlink_file(const char *file);
 extern int do_mkdir(const char *file, int mode);
 extern int do_rmdir(const char *file);
-extern int do_mknod(const char *file, int mode, unsigned int major, unsigned int minor);
+extern int do_mknod(const char *file, int mode, unsigned int major,
+		    unsigned int minor);
 extern int link_file(const char *from, const char *to);
 extern int do_readlink(char *file, char *buf, int size);
 extern int rename_file(char *from, char *to);

fs/hostfs/hostfs_kern.c +115 ~~-111~~

623 lines not on this page · raw message

fs/hostfs/hostfs_user.c +81 ~~-58~~

316 lines not on this page · raw message

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems? Stop. Now Search log events and configuration files using AJAX and a browser. Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
User-mode-linux-devel mailing list [email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Satyam Sharma

2007-08-18 14:06 UTC #1 lore raw

Quoted 16 lines · Jeff Dike

Style fixes in hostfs.

Index: linux-2.6.22/fs/hostfs/hostfs_kern.c
[...]

fs/hostfs/hostfs_kern.c +1 -4

@@ -6,22 +6,15 @@
  * 2003-02-10 Petr Baudis <[email protected]>
  */
 
-#include <linux/stddef.h>
 #include <linux/fs.h>
 #include <linux/module.h>
-#include <linux/init.h>
-#include <linux/slab.h>
+#include <linux/mm.h>
 #include <linux/pagemap.h>
-#include <linux/blkdev.h>

fs/hostfs/hostfs_kern.c line 17

Quoted 10 lines

fs/hostfs/hostfs_kern.c +1 -5

-#include <linux/list.h>
 #include <linux/statfs.h>
-#include <linux/kdev_t.h>
 #include <linux/swap.h> /* mark_page_accessed */
-#include <asm/uaccess.h>
 #include "hostfs.h"
-#include "kern_util.h"
-#include "kern.h"
 #include "init.h"
+#include "kern.h"

Not really a style fix :-)

fs/hostfs/hostfs_kern.c int hostfs_readdir(struct file *file, vo · line 339

Quoted 4 lines

fs/hostfs/hostfs_kern.c +1 -1

@@ -328,17 +326,17 @@ int hostfs_readdir(struct file *file, vo
[...]
-		if(error) break;
+		if (error) break;

		if (error)
			break;

fs/hostfs/hostfs_kern.c static int init_inode(struct inode *inod · line 535

Quoted 3 lines

fs/hostfs/hostfs_kern.c +0 -0

@@ -522,28 +523,28 @@ static int init_inode(struct inode *inod
[...]
 	else type = OS_TYPE_DIR;

I wonder what's the generally accepted / followed coding style for this, actually. Personally I'd prefer:

	else
		type = OS_TYPE_DIR;

fs/hostfs/hostfs_kern.c static int init_inode(struct inode *inod

Quoted 7 lines

fs/hostfs/hostfs_kern.c +0 -0

 	else inode->i_op = &hostfs_iops;
 
 	else inode->i_fop = &hostfs_file_fops;
 
 	else inode->i_mapping->a_ops = &hostfs_aops;
 
 	else error = read_name(inode, name);

Ditto.

fs/hostfs/hostfs_kern.c int hostfs_permission(struct inode *ino, · line 811

Quoted 2 lines

fs/hostfs/hostfs_kern.c +0 -0
 	else
 		err = access_file(name, r, w, x);

Here we've used the (different, and preferred IMHO) style. You could make this style common throughout this file.

fs/hostfs/hostfs_kern.c int hostfs_link_readpage(struct file *fi · line 919

Quoted 1 line

fs/hostfs/hostfs_kern.c +1 -0
+	else if (err > 0) {

This is fine, by the way. "if", or even a "{" in the same line after "else" is okay, but not a statement by itself.

Quoted 11 lines

Index: linux-2.6.22/fs/hostfs/hostfs_user.c
[...]

fs/hostfs/hostfs_user.c +3 -3

-#include <unistd.h>
 #include <stdio.h>
-#include <fcntl.h>
+#include <stddef.h>
+#include <unistd.h>
 #include <dirent.h>
 #include <errno.h>
-#include <utime.h>
+#include <fcntl.h>

fs/hostfs/hostfs_user.c line 20

Quoted 10 lines

fs/hostfs/hostfs_user.c +3 -1

 #include <string.h>
 #include <sys/stat.h>
 #include <sys/time.h>
+#include <sys/types.h>
 #include <sys/vfs.h>
 #include "hostfs.h"
-#include "kern_util.h"
+#include "os.h"
 #include "user.h"
+#include <utime.h>

Not a style fix again ...

fs/hostfs/hostfs_user.c int open_file(char *path, int r, int w, · line 130

Quoted 7 lines

fs/hostfs/hostfs_user.c +0 -0

 	else return OS_TYPE_FILE;
 
 	else return 0;
 
 	else panic("Impossible mode in open_file");
 
 	else return fd;

For the "else return" cases, you could consider making the code such that there's a single return at the end, and a "ret" that is set by the code appropriately. You'll find counter-examples, sure, but often multiple "return"s in a function are confusing from a style point of view.

Otherwise, I saw both the patches I was cc'ed on, and both look good to me, thank you.

Blaisorblade

2007-08-23 14:55 UTC #2 lore raw

Quoted 18 lines · Satyam Sharma

fs/hostfs/hostfs_kern.c int hostfs_readdir(struct file *file, vo · line 339
Quoted 6 lines · Jeff Dike
Style fixes in hostfs.
fs/hostfs/hostfs_kern.c +1 -1
@@ -328,17 +326,17 @@ int hostfs_readdir(struct file *file, vo
[...]
-		if(error) break;
+		if (error) break;
		if (error)
			break;
fs/hostfs/hostfs_kern.c static int init_inode(struct inode *inod · line 535
Quoted 3 lines
fs/hostfs/hostfs_kern.c +0 -0
@@ -522,28 +523,28 @@ static int init_inode(struct inode *inod
[...]
 	else type = OS_TYPE_DIR;
I wonder what's the generally accepted / followed coding style for this, actually. Personally I'd prefer:

Quoted 2 lines

	else
		type = OS_TYPE_DIR;

I strongly agree with this style; beyond style itself, one strong reason is that joining statements hinder singlestepping through function code (it's easy to run gdb on UML, and anyway kgdb exists).

Bye

Signature · 4 lines

"Doh!" (cit.), I've made another mistake!
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Jeff Dike

author

2007-08-24 15:21 UTC #3 lore raw

Quoted 8 lines · Blaisorblade

Quoted 4 lines
actually. Personally I'd prefer:
	else
		type = OS_TYPE_DIR;
I strongly agree with this style; beyond style itself, one strong reason is that joining statements hinder singlestepping through function code (it's easy to run gdb on UML, and anyway kgdb exists).

How does that help? gdb should stop as easily on a "else foo;" line as on

	else
		foo;

right?

Jeff

Signature · 11 lines

Work email - jdike at linux dot intel dot com

-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

Blaisorblade

2007-08-24 16:10 UTC #4 lore raw

Quoted 6 lines · Jeff Dike

Quoted 5 lines · Blaisorblade
Quoted 4 lines
actually. Personally I'd prefer:
	else
		type = OS_TYPE_DIR;

Quoted 8 lines

Quoted 3 lines
I strongly agree with this style; beyond style itself, one strong reason is that joining statements hinder singlestepping through function code (it's easy to run gdb on UML, and anyway kgdb exists).
How does that help? gdb should stop as easily on a "else foo;" line as on
	else
		foo;
right?

Sorry, a better example is on:

	if (bar)
		foo;

where the test and foo are two distinct parts. One step is "I execute the if", another (possible) step is "I perform foo" - which is not easy to tell if it is not on a different line.

Signature · 4 lines

"Doh!" (cit.), I've made another mistake!
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade

_______________________________________________
User-mode-linux-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel