SLES10 Fixes (part 9)

- Proper ioctl() 32/64-bit binary compatibility.  We need to ensure the
  ioctl data itself is always packed the same for 32/64-bit binaries.
  Additionally, the correct thing to do is encode this size in bytes
  as part of the command using _IOC_SIZE().
- Minor formatting changes to respect the 80 character limit.
- Move all SPLAT_SUBSYSTEM_* defines in to splat-ctl.h.
- Increase SPLAT_SUBSYSTEM_UNKNOWN because we were getting close
  to accidentally using it for a real registered subsystem.

cmd/splat.c

@@ -131,8 +131,8 @@ static int subsystem_setup(void)
 
 	rc = ioctl(splatctl_fd, SPLAT_CFG, cfg);
 	if (rc) {
-		fprintf(stderr, "Ioctl() error %lu / %d: %d\n",
+		fprintf(stderr, "Ioctl() error 0x%lx / %d: %d\n",
-		        (unsigned long) SPLAT_CFG, cfg->cfg_cmd, errno);
+		        (unsigned long)SPLAT_CFG, cfg->cfg_cmd, errno);
 		free(cfg);
 		return rc;
 	}
@@ -140,7 +140,7 @@ static int subsystem_setup(void)
 	size = cfg->cfg_rc1;
 	free(cfg);
 
-	/* Based on the newly aquired number of subsystems allocate enough
+	/* Based on the newly acquired number of subsystems allocate
 	 * memory to get the descriptive information for them all. */
 	cfg_size = sizeof(*cfg) + size * sizeof(splat_user_t);
 	cfg = (splat_cfg_t *)malloc(cfg_size);
@@ -180,23 +180,6 @@ static int subsystem_setup(void)
 	return 0;
 }
 
-/* XXX - Commented out until we sort the lists */
-#if 0
-static int subsystem_compare(const void *l_arg, const void *r_arg, void *private)
-{
-	const subsystem_t *l = l_arg;
-	const subsystem_t *r = r_arg;
-
-	if (l->sub_desc.id > r->sub_desc.id)
-		return 1;
-
-	if (l->sub_desc.id < r->sub_desc.id)
-		return -1;
-
-	return 0;
-}
-#endif
-
 static void subsystem_list(List l, int indent)
 {
 	ListIterator i;
@@ -271,7 +254,7 @@ static int test_setup(subsystem_t *sub)
 
 	/* Based on the newly aquired number of tests allocate enough
 	 * memory to get the descriptive information for them all. */
-	cfg = (splat_cfg_t *)malloc(sizeof(*cfg) + size * sizeof(splat_user_t));
+	cfg = (splat_cfg_t *)malloc(sizeof(*cfg) + size*sizeof(splat_user_t));
 	if (cfg == NULL)
 		return -ENOMEM;
 
@@ -309,23 +292,6 @@ static int test_setup(subsystem_t *sub)
 	return 0;
 }
 
-/* XXX - Commented out until we sort the lists */
-#if 0
-static int test_compare(const void *l_arg, const void *r_arg, void *private)
-{
-	const test_t *l = l_arg;
-	const test_t *r = r_arg;
-
-	if (l->test_desc.id > r->test_desc.id)
-		return 1;
-
-	if (l->test_desc.id < r->test_desc.id)
-		return -1;
-
-	return 0;
-}
-#endif
-
 static test_t *test_copy(test_t *test)
 {
 	return test_init(test->test_sub, &test->test_desc);
@@ -374,7 +340,7 @@ static test_t *test_find(char *sub_str, char *test_str)
 		while ((test = list_next(ti))) {
 
 			if (!strncmp(test->test_desc.name, test_str,
-		            SPLAT_NAME_SIZE) || test->test_desc.id == test_num) {
+		            SPLAT_NAME_SIZE) || test->test_desc.id==test_num) {
 				list_iterator_destroy(ti);
 			        list_iterator_destroy(si);
 				return test;
@@ -467,7 +433,8 @@ static int test_run(cmd_args_t *args, test_t *test)
 	free(cmd);
 
 	if (args->args_verbose) {
-		if ((rc = read(splatctl_fd, splat_buffer, splat_buffer_size - 1)) < 0) {
+		if ((rc = read(splatctl_fd, splat_buffer,
+			       splat_buffer_size - 1)) < 0) {
 			fprintf(stdout, "Error reading results: %d\n", rc);
 		} else {
 			fprintf(stdout, "\n%s\n", splat_buffer);
@@ -530,7 +497,7 @@ static int args_parse_test(cmd_args_t *args, char *str)
 	if (!strncasecmp(sub_str, "all", strlen(sub_str)) || (sub_num == -1))
 		sub_all = 1;
 
-	if (!strncasecmp(test_str, "all", strlen(test_str)) || (test_num == -1))
+	if (!strncasecmp(test_str,"all",strlen(test_str)) || (test_num == -1))
 		test_all = 1;
 
 	si = list_iterator_create(subsystems);
@@ -551,7 +518,7 @@ static int args_parse_test(cmd_args_t *args, char *str)
 		} else {
 			/* Add a specific test from all subsystems */
 			while ((s = list_next(si))) {
-				if ((t = test_find(s->sub_desc.name,test_str))) {
+				if ((t=test_find(s->sub_desc.name,test_str))) {
 					if ((rc = test_add(args, t)))
 						goto error_run;
 

include/splat-ctl.h

@@ -27,10 +27,13 @@
 #ifndef _SPLAT_CTL_H
 #define _SPLAT_CTL_H
 
-/* Contains shared definitions which both the userspace
+#include <linux/types.h>
- * and kernelspace portions of splat must agree on.
- */
 
+/*
+ * Contains shared definitions for both user space and kernel space.  To
+ * ensure 32-bit/64-bit interoperability over ioctl()'s only types with
+ * fixed sizes can be used.
+ */
 #define SPLAT_MAJOR			225 /* XXX - Arbitrary */
 #define SPLAT_MINORS                    1
 #define SPLAT_NAME			"splatctl"
@@ -40,24 +43,24 @@
 #define SPLAT_DESC_SIZE			60
 
 typedef struct splat_user {
-	char name[SPLAT_NAME_SIZE];	/* short name */
+	char name[SPLAT_NAME_SIZE];	/* Short name */
-	char desc[SPLAT_DESC_SIZE];	/* short description */
+	char desc[SPLAT_DESC_SIZE];	/* Short description */
-	int id;				/* unique numeric id */
+	__u32 id;			/* Unique numeric id */
 } splat_user_t;
 
 #define	SPLAT_CFG_MAGIC			0x15263748U
 typedef struct splat_cfg {
-	unsigned int cfg_magic;		/* Unique magic */
+	__u32 cfg_magic;		/* Unique magic */
-	int cfg_cmd;			/* Config command */
+	__u32 cfg_cmd;			/* Configure command */
-	int cfg_arg1;			/* Config command arg 1 */
+	__s32 cfg_arg1;			/* Configure command arg 1 */
-	int cfg_rc1;			/* Config response 1 */
+	__s32 cfg_rc1;			/* Configure response 1 */
 	union {
 		struct {
-			int size;
+			__u32 size;
 			splat_user_t descs[0];
 		} splat_subsystems;
 		struct {
-			int size;
+			__u32 size;
 			splat_user_t descs[0];
 		} splat_tests;
 	} cfg_data;
@@ -65,16 +68,16 @@ typedef struct splat_cfg {
 
 #define	SPLAT_CMD_MAGIC			0x9daebfc0U
 typedef struct splat_cmd {
-	unsigned int cmd_magic;		/* Unique magic */
+	__u32 cmd_magic;		/* Unique magic */
-	int cmd_subsystem;		/* Target subsystem */
+	__u32 cmd_subsystem;		/* Target subsystem */
-	int cmd_test;			/* Subsystem test */
+	__u32 cmd_test;			/* Subsystem test */
-	int cmd_data_size;		/* Extra opaque data */
+	__u32 cmd_data_size;		/* Opaque data size */
 	char cmd_data_str[0];		/* Opaque data region */
 } splat_cmd_t;
 
 /* Valid ioctls */
-#define SPLAT_CFG				_IOWR('f', 101, long)
+#define SPLAT_CFG			_IOWR('f', 101, splat_cfg_t)
-#define SPLAT_CMD				_IOWR('f', 102, long)
+#define SPLAT_CMD			_IOWR('f', 102, splat_cmd_t)
 
 /* Valid configuration commands */
 #define SPLAT_CFG_BUFFER_CLEAR		0x001	/* Clear text buffer */
@@ -84,11 +87,24 @@ typedef struct splat_cmd {
 #define SPLAT_CFG_TEST_COUNT		0x201	/* Number of tests */
 #define SPLAT_CFG_TEST_LIST		0x202	/* List of N tests */
 
-/* Valid subsystem and test commands defined in each subsystem, we do
+/*
- * need to be careful to avoid colisions.  That alone may argue to define
+ * Valid subsystem and test commands are defined in each subsystem as
- * them all here, for now we just define the global error codes.
+ * SPLAT_SUBSYSTEM_*.  We do need to be careful to avoid collisions, the
+ * currently defined subsystems are as follows:
  */
-#define SPLAT_SUBSYSTEM_UNKNOWN		0xF00
+#define SPLAT_SUBSYSTEM_KMEM		0x0100
-#define SPLAT_TEST_UNKNOWN		0xFFF
+#define SPLAT_SUBSYSTEM_TASKQ		0x0200
+#define SPLAT_SUBSYSTEM_KRNG		0x0300
+#define SPLAT_SUBSYSTEM_MUTEX		0x0400
+#define SPLAT_SUBSYSTEM_CONDVAR		0x0500
+#define SPLAT_SUBSYSTEM_THREAD		0x0600
+#define SPLAT_SUBSYSTEM_RWLOCK		0x0700
+#define SPLAT_SUBSYSTEM_TIME		0x0800
+#define SPLAT_SUBSYSTEM_VNODE		0x0900
+#define SPLAT_SUBSYSTEM_KOBJ		0x0a00
+#define SPLAT_SUBSYSTEM_ATOMIC		0x0b00
+#define SPLAT_SUBSYSTEM_LIST		0x0c00
+#define SPLAT_SUBSYSTEM_GENERIC		0x0d00
+#define SPLAT_SUBSYSTEM_UNKNOWN		0xff00
 
 #endif /* _SPLAT_CTL_H */

module/splat/splat-atomic.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_ATOMIC		0x0b00
 #define SPLAT_ATOMIC_NAME		"atomic"
 #define SPLAT_ATOMIC_DESC		"Kernel Atomic Tests"
 

module/splat/splat-condvar.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_CONDVAR		0x0500
 #define SPLAT_CONDVAR_NAME		"condvar"
 #define SPLAT_CONDVAR_DESC		"Kernel Condition Variable Tests"
 

module/splat/splat-ctl.c

@@ -326,11 +326,15 @@ splat_validate(struct file *file, splat_subsystem_t *sub, int cmd, void *arg)
 }
 
 static int
-splat_ioctl_cfg(struct file *file, unsigned long arg)
+splat_ioctl_cfg(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	splat_cfg_t kcfg;
 	int rc = 0;
 
+	/* User and kernel space agree about arg size */
+	if (_IOC_SIZE(cmd) != sizeof(kcfg))
+		return -EBADMSG;
+
 	if (copy_from_user(&kcfg, (splat_cfg_t *)arg, sizeof(kcfg)))
 		return -EFAULT;
 
@@ -362,7 +366,7 @@ splat_ioctl_cfg(struct file *file, unsigned long arg)
 		case SPLAT_CFG_SUBSYSTEM_LIST:
 			/* cfg_arg1 - Unused
 			 * cfg_rc1  - Set to number of subsystems
-			 * cfg_data.splat_subsystems - Populated with subsystems
+			 * cfg_data.splat_subsystems - Set with subsystems
 			 */
 			rc = splat_subsystem_list(&kcfg, arg);
 			break;
@@ -380,7 +384,8 @@ splat_ioctl_cfg(struct file *file, unsigned long arg)
 			rc = splat_test_list(&kcfg, arg);
 			break;
 		default:
-			splat_print(file, "Bad config command %d\n", kcfg.cfg_cmd);
+			splat_print(file, "Bad config command %d\n",
+				    kcfg.cfg_cmd);
 			rc = -EINVAL;
 			break;
 	}
@@ -389,13 +394,17 @@ splat_ioctl_cfg(struct file *file, unsigned long arg)
 }
 
 static int
-splat_ioctl_cmd(struct file *file, unsigned long arg)
+splat_ioctl_cmd(struct file *file, unsigned int cmd, unsigned long arg)
 {
 	splat_subsystem_t *sub;
 	splat_cmd_t kcmd;
 	int rc = -EINVAL;
 	void *data = NULL;
 
+	/* User and kernel space agree about arg size */
+	if (_IOC_SIZE(cmd) != sizeof(kcmd))
+		return -EBADMSG;
+
 	if (copy_from_user(&kcmd, (splat_cfg_t *)arg, sizeof(kcmd)))
 		return -EFAULT;
 
@@ -432,7 +441,7 @@ splat_ioctl_cmd(struct file *file, unsigned long arg)
 
 static int
 splat_ioctl(struct inode *inode, struct file *file,
-	  unsigned int cmd, unsigned long arg)
+	    unsigned int cmd, unsigned long arg)
 {
         unsigned int minor = iminor(file->f_dentry->d_inode);
 	int rc = 0;
@@ -446,10 +455,10 @@ splat_ioctl(struct inode *inode, struct file *file,
 
 	switch (cmd) {
 		case SPLAT_CFG:
-			rc = splat_ioctl_cfg(file, arg);
+			rc = splat_ioctl_cfg(file, cmd, arg);
 			break;
 		case SPLAT_CMD:
-			rc = splat_ioctl_cmd(file, arg);
+			rc = splat_ioctl_cmd(file, cmd, arg);
 			break;
 		default:
 			splat_print(file, "Bad ioctl command %d\n", cmd);

module/splat/splat-generic.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_GENERIC		0x0d00
 #define SPLAT_GENERIC_NAME		"generic"
 #define SPLAT_GENERIC_DESC		"Kernel Generic Tests"
 

module/splat/splat-kmem.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_KMEM		0x0100
 #define SPLAT_KMEM_NAME			"kmem"
 #define SPLAT_KMEM_DESC			"Kernel Malloc/Slab Tests"
 

module/splat/splat-kobj.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_KOBJ		0x0a00
 #define SPLAT_KOBJ_NAME			"kobj"
 #define SPLAT_KOBJ_DESC			"Kernel Kobj Tests"
 

module/splat/splat-list.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_LIST		0x0c00
 #define SPLAT_LIST_NAME			"list"
 #define SPLAT_LIST_DESC			"Kernel List Tests"
 

module/splat/splat-mutex.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_MUTEX		0x0400
 #define SPLAT_MUTEX_NAME		"mutex"
 #define SPLAT_MUTEX_DESC		"Kernel Mutex Tests"
 

module/splat/splat-random.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_KRNG		0x0300
 #define SPLAT_KRNG_NAME			"krng"
 #define SPLAT_KRNG_DESC			"Kernel Random Number Generator Tests"
 

module/splat/splat-rwlock.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_RWLOCK		0x0700
 #define SPLAT_RWLOCK_NAME		"rwlock"
 #define SPLAT_RWLOCK_DESC		"Kernel RW Lock Tests"
 

module/splat/splat-taskq.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_TASKQ		0x0200
 #define SPLAT_TASKQ_NAME		"taskq"
 #define SPLAT_TASKQ_DESC		"Kernel Task Queue Tests"
 

module/splat/splat-thread.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_THREAD		0x0600
 #define SPLAT_THREAD_NAME		"thread"
 #define SPLAT_THREAD_DESC		"Kernel Thread Tests"
 

module/splat/splat-time.c

@@ -26,7 +26,6 @@
 
 #include "splat-internal.h"
 
-#define SPLAT_SUBSYSTEM_TIME		0x0800
 #define SPLAT_TIME_NAME			"time"
 #define SPLAT_TIME_DESC			"Kernel Time Tests"
 

module/splat/splat-vnode.c

@@ -27,7 +27,6 @@
 #include "splat-internal.h"
 #include <linux/rcupdate.h>
 
-#define SPLAT_SUBSYSTEM_VNODE		0x0900
 #define SPLAT_VNODE_NAME		"vnode"
 #define SPLAT_VNODE_DESC		"Kernel Vnode Tests"