[PATCH] kernel/sysctl.c: Type safe macros

[Manfred Spraul]

struct ctl_table is used for creating entries in e.g. /proc/sys/kernel.

The structure contains three void * entries and a function pointer, thus
there is the risk of incorrectly mixing types.

The patch create a macro that prevents accidential mixing, it enforces
that the type expected by the function pointer and the three void * are
all of the same type.

Notes:
- From my first impression, most proc entries mix types, and it works,
  because (int)1 and (unsigned int)1 are identical.
  Thus I'm not sure if this is the right approach.

- the code doesn't compile, it contains an intentional incompatible type

What do you think?

--
	Manfred
---
 kernel/sysctl.c | 113 ++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 97 insertions(+), 16 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 15f2511..bc446a6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -276,6 +276,101 @@ static int min_extfrag_threshold;
 static int max_extfrag_threshold = 1000;
 #endif
 
+/*
+ * Type safe macros for creating the sysctl table entries:
+ *
+ * TODO:
+ * - 1) It works.
+ *	I.e. it complains when _dointvec is used to assign
+ *      to an unsigned int. Unfortunately, this is very common:
+ *       * _minmax is used, with min=0 and e.g. max=int_max.
+ *	 * the variable is actually used as a boolean, thus it doesn't matter
+ *	   if the user space interface returns -1 or 0xffffffff.
+ *	 * unsigned int zero is used as min
+ *	 * ...
+ *      Thus most error messages would be false positives.
+ *
+ * - 2) The error message is difficult to understand
+ *		error: initializer element is not constant
+ */
+
+#define ASSIGN_TYPE_SAFE(param, type)					\
+    __builtin_choose_expr(__builtin_types_compatible_p(typeof(param), type), \
+        param,							\
+        /* The void expression results in a compile-time error	\
+             when assigning the result to something.  */		\
+        ((void)0)							\
+    )
+
+#define SYSCTL_BUILD_INT_ENTRY_MINMAX(name, entry, min_ptr, max_ptr) \
+	{ \
+		.procname	= (name), \
+		.data		= ASSIGN_TYPE_SAFE(&(entry), int *), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= proc_dointvec_minmax, \
+		.extra1		= ASSIGN_TYPE_SAFE( (min_ptr), int *), \
+		.extra2		= ASSIGN_TYPE_SAFE( (max_ptr), int *) \
+	}
+
+#define SYSCTL_BUILD_ULONG_ENTRY_MINMAX(name, entry, min_ptr, max_ptr) \
+	{ \
+		.procname	= (name), \
+		.data		= ASSIGN_TYPE_SAFE(&(entry), unsigned long *), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= proc_doulongvec_minmax, \
+		.extra1		= ASSIGN_TYPE_SAFE( (min_ptr), unsigned long *), \
+		.extra2		= ASSIGN_TYPE_SAFE( (max_ptr), unsigned long *) \
+	}
+
+#define SYSCTL_BUILD_ENTRY_MINMAX(name, entry, min_ptr, max_ptr)	\
+	{ \
+		.procname	= (name), \
+		.data		= &(entry), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), int *), \
+					proc_dointvec_minmax, \
+					__builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), unsigned long *), \
+						proc_doulongvec_minmax, \
+						((void)0) ) ), \
+		.extra1		= ASSIGN_TYPE_SAFE( (min_ptr), typeof( &(entry) )), \
+		.extra2		= ASSIGN_TYPE_SAFE( (max_ptr), typeof( &(entry) )) \
+	}
+
+#define SYSCTL_BUILD_INT_ENTRY(name, entry) \
+	{ \
+		.procname	= (name), \
+		.data		= ASSIGN_TYPE_SAFE( &(entry), int *), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= proc_dointvec, \
+	}
+
+#define SYSCTL_BUILD_ULONG_ENTRY(name, entry) \
+	{ \
+		.procname	= (name), \
+		.data		= ASSIGN_TYPE_SAFE( &(entry), unsigned long *), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= proc_doulongvec, \
+	}
+
+#define SYSCTL_BUILD_ENTRY(name, entry)	\
+	{ \
+		.procname	= (name), \
+		.data		= &(entry), \
+		.maxlen		= sizeof( (entry) ), \
+		.mode		= 0644, \
+		.proc_handler	= __builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), int *), \
+					proc_dointvec, \
+					__builtin_choose_expr(__builtin_types_compatible_p(typeof( &(entry)), unsigned long *), \
+						proc_doulongvec, \
+						((void)0) ) ) \
+	}
+
+
 static struct ctl_table kern_table[] = {
 	{
 		.procname	= "sched_child_runs_first",
@@ -1344,22 +1439,8 @@ static struct ctl_table vm_table[] = {
 		.mode		= 0644,
 		.proc_handler	= proc_dointvec_jiffies,
 	},
-	{
-		.procname	= "block_dump",
-		.data		= &block_dump,
-		.maxlen		= sizeof(block_dump),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-		.extra1		= &zero,
-	},
-	{
-		.procname	= "vfs_cache_pressure",
-		.data		= &sysctl_vfs_cache_pressure,
-		.maxlen		= sizeof(sysctl_vfs_cache_pressure),
-		.mode		= 0644,
-		.proc_handler	= proc_dointvec,
-		.extra1		= &zero,
-	},
+	SYSCTL_BUILD_ENTRY_MINMAX("block_dump", block_dump, &zero, (unsigned int *)NULL),
+	SYSCTL_BUILD_ENTRY_MINMAX("vfs_cache_pressure", sysctl_vfs_cache_pressure, &zero, (int *)NULL),
 #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT
 	{
 		.procname	= "legacy_va_layout",
-- 
2.1.0


--------------090408000305030706090900--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/