mqueue namespace: adapt sysctl

PATCH In review Serge E. Hallyn 3 participants 6 messages Feb 24, 2009

lore

Share & tools

lore

Serge E. Hallyn

author

2009-02-24 22:02 UTC #0 lore raw

PATCH 4/4 mqueue namespace: adapt sysctl

Largely inspired from ipc/ipc_sysctl.c. This patch isolates the mqueue sysctl stuff in its own file.

Changelog:
Feb 24: comments from Alexey Dobriyan:
1. put proc handlers under CONFIG_PROC_SYSCTL and make them NULL otherwise.
2. remove & from proc_handler when assigned to ctl_table.

Signed-off-by Cedric Le Goater <[email protected]>

Signed-off-by Nadia Derbey <[email protected]>

Signed-off-by Serge E. Hallyn <[email protected]>

---
 include/linux/ipc_namespace.h |   14 +++++
 init/Kconfig                  |    6 ++
 ipc/Makefile                  |    1 +
 ipc/mq_sysctl.c               |  116 +++++++++++++++++++++++++++++++++++++++++
 ipc/mqueue.c                  |   65 +----------------------
 5 files changed, 138 insertions(+), 64 deletions(-)
 create mode 100644 ipc/mq_sysctl.c

include/linux/ipc_namespace.h +14 -0

index 3392d50..3bf40e2 100644
@@ -128,4 +128,18 @@ static inline void put_ipc_ns(struct ipc_namespace *ns)
 {
 }
 #endif
+
+#ifdef CONFIG_POSIX_MQUEUE_SYSCTL
+
+struct ctl_table_header;
+extern struct ctl_table_header *mq_register_sysctl_table(void);
+
+#else /* CONFIG_POSIX_MQUEUE_SYSCTL */
+
+static inline struct ctl_table_header *mq_register_sysctl_table(void)
+{
+	return NULL;
+}
+
+#endif /* CONFIG_POSIX_MQUEUE_SYSCTL */
 #endif

init/Kconfig +6 -0

index 53afa38..ee70d25 100644
@@ -148,6 +148,12 @@ config POSIX_MQUEUE
 
 	  If unsure, say Y.
 
+config POSIX_MQUEUE_SYSCTL
+	bool
+	depends on POSIX_MQUEUE
+	depends on SYSCTL
+	default y
+
 config BSD_PROCESS_ACCT
 	bool "BSD Process Accounting"
 	help

ipc/Makefile +1 -0

index 65c3843..4e1955e 100644
@@ -8,4 +8,5 @@ obj-$(CONFIG_SYSVIPC_SYSCTL) += ipc_sysctl.o
 obj_mq-$(CONFIG_COMPAT) += compat_mq.o
 obj-$(CONFIG_POSIX_MQUEUE) += mqueue.o msgutil.o $(obj_mq-y)
 obj-$(CONFIG_IPC_NS) += namespace.o
+obj-$(CONFIG_POSIX_MQUEUE_SYSCTL) += mq_sysctl.o

ipc/mq_sysctl.c new file +116 -0

116 lines not on this page · raw message

ipc/mqueue.c +1 ~~-64~~

83 lines not on this page · raw message

Signature · 1 line

1.5.4.3

Alexey Dobriyan

2009-02-24 22:26 UTC #2 lore raw

Quoted 34 lines · Andrew Morton

Quoted 15 lines · On Tue, 24 Feb 2009 16:02:56 -0600 "Serge E. Hallyn" <[email protected]> wrote:

(quoted diff) +15 -0

+#ifdef CONFIG_PROC_SYSCTL
+static int proc_mq_dointvec(ctl_table *table, int write, struct file *filp,
+	void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table mq_table;
+	memcpy(&mq_table, table, sizeof(mq_table));
+	mq_table.data = get_mq(table);
+
+	return proc_dointvec(&mq_table, write, filp, buffer, lenp, ppos);
+}
+
+static int proc_mq_dointvec_minmax(ctl_table *table, int write,
+	struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table mq_table;

Review

Quoted 10 lines

(quoted diff) +10 -0

+	memcpy(&mq_table, table, sizeof(mq_table));
+	mq_table.data = get_mq(table);
+
+	return proc_dointvec_minmax(&mq_table, write, filp, buffer,
+					lenp, ppos);
+}
+#else
+proc_handler *proc_mq_dointvec = NULL;
+proc_handler *proc_mq_dointvec_minmax = NULL;
+#endif

looks odd. Was this

ipc/mq_sysctl.c static int proc_mq_dointvec_minmax(ctl_t · line 57

Quoted 8 lines

ipc/mq_sysctl.c renamed from ipc/mq_sysctl.c~mqueue-namespace-adapt-sysctl-update-fix +2 -2

@@ -52,8 +52,8 @@ static int proc_mq_dointvec_minmax(ctl_t
 					lenp, ppos);
 }
 #else
-proc_handler *proc_mq_dointvec = NULL;
-proc_handler *proc_mq_dointvec_minmax = NULL;
+#define proc_mq_dointvec NULL
+#define proc_mq_dointvec_minmax NULL
 #endif

I hope, not.

"#define foo NULL" is the least intrusive thing.

Andrew Morton

2009-02-24 22:48 UTC #3 lore raw

Quoted 20 lines · On Wed, 25 Feb 2009 01:32:45 +0300 Alexey Dobriyan <[email protected]> wrote:

Quoted 9 lines
Review
Quoted 3 lines
(quoted diff) +3 -0
+proc_handler *proc_mq_dointvec = NULL;
+proc_handler *proc_mq_dointvec_minmax = NULL;
+#endif
looks odd. Was this
ipc/mq_sysctl.c static int proc_mq_dointvec_minmax(ctl_t · line 57
Quoted 8 lines
ipc/mq_sysctl.c renamed from ipc/mq_sysctl.c~mqueue-namespace-adapt-sysctl-update-fix +2 -2
@@ -52,8 +52,8 @@ static int proc_mq_dointvec_minmax(ctl_t
 					lenp, ppos);
 }
 #else
-proc_handler *proc_mq_dointvec = NULL;
-proc_handler *proc_mq_dointvec_minmax = NULL;
+#define proc_mq_dointvec NULL
+#define proc_mq_dointvec_minmax NULL
 #endif
I hope, not.

Quoted 1 line

"#define foo NULL" is the least intrusive thing.

well the original code would have spat a warning at compile time then tried to execute code from BSS at runtime, methinks.

But I can't be assed working out how to disable CONFIG_SYSCTL to test that. Gawd I hate `select'.

Serge E. Hallyn

author

2009-02-24 23:23 UTC #4 lore raw

Quoted 23 lines · Andrew Morton

Quoted 20 lines · On Wed, 25 Feb 2009 01:32:45 +0300 Alexey Dobriyan <[email protected]> wrote:
Quoted 9 lines
Review
Quoted 3 lines
(quoted diff) +3 -0
+proc_handler *proc_mq_dointvec = NULL;
+proc_handler *proc_mq_dointvec_minmax = NULL;
+#endif
looks odd. Was this
ipc/mq_sysctl.c static int proc_mq_dointvec_minmax(ctl_t · line 57
Quoted 8 lines
ipc/mq_sysctl.c renamed from ipc/mq_sysctl.c~mqueue-namespace-adapt-sysctl-update-fix +2 -2
@@ -52,8 +52,8 @@ static int proc_mq_dointvec_minmax(ctl_t
 					lenp, ppos);
 }
 #else
-proc_handler *proc_mq_dointvec = NULL;
-proc_handler *proc_mq_dointvec_minmax = NULL;
+#define proc_mq_dointvec NULL
+#define proc_mq_dointvec_minmax NULL
 #endif
I hope, not.

Quoted 7 lines

Quoted 1 line
"#define foo NULL" is the least intrusive thing.
well the original code would have spat a warning at compile time then tried to execute code from BSS at runtime, methinks.
But I can't be assed working out how to disable CONFIG_SYSCTL to test that. Gawd I hate `select'.

Yeah that's a pain... but so I tested it with the below userspace program before sending the patch and the 'if (myhandler)' check (which is analogous to the check in proc_sys_call_handler) worked as expected.

thanks,
-serge

#include <stdio.h>

typedef int proc_handler (int a, int b);

#ifdef FOO

int myhandler(int a, int b) {
	printf("a was %d b was %d\n", a, b);
}

#else
proc_handler *myhandler = NULL;
#endif

int main()

{
	if (myhandler)
		myhandler(1, 2);
}

Serge E. Hallyn

author

2009-02-24 22:32 UTC #5 lore raw

In reply to Andrew Morton

Quoted 46 lines · Andrew Morton

Quoted 15 lines · On Tue, 24 Feb 2009 16:02:56 -0600 "Serge E. Hallyn" <[email protected]> wrote:

(quoted diff) +15 -0

+#ifdef CONFIG_PROC_SYSCTL
+static int proc_mq_dointvec(ctl_table *table, int write, struct file *filp,
+	void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table mq_table;
+	memcpy(&mq_table, table, sizeof(mq_table));
+	mq_table.data = get_mq(table);
+
+	return proc_dointvec(&mq_table, write, filp, buffer, lenp, ppos);
+}
+
+static int proc_mq_dointvec_minmax(ctl_table *table, int write,
+	struct file *filp, void __user *buffer, size_t *lenp, loff_t *ppos)
+{
+	struct ctl_table mq_table;

Review

Quoted 10 lines

(quoted diff) +10 -0

+	memcpy(&mq_table, table, sizeof(mq_table));
+	mq_table.data = get_mq(table);
+
+	return proc_dointvec_minmax(&mq_table, write, filp, buffer,
+					lenp, ppos);
+}
+#else
+proc_handler *proc_mq_dointvec = NULL;
+proc_handler *proc_mq_dointvec_minmax = NULL;
+#endif

looks odd. Was this

ipc/mq_sysctl.c renamed from ipc/mq_sysctl.c~mqueue-namespace-adapt-sysctl-update-fix +2 -2

@@ -52,8 +52,8 @@ static int proc_mq_dointvec_minmax(ctl_t
 					lenp, ppos);
 }
 #else
-proc_handler *proc_mq_dointvec = NULL;
-proc_handler *proc_mq_dointvec_minmax = NULL;
+#define proc_mq_dointvec NULL
+#define proc_mq_dointvec_minmax NULL
 #endif
 
 static int msg_max_limit_min = MIN_MSGMAX;

Quoted 1 line

intended?

It just seemed more in line with the idea of ensuring type safety... Of course the #defines will work and are how the others (at least ipc and utsname) do it, but this seemed a touch better. Of course I'll change it if that's the preference.

thanks,
-serge