[patch 0/4] A series of cleanup patches

[patch 0/4] A series of cleanup patches

[Jeremy Fitzhardinge]

Hi Andi, Andrew,

Here's a series of cleanup patches:

1. add kstrndup, which is needed by
2. add argv_split, a helper for splitting a string into an argv array,
3. a restructuring of the call_usermodehelper set of functions, to make
   them more flexible, needed by
4. a common orderly_poweroff() function, which replaces various ad-hoc
   ways of powering off the machine used around the kernel

Thanks,
	J
-- 

[patch 1/4] add kstrndup

[Jeremy Fitzhardinge]

[-- Attachment #1: add-kstrndup.patch --]
[-- Type: text/plain, Size: 4689 bytes --]

Add a kstrndup function, modelled on strndup.  Like strndup this
returns a string copied into its own allocated memory, but it copies
no more than the specified number of bytes from the source.

Remove private strndup() from irda code.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: YOSHIFUJI Hideaki <yoshfuji@linux-ipv6.org>
Cc: Akinobu Mita <akinobu.mita@gmail.com>
Cc: Arnaldo Carvalho de Melo <acme@mandriva.com>
Cc: Al Viro <viro@ftp.linux.org.uk>
Cc: Panagiotis Issaris <takis@issaris.org>

---
 include/linux/string.h  |    1 +
 mm/util.c               |   29 ++++++++++++++++++++++++++++-
 net/irda/irias_object.c |   43 +++++--------------------------------------
 3 files changed, 34 insertions(+), 39 deletions(-)

===================================================================
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -105,6 +105,7 @@ extern void * memchr(const void *,int,__
 #endif
 
 extern char *kstrdup(const char *s, gfp_t gfp);
+extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 
 #ifdef __cplusplus
===================================================================
--- a/mm/util.c
+++ b/mm/util.c
@@ -18,7 +18,7 @@ void *__kzalloc(size_t size, gfp_t flags
 }
 EXPORT_SYMBOL(__kzalloc);
 
-/*
+/**
  * kstrdup - allocate space for and copy an existing string
  *
  * @s: the string to duplicate
@@ -39,6 +39,33 @@ char *kstrdup(const char *s, gfp_t gfp)
 	return buf;
 }
 EXPORT_SYMBOL(kstrdup);
+
+/**
+ * kstrndup - allocate space for and copy an existing string
+ *
+ * @s: the string to duplicate
+ * @max: read at most @max chars from @s
+ * @gfp: the GFP mask used in the kmalloc() call when allocating memory
+ */
+char *kstrndup(const char *s, size_t max, gfp_t gfp)
+{
+	size_t len;
+	char *buf;
+
+	if (!s)
+		return NULL;
+
+	len = strlen(s);
+	if (len > max)
+		len = max;
+	buf = kmalloc_track_caller(len+1, gfp);
+ 	if (buf) {
+		memcpy(buf, s, len);
+		buf[len] = '\0';
+	}
+	return buf;
+}
+EXPORT_SYMBOL(kstrndup);
 
 /**
  * kmemdup - duplicate region of memory
===================================================================
--- a/net/irda/irias_object.c
+++ b/net/irda/irias_object.c
@@ -36,39 +36,6 @@ hashbin_t *irias_objects;
  */
 struct ias_value irias_missing = { IAS_MISSING, 0, 0, 0, {0}};
 
-/*
- * Function strndup (str, max)
- *
- *    My own kernel version of strndup!
- *
- * Faster, check boundary... Jean II
- */
-static char *strndup(char *str, size_t max)
-{
-	char *new_str;
-	int len;
-
-	/* Check string */
-	if (str == NULL)
-		return NULL;
-	/* Check length, truncate */
-	len = strlen(str);
-	if(len > max)
-		len = max;
-
-	/* Allocate new string */
-	new_str = kmalloc(len + 1, GFP_ATOMIC);
-	if (new_str == NULL) {
-		IRDA_WARNING("%s: Unable to kmalloc!\n", __FUNCTION__);
-		return NULL;
-	}
-
-	/* Copy and truncate */
-	memcpy(new_str, str, len);
-	new_str[len] = '\0';
-
-	return new_str;
-}
 
 /*
  * Function ias_new_object (name, id)
@@ -90,7 +57,7 @@ struct ias_object *irias_new_object( cha
 	}
 
 	obj->magic = IAS_OBJECT_MAGIC;
-	obj->name = strndup(name, IAS_MAX_CLASSNAME);
+	obj->name = kstrndup(name, IAS_MAX_CLASSNAME, GFP_ATOMIC);
 	if (!obj->name) {
 		IRDA_WARNING("%s(), Unable to allocate name!\n",
 			     __FUNCTION__);
@@ -360,7 +327,7 @@ void irias_add_integer_attrib(struct ias
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	/* Insert value */
 	attrib->value = irias_new_integer_value(value);
@@ -404,7 +371,7 @@ void irias_add_octseq_attrib(struct ias_
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	attrib->value = irias_new_octseq_value( octets, len);
 	if (!attrib->name || !attrib->value) {
@@ -446,7 +413,7 @@ void irias_add_string_attrib(struct ias_
 	}
 
 	attrib->magic = IAS_ATTRIB_MAGIC;
-	attrib->name = strndup(name, IAS_MAX_ATTRIBNAME);
+	attrib->name = kstrndup(name, IAS_MAX_ATTRIBNAME, GFP_ATOMIC);
 
 	attrib->value = irias_new_string_value(value);
 	if (!attrib->name || !attrib->value) {
@@ -506,7 +473,7 @@ struct ias_value *irias_new_string_value
 
 	value->type = IAS_STRING;
 	value->charset = CS_ASCII;
-	value->t.string = strndup(string, IAS_MAX_STRING);
+	value->t.string = kstrndup(string, IAS_MAX_STRING, GFP_ATOMIC);
 	if (!value->t.string) {
 		IRDA_WARNING("%s: Unable to kmalloc!\n", __FUNCTION__);
 		kfree(value);

-- 

[patch 2/4] add argv_split()

[Jeremy Fitzhardinge]

[-- Attachment #1: split-argv.patch --]
[-- Type: text/plain, Size: 4584 bytes --]

argv_split() is a helper function which takes a string, splits it at
whitespace, and returns a NULL-terminated argv vector.  This is
deliberately simple - it does no quote processing of any kind.

[ Seems to me that this is something which is already being done in
  the kernel, but I couldn't find any other implementations, either to
  steal or replace.  Keep an eye out. ]

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>

---
 include/linux/string.h |    3 
 lib/Makefile           |    2 
 lib/argv_split.c       |  160 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 164 insertions(+), 1 deletion(-)

===================================================================
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -108,6 +108,9 @@ extern char *kstrndup(const char *s, siz
 extern char *kstrndup(const char *s, size_t len, gfp_t gfp);
 extern void *kmemdup(const void *src, size_t len, gfp_t gfp);
 
+extern char **argv_split(gfp_t gfp, const char *str, int *argcp);
+extern void argv_free(char **argv);
+
 #ifdef __cplusplus
 }
 #endif
===================================================================
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -5,7 +5,7 @@ lib-y := ctype.o string.o vsprintf.o cmd
 lib-y := ctype.o string.o vsprintf.o cmdline.o \
 	 rbtree.o radix-tree.o dump_stack.o \
 	 idr.o int_sqrt.o bitmap.o extable.o prio_tree.o \
-	 sha1.o irq_regs.o reciprocal_div.o
+	 sha1.o irq_regs.o reciprocal_div.o argv_split.o
 
 lib-$(CONFIG_MMU) += ioremap.o
 lib-$(CONFIG_SMP) += cpumask.o
===================================================================
--- /dev/null
+++ b/lib/argv_split.c
@@ -0,0 +1,160 @@
+/*
+ * Helper function for splitting a string into an argv-like array.
+ */
+
+#ifndef TEST
+#include <linux/kernel.h>
+#include <linux/ctype.h>
+#include <linux/bug.h>
+#endif
+
+static const char *skip_sep(const char *cp)
+{
+	while(*cp && isspace(*cp))
+		cp++;
+
+	return cp;
+}
+
+static const char *skip_arg(const char *cp)
+{
+	while(*cp && !isspace(*cp))
+		cp++;
+
+	return cp;
+}
+
+static int count_argc(const char *str)
+{
+	int count = 0;
+
+	while(*str) {
+		str = skip_sep(str);
+		if (*str) {
+			count++;
+			str = skip_arg(str);
+		}
+	}
+
+	return count;
+}
+
+/**
+ * argv_free - free an argv
+ *
+ * @argv - the argument vector to be freed
+ *
+ * Frees an argv and the strings it points to.
+ */
+void argv_free(char **argv)
+{
+	char **p;
+	for(p = argv; *p; p++)
+		kfree(*p);
+
+	kfree(argv);
+}
+EXPORT_SYMBOL(argv_free);
+
+/**
+ * argv_split - split a string at whitespace, returning an argv
+ *
+ * @gfp: the GFP mask used to allocate memory
+ * @str: the string to be split
+ * @argcp: returned argument count
+ *
+ * Returns an array of pointers to strings which are split out from
+ * @str.  This is performed by strictly splitting on white-space; no
+ * quote processing is performed.  Multiple whitespace characters are
+ * considered to be a single argument separator.  The returned array
+ * is always NULL-terminated.  Returns NULL on memory allocation
+ * failure.
+ */
+char **argv_split(gfp_t gfp, const char *str, int *argcp)
+{
+	int argc = count_argc(str);
+	char **argv = kzalloc(sizeof(*argv) * (argc+1), gfp);
+	char **argvp;
+
+	if (argv == NULL)
+		goto out;
+
+	*argcp = argc;
+	argvp = argv;
+
+	while(*str) {
+		str = skip_sep(str);
+
+		if (*str) {
+			const char *p = str;
+			char *t;
+
+			str = skip_arg(str);
+
+			t = kstrndup(p, str-p, gfp);
+			if (t == NULL)
+				goto fail;
+			*argvp++ = t;
+		}
+	}
+	*argvp = NULL;
+
+  out:
+	return argv;
+
+  fail:
+	argv_free(argv);
+	return NULL;
+}
+EXPORT_SYMBOL(argv_split);
+
+#ifdef TEST
+#define _GNU_SOURCE
+#include <ctype.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+typedef enum {
+	GFP_KERNEL,
+} gfp_t;
+#define kzalloc(size, x)	malloc(size)
+#define kfree(x)		free(x)
+#define kstrndup(s, n, gfp)	strndup(s, n)
+#define BUG()	abort()
+
+int main() {
+	const char *testvec[] = {
+		"",
+		"x",
+		"\"",
+		"\\\0",
+		"\"",
+		"test one two three",
+		"arg\"foo\"bar biff",
+		"one two\\ three four",
+		"one \"two three\" four",
+		NULL,
+	};
+	const char **t;
+
+	for(t = testvec; *t; t++) {
+		char **argv;
+		int argc;
+		char **a;
+
+		printf("%d: test [%s]\n", t-testvec, *t);
+
+		argv = argv_split(GFP_KERNEL, *t, &argc);
+
+		printf("argc=%d vec=", argc);
+		for(a = argv; *a; a++)
+			printf("[%s] ", *a);
+		printf("\n");
+
+		argv_free(argv);
+	}
+
+	return 0;
+}
+#endif

-- 

[patch 3/4] split usermodehelper setup from execution

[Jeremy Fitzhardinge]

[-- Attachment #1: usermodehelper-split-init.patch --]
[-- Type: text/plain, Size: 10202 bytes --]

Rather than having hundreds of variations of call_usermodehelper for
various pieces of usermode state which could be set up, split the
info allocation and initialization from the actual process execution.

This means the general pattern becomes:
 info = call_usermodehelper_setup(path, argv, envp); /* basic state */
 call_usermodehelper_<SET EXTRA STATE>(info, stuff...);	/* extra state */
 call_usermodehelper_exec(info, wait);	/* run process and free info */

This patch introduces wrappers for all the existing calling styles for
call_usermodehelper_*, but folds their implementations into one.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Andi Kleen <ak@suse.de>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: David Howells <dhowells@redhat.com>
Cc: Bj?rn Steinbrink <B.Steinbrink@gmx.de>

---
 include/linux/kmod.h |   44 +++++++++-
 kernel/kmod.c        |  207 ++++++++++++++++++++++++++++++++++----------------
 2 files changed, 184 insertions(+), 67 deletions(-)

===================================================================
--- a/include/linux/kmod.h
+++ b/include/linux/kmod.h
@@ -36,13 +36,51 @@ static inline int request_module(const c
 #define try_then_request_module(x, mod...) ((x) ?: (request_module(mod), (x)))
 
 struct key;
-extern int call_usermodehelper_keys(char *path, char *argv[], char *envp[],
-				    struct key *session_keyring, int wait);
+struct file;
+struct subprocess_info;
+
+/* Allocate a subprocess_info structure */
+struct subprocess_info *call_usermodehelper_setup(char *path,
+						  char **argv, char **envp);
+
+/* Set various pieces of state into the subprocess_info structure */
+void call_usermodehelper_setkeys(struct subprocess_info *info,
+				 struct key *session_keyring);
+int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
+				  struct file **filp);
+void call_usermodehelper_setcleanup(struct subprocess_info *info,
+				    void (*cleanup)(char **argv, char **envp));
+
+/* Actually execute the sub-process */
+int call_usermodehelper_exec(struct subprocess_info *info, int wait);
+
+/* Free the subprocess_info. This is only needed if you're not going
+   to call call_usermodehelper_exec */
+void call_usermodehelper_freeinfo(struct subprocess_info *info);
 
 static inline int
 call_usermodehelper(char *path, char **argv, char **envp, int wait)
 {
-	return call_usermodehelper_keys(path, argv, envp, NULL, wait);
+	struct subprocess_info *info;
+
+	info = call_usermodehelper_setup(path, argv, envp);
+	if (info == NULL)
+		return -ENOMEM;
+	return call_usermodehelper_exec(info, wait);
+}
+
+static inline int
+call_usermodehelper_keys(char *path, char **argv, char **envp,
+			 struct key *session_keyring, int wait)
+{
+	struct subprocess_info *info;
+
+	info = call_usermodehelper_setup(path, argv, envp);
+	if (info == NULL)
+		return -ENOMEM;
+
+	call_usermodehelper_setkeys(info, session_keyring);
+	return call_usermodehelper_exec(info, wait);
 }
 
 extern void usermodehelper_init(void);
===================================================================
--- a/kernel/kmod.c
+++ b/kernel/kmod.c
@@ -123,6 +123,7 @@ struct subprocess_info {
 	int wait;
 	int retval;
 	struct file *stdin;
+	void (*cleanup)(char **argv, char **envp);
 };
 
 /*
@@ -176,6 +177,14 @@ static int ____call_usermodehelper(void 
 	do_exit(0);
 }
 
+void call_usermodehelper_freeinfo(struct subprocess_info *info)
+{
+	if (info->cleanup)
+		(*info->cleanup)(info->argv, info->envp);
+	kfree(info);
+}
+EXPORT_SYMBOL(call_usermodehelper_freeinfo);
+
 /* Keventd can't block, but this (a child) can. */
 static int wait_for_helper(void *data)
 {
@@ -218,7 +227,7 @@ static int wait_for_helper(void *data)
 	}
 
 	if (sub_info->wait < 0)
-		kfree(sub_info);
+		call_usermodehelper_freeinfo(sub_info);
 	else
 		complete(sub_info->complete);
 	return 0;
@@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
 }
 
 /**
- * call_usermodehelper_keys - start a usermode application
- * @path: pathname for the application
- * @argv: null-terminated argument list
- * @envp: null-terminated environment list
- * @session_keyring: session keyring for process (NULL for an empty keyring)
+ * call_usermodehelper_setup - prepare to call a usermode helper
+ * @path - path to usermode executable
+ * @argv - arg vector for process
+ * @envp - environment for process
+ *
+ * Returns either NULL on allocation failure, or a subprocess_info
+ * structure.  This should be passed to call_usermodehelper_exec to
+ * exec the process and free the structure.
+ */
+struct subprocess_info *call_usermodehelper_setup(char *path,
+						  char **argv, char **envp)
+{
+	struct subprocess_info *sub_info;
+	sub_info = kzalloc(sizeof(struct subprocess_info),  GFP_ATOMIC);
+	if (!sub_info)
+		goto out;
+
+	INIT_WORK(&sub_info->work, __call_usermodehelper);
+	sub_info->path = path;
+	sub_info->argv = argv;
+	sub_info->envp = envp;
+
+  out:
+	return sub_info;
+}
+EXPORT_SYMBOL(call_usermodehelper_setup);
+
+/**
+ * call_usermodehelper_setkeys - set the session keys for usermode helper
+ * @info - a subprocess_info returned by call_usermodehelper_setup
+ * @session_keyring - the session keyring for the process
+ */
+void call_usermodehelper_setkeys(struct subprocess_info *info,
+				 struct key *session_keyring)
+{
+	info->ring = session_keyring;
+}
+EXPORT_SYMBOL(call_usermodehelper_setkeys);
+
+/**
+ * call_usermodehelper_setcleanup - set a cleanup function
+ * @info  - a subprocess_info returned by call_usermodehelper_setup
+ * @cleanup - a cleanup function
+ *
+ * The cleanup function is just befor ethe subprocess_info is about to
+ * be freed.  This can be used for freeing the argv and envp.  The
+ * Function must be runnable in either a process context or the
+ * context in which call_usermodehelper_exec is called.
+ */
+void call_usermodehelper_setcleanup(struct subprocess_info *info,
+				    void (*cleanup)(char **argv, char **envp))
+{
+	info->cleanup = cleanup;
+}
+EXPORT_SYMBOL(call_usermodehelper_setcleanup);
+
+/**
+ * call_usermodehelper_stdinpipe - set up a pipe to be used for stdin
+ * @sub_info - a subprocess_info returned by call_usermodehelper_setup
+ * @filp - set to the write-end of a pipe
+ *
+ * This constructs a pipe, and sets the read end to be the stdin of the
+ * subprocess, and returns the write-end in *@filp.
+ */
+int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
+				  struct file **filp)
+{
+	struct file *f;
+
+	f = create_write_pipe();
+	if (IS_ERR(f))
+		return PTR_ERR(f);
+	*filp = f;
+
+	f = create_read_pipe(f);
+	if (IS_ERR(f)) {
+		free_write_pipe(*filp);
+		return PTR_ERR(f);
+	}
+	sub_info->stdin = f;
+
+	return 0;
+}
+EXPORT_SYMBOL(call_usermodehelper_stdinpipe);
+
+/**
+ * call_usermodehelper_exec - start a usermode application
+ * @sub_info: information about the subprocessa
  * @wait: wait for the application to finish and return status.
  *        when -1 don't wait at all, but you get no useful error back when
  *        the program couldn't be exec'ed. This makes it safe to call
@@ -266,33 +358,24 @@ static void __call_usermodehelper(struct
  * Runs a user-space application.  The application is started
  * asynchronously if wait is not set, and runs as a child of keventd.
  * (ie. it runs with full root capabilities).
- *
- * Must be called from process context.  Returns a negative error code
- * if program was not execed successfully, or 0.
- */
-int call_usermodehelper_keys(char *path, char **argv, char **envp,
-			     struct key *session_keyring, int wait)
+ */
+int call_usermodehelper_exec(struct subprocess_info *sub_info,
+			     int wait)
 {
 	DECLARE_COMPLETION_ONSTACK(done);
-	struct subprocess_info *sub_info;
 	int retval;
 
-	if (!khelper_wq)
-		return -EBUSY;
-
-	if (path[0] == '\0')
-		return 0;
-
-	sub_info = kzalloc(sizeof(struct subprocess_info),  GFP_ATOMIC);
-	if (!sub_info)
-		return -ENOMEM;
-
-	INIT_WORK(&sub_info->work, __call_usermodehelper);
+	if (sub_info->path[0] == '\0') {
+		retval = 0;
+		goto out;
+	}
+
+	if (!khelper_wq) {
+		retval = -EBUSY;
+		goto out;
+	}
+
 	sub_info->complete = &done;
-	sub_info->path = path;
-	sub_info->argv = argv;
-	sub_info->envp = envp;
-	sub_info->ring = session_keyring;
 	sub_info->wait = wait;
 
 	queue_work(khelper_wq, &sub_info->work);
@@ -300,47 +383,43 @@ int call_usermodehelper_keys(char *path,
 		return 0;
 	wait_for_completion(&done);
 	retval = sub_info->retval;
-	kfree(sub_info);
+
+  out:
+	call_usermodehelper_freeinfo(sub_info);
 	return retval;
 }
-EXPORT_SYMBOL(call_usermodehelper_keys);
-
+EXPORT_SYMBOL(call_usermodehelper_exec);
+
+/**
+ * call_usermodehelper_pipe - call a usermode helper process with a pipe stdin
+ * @path - path to usermode executable
+ * @argv - arg vector for process
+ * @envp - environment for process
+ * @filp - set to the write-end of a pipe
+ *
+ * This is a simple wrapper which executes a usermode-helper function
+ * with a pipe as stdin.  It is implemented entirely in terms of
+ * lower-level call_usermodehelper_* functions.
+ */
 int call_usermodehelper_pipe(char *path, char **argv, char **envp,
 			     struct file **filp)
 {
-	DECLARE_COMPLETION(done);
-	struct subprocess_info sub_info = {
-		.work		= __WORK_INITIALIZER(sub_info.work,
-						     __call_usermodehelper),
-		.complete	= &done,
-		.path		= path,
-		.argv		= argv,
-		.envp		= envp,
-		.retval		= 0,
-	};
-	struct file *f;
-
-	if (!khelper_wq)
-		return -EBUSY;
-
-	if (path[0] == '\0')
-		return 0;
-
-	f = create_write_pipe();
-	if (IS_ERR(f))
-		return PTR_ERR(f);
-	*filp = f;
-
-	f = create_read_pipe(f);
-	if (IS_ERR(f)) {
-		free_write_pipe(*filp);
-		return PTR_ERR(f);
-	}
-	sub_info.stdin = f;
-
-	queue_work(khelper_wq, &sub_info.work);
-	wait_for_completion(&done);
-	return sub_info.retval;
+	struct subprocess_info *sub_info;
+	int ret;
+
+	sub_info = call_usermodehelper_setup(path, argv, envp);
+	if (sub_info == NULL)
+		return -ENOMEM;
+
+	ret = call_usermodehelper_stdinpipe(sub_info, filp);
+	if (ret < 0)
+		goto out;
+
+	return call_usermodehelper_exec(sub_info, 1);
+
+  out:
+	call_usermodehelper_freeinfo(sub_info);
+	return ret;
 }
 EXPORT_SYMBOL(call_usermodehelper_pipe);
 

-- 

[patch 4/4] Add common orderly_poweroff()

[Jeremy Fitzhardinge]

[-- Attachment #1: orderly-poweroff.patch --]
[-- Type: text/plain, Size: 10119 bytes --]

Various pieces of code around the kernel want to be able to trigger an
orderly poweroff.  This pulls them together into a single
implementation.

By default the poweroff command is /sbin/poweroff, but it can be set
via sysctl: kernel/poweroff_cmd.  This is split at whitespace, so it
can include command-line arguments.

This patch replaces four other instances of invoking either "poweroff"
or "shutdown -h now": one sparc64, two sbus drivers, and acpi thermal
management.

Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
Cc: Chris Wright <chrisw@sous-sol.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andi Kleen <ak@suse.de>
Cc: Len Brown <lenb@kernel.org>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: David S. Miller <davem@davemloft.net>

---
 arch/sparc64/kernel/power.c     |   40 +------------------------------
 drivers/acpi/thermal.c          |   24 +-----------------
 drivers/sbus/char/bbc_envctrl.c |    5 +--
 drivers/sbus/char/envctrl.c     |    7 +----
 include/linux/reboot.h          |    5 +++
 include/linux/sysctl.h          |    1
 kernel/sys.c                    |   50 +++++++++++++++++++++++++++++++++++++++
 kernel/sysctl.c                 |   10 +++++++
 8 files changed, 74 insertions(+), 68 deletions(-)

diff -r 9eea40ea89b7 arch/sparc64/kernel/power.c
--- a/arch/sparc64/kernel/power.c	Tue May 08 12:59:41 2007 -0700
+++ b/arch/sparc64/kernel/power.c	Tue May 08 13:42:22 2007 -0700
@@ -13,6 +13,7 @@
 #include <linux/interrupt.h>
 #include <linux/pm.h>
 #include <linux/syscalls.h>
+#include <linux/reboot.h>
 
 #include <asm/system.h>
 #include <asm/auxio.h>
@@ -32,14 +33,13 @@ int scons_pwroff = 1;
 #include <linux/pci.h>
 static void __iomem *power_reg;
 
-static DECLARE_WAIT_QUEUE_HEAD(powerd_wait);
 static int button_pressed;
 
 static irqreturn_t power_handler(int irq, void *dev_id)
 {
 	if (button_pressed == 0) {
 		button_pressed = 1;
-		wake_up(&powerd_wait);
+		orderly_poweroff(true);
 	}
 
 	/* FIXME: Check registers for status... */
@@ -75,36 +75,6 @@ EXPORT_SYMBOL(pm_power_off);
 EXPORT_SYMBOL(pm_power_off);
 
 #ifdef CONFIG_PCI
-static int powerd(void *__unused)
-{
-	static char *envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { "/sbin/shutdown", "-h", "now", NULL };
-	DECLARE_WAITQUEUE(wait, current);
-
-	daemonize("powerd");
-
-	add_wait_queue(&powerd_wait, &wait);
-again:
-	for (;;) {
-		set_task_state(current, TASK_INTERRUPTIBLE);
-		if (button_pressed)
-			break;
-		flush_signals(current);
-		schedule();
-	}
-	__set_current_state(TASK_RUNNING);
-	remove_wait_queue(&powerd_wait, &wait);
-
-	/* Ok, down we go... */
-	button_pressed = 0;
-	if (kernel_execve("/sbin/shutdown", argv, envp) < 0) {
-		printk("powerd: shutdown execution failed\n");
-		add_wait_queue(&powerd_wait, &wait);
-		goto again;
-	}
-	return 0;
-}
-
 static int __init has_button_interrupt(unsigned int irq, struct device_node *dp)
 {
 	if (irq == PCI_IRQ_NONE)
@@ -128,12 +98,6 @@ static int __devinit power_probe(struct 
 	poweroff_method = machine_halt;  /* able to use the standard halt */
 
 	if (has_button_interrupt(irq, op->node)) {
-		if (kernel_thread(powerd, NULL, CLONE_FS) < 0) {
-			printk("Failed to start power daemon.\n");
-			return 0;
-		}
-		printk("powerd running.\n");
-
 		if (request_irq(irq,
 				power_handler, 0, "power", NULL) < 0)
 			printk("power: Error, cannot register IRQ handler.\n");
diff -r 9eea40ea89b7 drivers/acpi/thermal.c
--- a/drivers/acpi/thermal.c	Tue May 08 12:59:41 2007 -0700
+++ b/drivers/acpi/thermal.c	Tue May 08 13:42:22 2007 -0700
@@ -40,6 +40,7 @@
 #include <linux/jiffies.h>
 #include <linux/kmod.h>
 #include <linux/seq_file.h>
+#include <linux/reboot.h>
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
@@ -61,7 +62,6 @@
 #define ACPI_THERMAL_MODE_ACTIVE	0x00
 #define ACPI_THERMAL_MODE_PASSIVE	0x01
 #define ACPI_THERMAL_MODE_CRITICAL   	0xff
-#define ACPI_THERMAL_PATH_POWEROFF	"/sbin/poweroff"
 
 #define ACPI_THERMAL_MAX_ACTIVE	10
 #define ACPI_THERMAL_MAX_LIMIT_STR_LEN 65
@@ -431,26 +431,6 @@ static int acpi_thermal_get_devices(stru
 	return 0;
 }
 
-static int acpi_thermal_call_usermode(char *path)
-{
-	char *argv[2] = { NULL, NULL };
-	char *envp[3] = { NULL, NULL, NULL };
-
-
-	if (!path)
-		return -EINVAL;
-
-	argv[0] = path;
-
-	/* minimal command environment */
-	envp[0] = "HOME=/";
-	envp[1] = "PATH=/sbin:/bin:/usr/sbin:/usr/bin";
-
-	call_usermodehelper(argv[0], argv, envp, 0);
-
-	return 0;
-}
-
 static int acpi_thermal_critical(struct acpi_thermal *tz)
 {
 	if (!tz || !tz->trips.critical.flags.valid)
@@ -468,7 +448,7 @@ static int acpi_thermal_critical(struct 
 	acpi_bus_generate_event(tz->device, ACPI_THERMAL_NOTIFY_CRITICAL,
 				tz->trips.critical.flags.enabled);
 
-	acpi_thermal_call_usermode(ACPI_THERMAL_PATH_POWEROFF);
+	orderly_poweroff(true);
 
 	return 0;
 }
diff -r 9eea40ea89b7 drivers/sbus/char/bbc_envctrl.c
--- a/drivers/sbus/char/bbc_envctrl.c	Tue May 08 12:59:41 2007 -0700
+++ b/drivers/sbus/char/bbc_envctrl.c	Tue May 08 13:42:22 2007 -0700
@@ -7,6 +7,7 @@
 #include <linux/kthread.h>
 #include <linux/delay.h>
 #include <linux/kmod.h>
+#include <linux/reboot.h>
 #include <asm/oplib.h>
 #include <asm/ebus.h>
 
@@ -170,8 +171,6 @@ static void do_envctrl_shutdown(struct b
 static void do_envctrl_shutdown(struct bbc_cpu_temperature *tp)
 {
 	static int shutting_down = 0;
-	static char *envp[] = { "HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { "/sbin/shutdown", "-h", "now", NULL };
 	char *type = "???";
 	s8 val = -1;
 
@@ -195,7 +194,7 @@ static void do_envctrl_shutdown(struct b
 	printk(KERN_CRIT "kenvctrld: Shutting down the system now.\n");
 
 	shutting_down = 1;
-	if (call_usermodehelper("/sbin/shutdown", argv, envp, 0) < 0)
+	if (orderly_poweroff(true) < 0)
 		printk(KERN_CRIT "envctrl: shutdown execution failed\n");
 }
 
diff -r 9eea40ea89b7 drivers/sbus/char/envctrl.c
--- a/drivers/sbus/char/envctrl.c	Tue May 08 12:59:41 2007 -0700
+++ b/drivers/sbus/char/envctrl.c	Tue May 08 13:42:22 2007 -0700
@@ -26,6 +26,7 @@
 #include <linux/ioport.h>
 #include <linux/miscdevice.h>
 #include <linux/kmod.h>
+#include <linux/reboot.h>
 
 #include <asm/ebus.h>
 #include <asm/uaccess.h>
@@ -965,10 +966,6 @@ static void envctrl_do_shutdown(void)
 static void envctrl_do_shutdown(void)
 {
 	static int inprog = 0;
-	static char *envp[] = {	
-		"HOME=/", "TERM=linux", "PATH=/sbin:/usr/sbin:/bin:/usr/bin", NULL };
-	char *argv[] = { 
-		"/sbin/shutdown", "-h", "now", NULL };	
 	int ret;
 
 	if (inprog != 0)
@@ -976,7 +973,7 @@ static void envctrl_do_shutdown(void)
 
 	inprog = 1;
 	printk(KERN_CRIT "kenvctrld: WARNING: Shutting down the system now.\n");
-	ret = call_usermodehelper("/sbin/shutdown", argv, envp, 0);
+	ret = orderly_poweroff(true);
 	if (ret < 0) {
 		printk(KERN_CRIT "kenvctrld: WARNING: system shutdown failed!\n"); 
 		inprog = 0;  /* unlikely to succeed, but we could try again */
diff -r 9eea40ea89b7 include/linux/reboot.h
--- a/include/linux/reboot.h	Tue May 08 12:59:41 2007 -0700
+++ b/include/linux/reboot.h	Tue May 08 13:42:22 2007 -0700
@@ -67,6 +67,11 @@ extern void kernel_power_off(void);
 
 void ctrl_alt_del(void);
 
+#define POWEROFF_CMD_PATH_LEN	256
+extern char poweroff_cmd[POWEROFF_CMD_PATH_LEN];
+
+extern int orderly_poweroff(bool force);
+
 /*
  * Emergency restart, callable from an interrupt handler.
  */
diff -r 9eea40ea89b7 include/linux/sysctl.h
--- a/include/linux/sysctl.h	Tue May 08 12:59:41 2007 -0700
+++ b/include/linux/sysctl.h	Tue May 08 13:42:22 2007 -0700
@@ -165,6 +165,7 @@ enum
 	KERN_MAX_LOCK_DEPTH=74,
 	KERN_NMI_WATCHDOG=75, /* int: enable/disable nmi watchdog */
 	KERN_PANIC_ON_NMI=76, /* int: whether we will panic on an unrecovered */
+	KERN_POWEROFF_CMD=77,	/* string: poweroff command line */
 };
 
 
diff -r 9eea40ea89b7 kernel/sys.c
--- a/kernel/sys.c	Tue May 08 12:59:41 2007 -0700
+++ b/kernel/sys.c	Tue May 08 13:42:22 2007 -0700
@@ -2208,3 +2208,58 @@ asmlinkage long sys_getcpu(unsigned __us
 	}
 	return err ? -EFAULT : 0;
 }
+
+char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
+
+static void argv_cleanup(char **argv, char **envp)
+{
+	argv_free(argv);
+}
+
+/**
+ * Trigger an orderly system poweroff
+ *
+ * This may be called from any context to trigger a system shutdown.
+ * If the orderly shutdown fails, it will force an immediate shutdown.
+ */
+int orderly_poweroff(bool force)
+{
+	int argc;
+	char **argv = argv_split(GFP_ATOMIC, poweroff_cmd, &argc);
+	static char *envp[] = {
+		"HOME=/",
+		"PATH=/sbin:/bin:/usr/sbin:/usr/bin",
+		NULL
+	};
+	int ret = -ENOMEM;
+	struct subprocess_info *info;
+
+	if (argv == NULL) {
+		printk(KERN_WARNING "%s failed to allocate memory for \"%s\"\n",
+		       __func__, poweroff_cmd);
+		goto out;
+	}
+
+	info = call_usermodehelper_setup(argv[0], argv, envp);
+	if (info == NULL)
+		goto out;
+
+	call_usermodehelper_setcleanup(info, argv_cleanup);
+
+	ret = call_usermodehelper_exec(info, -1);
+
+  out:
+	if (ret && force) {
+		printk(KERN_WARNING "Failed to start orderly shutdown: "
+		       "forcing the issue\n");
+
+		/* I guess this should try to kick off some daemon to
+		   sync and poweroff asap.  Or not even bother syncing
+		   if we're doing an emergency shutdown? */
+		emergency_sync();
+		kernel_power_off();
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL_GPL(orderly_poweroff);
diff -r 9eea40ea89b7 kernel/sysctl.c
--- a/kernel/sysctl.c	Tue May 08 12:59:41 2007 -0700
+++ b/kernel/sysctl.c	Tue May 08 13:42:22 2007 -0700
@@ -45,6 +45,7 @@
 #include <linux/syscalls.h>
 #include <linux/nfs_fs.h>
 #include <linux/acpi.h>
+#include <linux/reboot.h>
 
 #include <asm/uaccess.h>
 #include <asm/processor.h>
@@ -603,6 +604,15 @@ static ctl_table kern_table[] = {
 		.proc_handler	= &proc_dointvec,
 	},
 #endif
+	{
+		.ctl_name	= KERN_POWEROFF_CMD,
+		.procname	= "poweroff_cmd",
+		.data		= &poweroff_cmd,
+		.maxlen		= POWEROFF_CMD_PATH_LEN,
+		.mode		= 0644,
+		.proc_handler	= &proc_dostring,
+		.strategy	= &sysctl_string,
+	},
 
 	{ .ctl_name = 0 }
 };

-- 

Re: [patch 4/4] Add common orderly_poweroff()

[Len Brown]

On Tuesday 08 May 2007 16:51, Jeremy Fitzhardinge wrote:
> Various pieces of code around the kernel want to be able to trigger an
> orderly poweroff.  This pulls them together into a single
> implementation.
> 
> By default the poweroff command is /sbin/poweroff, but it can be set
> via sysctl: kernel/poweroff_cmd.  This is split at whitespace, so it
> can include command-line arguments.
> 
> This patch replaces four other instances of invoking either "poweroff"
> or "shutdown -h now": one sparc64, two sbus drivers, and acpi thermal
> management.
> 
> Signed-off-by: Jeremy Fitzhardinge <jeremy@xensource.com>
> Cc: Chris Wright <chrisw@sous-sol.org>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Andi Kleen <ak@suse.de>
> Cc: Len Brown <lenb@kernel.org>

Re: the ACPI parts
Acked-by: Len Brown <len.brown@intel.com>

thanks,
-Len
 
> Cc: Al Viro <viro@zeniv.linux.org.uk>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: David S. Miller <davem@davemloft.net>
> 
> ---
>  arch/sparc64/kernel/power.c     |   40 +------------------------------
>  drivers/acpi/thermal.c          |   24 +-----------------
>  drivers/sbus/char/bbc_envctrl.c |    5 +--
>  drivers/sbus/char/envctrl.c     |    7 +----
>  include/linux/reboot.h          |    5 +++
>  include/linux/sysctl.h          |    1
>  kernel/sys.c                    |   50 +++++++++++++++++++++++++++++++++++++++
>  kernel/sysctl.c                 |   10 +++++++
>  8 files changed, 74 insertions(+), 68 deletions(-)

Re: [patch 1/4] add kstrndup

[Randy Dunlap]

On Tue, 08 May 2007 13:51:30 -0700 Jeremy Fitzhardinge wrote:

> --- a/mm/util.c
> +++ b/mm/util.c
> @@ -18,7 +18,7 @@ void *__kzalloc(size_t size, gfp_t flags
>  }
>  EXPORT_SYMBOL(__kzalloc);
>  
> -/*
> +/**
>   * kstrdup - allocate space for and copy an existing string

No blank ("*") line between function name and its parameters.
(occurs in multiple places in multiple patches in this series;
I'm not going to point out each one of them separately)

>   *
>   * @s: the string to duplicate
> @@ -39,6 +39,33 @@ char *kstrdup(const char *s, gfp_t gfp)
>  	return buf;
>  }
>  EXPORT_SYMBOL(kstrdup);
> +
> +/**
> + * kstrndup - allocate space for and copy an existing string
> + *
> + * @s: the string to duplicate
> + * @max: read at most @max chars from @s
> + * @gfp: the GFP mask used in the kmalloc() call when allocating memory
> + */
> +char *kstrndup(const char *s, size_t max, gfp_t gfp)
> +{
> +}
> +EXPORT_SYMBOL(kstrndup);
>  
>  /**
>   * kmemdup - duplicate region of memory
> ===================================================================

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 2/4] add argv_split()

[Randy Dunlap]

On Tue, 08 May 2007 13:51:31 -0700 Jeremy Fitzhardinge wrote:

> --- /dev/null
> +++ b/lib/argv_split.c
> @@ -0,0 +1,160 @@
> +
> +static const char *skip_sep(const char *cp)
> +{
> +	while(*cp && isspace(*cp))

Kernel style is space after "while", "for", "if"...

> +		cp++;
> +
> +	return cp;
> +}
> +
> +static const char *skip_arg(const char *cp)
> +{
> +	while(*cp && !isspace(*cp))
> +		cp++;
> +
> +	return cp;
> +}
> +
> +static int count_argc(const char *str)
> +{
> +	int count = 0;
> +
> +	while(*str) {
> +		str = skip_sep(str);
> +		if (*str) {
> +			count++;
> +			str = skip_arg(str);
> +		}
> +	}
> +
> +	return count;
> +}
> +
> +/**
> + * argv_free - free an argv
> + *

no blank "*" line.

> + * @argv - the argument vector to be freed

    * @argv: the argument vector to be freed


> + *
> + * Frees an argv and the strings it points to.
> + */
> +void argv_free(char **argv)
> +{
> +	char **p;
> +	for(p = argv; *p; p++)
> +		kfree(*p);
> +
> +	kfree(argv);
> +}
> +EXPORT_SYMBOL(argv_free);


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 3/4] split usermodehelper setup from execution

[Jeremy Fitzhardinge]

Randy Dunlap wrote:
> On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:
>
>   
>> --- a/kernel/kmod.c
>> +++ b/kernel/kmod.c
>> @@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
>>  }
>>  
>>  /**
>> - * call_usermodehelper_keys - start a usermode application
>> - * @path: pathname for the application
>> - * @argv: null-terminated argument list
>> - * @envp: null-terminated environment list
>> - * @session_keyring: session keyring for process (NULL for an empty keyring)
>> + * call_usermodehelper_setup - prepare to call a usermode helper
>> + * @path - path to usermode executable
>> + * @argv - arg vector for process
>> + * @envp - environment for process
>>     
>
> s / - /: / above please.
>   

The power of cut'n'paste makes the errors consistent, at least...  Will
fix up.

    J

Re: [patch 3/4] split usermodehelper setup from execution

[Randy Dunlap]

On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:

> --- a/kernel/kmod.c
> +++ b/kernel/kmod.c
> @@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
>  }
>  
>  /**
> - * call_usermodehelper_keys - start a usermode application
> - * @path: pathname for the application
> - * @argv: null-terminated argument list
> - * @envp: null-terminated environment list
> - * @session_keyring: session keyring for process (NULL for an empty keyring)
> + * call_usermodehelper_setup - prepare to call a usermode helper
> + * @path - path to usermode executable
> + * @argv - arg vector for process
> + * @envp - environment for process

s / - /: / above please.

> + *
> + * Returns either NULL on allocation failure, or a subprocess_info
> + * structure.  This should be passed to call_usermodehelper_exec to
> + * exec the process and free the structure.
> + */
> +struct subprocess_info *call_usermodehelper_setup(char *path,
> +						  char **argv, char **envp)
> +{
> +}
> +EXPORT_SYMBOL(call_usermodehelper_setup);
> +
> +/**
> + * call_usermodehelper_setkeys - set the session keys for usermode helper
> + * @info - a subprocess_info returned by call_usermodehelper_setup
> + * @session_keyring - the session keyring for the process

and here.

> + */
> +
> +/**
> + * call_usermodehelper_setcleanup - set a cleanup function
> + * @info  - a subprocess_info returned by call_usermodehelper_setup
> + * @cleanup - a cleanup function

again.

> + *
> + * The cleanup function is just befor ethe subprocess_info is about to
> + * be freed.  This can be used for freeing the argv and envp.  The
> + * Function must be runnable in either a process context or the
> + * context in which call_usermodehelper_exec is called.
> + */
> +void call_usermodehelper_setcleanup(struct subprocess_info *info,
> +				    void (*cleanup)(char **argv, char **envp))
> +{
> +	info->cleanup = cleanup;
> +}
> +EXPORT_SYMBOL(call_usermodehelper_setcleanup);
> +
> +/**
> + * call_usermodehelper_stdinpipe - set up a pipe to be used for stdin
> + * @sub_info - a subprocess_info returned by call_usermodehelper_setup
> + * @filp - set to the write-end of a pipe

again.

> + *
> + * This constructs a pipe, and sets the read end to be the stdin of the
> + * subprocess, and returns the write-end in *@filp.
> + */
> +int call_usermodehelper_stdinpipe(struct subprocess_info *sub_info,
> +				  struct file **filp)
> +{
> +}
> +EXPORT_SYMBOL(call_usermodehelper_stdinpipe);

...

> +/**
> + * call_usermodehelper_pipe - call a usermode helper process with a pipe stdin
> + * @path - path to usermode executable
> + * @argv - arg vector for process
> + * @envp - environment for process
> + * @filp - set to the write-end of a pipe

and here.

> + *
> + * This is a simple wrapper which executes a usermode-helper function
> + * with a pipe as stdin.  It is implemented entirely in terms of
> + * lower-level call_usermodehelper_* functions.
> + */
>  int call_usermodehelper_pipe(char *path, char **argv, char **envp,
>  			     struct file **filp)
>  {


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 4/4] Add common orderly_poweroff()

[Randy Dunlap]

On Tue, 08 May 2007 13:51:33 -0700 Jeremy Fitzhardinge wrote:

> diff -r 9eea40ea89b7 kernel/sys.c
> --- a/kernel/sys.c	Tue May 08 12:59:41 2007 -0700
> +++ b/kernel/sys.c	Tue May 08 13:42:22 2007 -0700
> @@ -2208,3 +2208,58 @@ asmlinkage long sys_getcpu(unsigned __us
>  	}
>  	return err ? -EFAULT : 0;
>  }
> +
> +char poweroff_cmd[POWEROFF_CMD_PATH_LEN] = "/sbin/poweroff";
> +
> +static void argv_cleanup(char **argv, char **envp)
> +{
> +	argv_free(argv);
> +}
> +
> +/**

   /*
would be OK since this isn't kernel-doc.... (or make it kernel-doc)

> + * Trigger an orderly system poweroff
> + *
> + * This may be called from any context to trigger a system shutdown.
> + * If the orderly shutdown fails, it will force an immediate shutdown.
> + */
> +int orderly_poweroff(bool force)
> +{


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 3/4] split usermodehelper setup from execution

[Randy Dunlap]

On Tue, 08 May 2007 15:00:44 -0700 Jeremy Fitzhardinge wrote:

> Randy Dunlap wrote:
> > On Tue, 08 May 2007 13:51:32 -0700 Jeremy Fitzhardinge wrote:
> >
> >   
> >> --- a/kernel/kmod.c
> >> +++ b/kernel/kmod.c
> >> @@ -253,11 +262,94 @@ static void __call_usermodehelper(struct
> >>  }
> >>  
> >>  /**
> >> - * call_usermodehelper_keys - start a usermode application
> >> - * @path: pathname for the application
> >> - * @argv: null-terminated argument list
> >> - * @envp: null-terminated environment list
> >> - * @session_keyring: session keyring for process (NULL for an empty keyring)
> >> + * call_usermodehelper_setup - prepare to call a usermode helper
> >> + * @path - path to usermode executable
> >> + * @argv - arg vector for process
> >> + * @envp - environment for process
> >>     
> >
> > s / - /: / above please.
> >   
> 
> The power of cut'n'paste makes the errors consistent, at least...  Will
> fix up.

Thanks.  If you point me to such foobars (in general), I'll fix them up.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [patch 2/4] add argv_split()

[Jan Engelhardt]

On May 8 2007 13:51, Jeremy Fitzhardinge wrote:
>+
>+static const char *skip_sep(const char *cp)

These are so simple they could use an 'inline' tag.

>+{
>+	while(*cp && isspace(*cp))
>+		cp++;
>+
>+	return cp;
>+}
>+

>+#ifdef TEST

But not in the end result.

>+int main() {
>+	const char *testvec[] = {

Suggesting static const char *const testvec[].



Jan
-- 

Re: [patch 2/4] add argv_split()

[Jeremy Fitzhardinge]

Jan Engelhardt wrote:
> On May 8 2007 13:51, Jeremy Fitzhardinge wrote:
>   
>> +
>> +static const char *skip_sep(const char *cp)
>>     
>
> These are so simple they could use an 'inline' tag.
>   

I'm sure the compiler can work that out for itself.

>> +#ifdef TEST
>>     
>
> But not in the end result.
>   

?

>> +int main() {
>> +	const char *testvec[] = {
>>     
>
> Suggesting static const char *const testvec[].
>   

I guess, but it doesn't matter much.

    J

Re: [patch 3/4] split usermodehelper setup from execution

[Rusty Russell]

On Tue, 2007-05-08 at 13:51 -0700, Jeremy Fitzhardinge wrote:
> plain text document attachment (usermodehelper-split-init.patch)
> Rather than having hundreds of variations of call_usermodehelper for
> various pieces of usermode state which could be set up, split the
> info allocation and initialization from the actual process execution.

Erk, I hadn't realized how extended call_usermodehelper had become.

call_usermodehelper_pipe()?  Erk.  Grepping... for core dumping via a
process?  Without modifying the ELF core dumper (at least) to handle
short writes?  Why not dump it in an agreed location and exec the
process with that as an arg?

When did this go in?  2.6.19... hmm, too late to rip it out I guess 8(

Anyway, I'm not sure exposing an open-ended interface wins: refactoring
internally definitely makes sense though.

Rusty.

Re: [patch 3/4] split usermodehelper setup from execution

[Jeremy Fitzhardinge]

Rusty Russell wrote:
> call_usermodehelper_pipe()?  Erk.  Grepping... for core dumping via a
> process?  Without modifying the ELF core dumper (at least) to handle
> short writes?  Why not dump it in an agreed location and exec the
> process with that as an arg?
>   

I think the idea is that you might want to send it over the net on a
diskless system, or filter/compress the core before it hits disk because
you don't have enough space.

> When did this go in?  2.6.19... hmm, too late to rip it out I guess 8(
>
> Anyway, I'm not sure exposing an open-ended interface wins: refactoring
> internally definitely makes sense though.

Well, I added this specifically so I could add a cleanup callback, which
can be used to free a dynamically allocated argv array (see the
following patch).  And if you're going to have all this stuff, you may
as well make it work properly together.  Why have "you can set the
session keyring *or* use a pipe for stdin"?

As distended as it is, at least it actually fits together somewhat
coherently now.  I'd be up for dropping the single-use wrapper functions.

    J

Re: [patch 4/4] Add common orderly_poweroff()

[Andi Kleen]

On Tuesday 08 May 2007 22:51, Jeremy Fitzhardinge wrote:
> Various pieces of code around the kernel want to be able to trigger an
> orderly poweroff.  This pulls them together into a single
> implementation.

Fine for me from the x86 side.

-Andi