[PATCH] udev - simpler PROGRAM execution with arguments

[PATCH] udev - simpler PROGRAM execution with arguments

[Kay Sievers]

[-- Attachment #1: Type: text/plain, Size: 457 bytes --]

While putting the limitations of our PROGRAM execution with arguments in
the man page, I asked myself why we don't simply use a subshell if a space
is found in the PROGRAM key.
It would be possible to use all shell functions like pipe or diversion
and we get rid of the hardcoded argument limit.

Pat, is this fine with you? scsi_id without arguments is still called
directly. As promised, I've added a info() for a failing PROGRAM execution.


thanks,
Kay

[-- Attachment #2: 04-exec-in-subshell.patch --]
[-- Type: text/plain, Size: 1994 bytes --]

===== namedev.c 1.112 vs edited =====
--- 1.112/namedev.c	Wed Feb 25 00:46:55 2004
+++ edited/namedev.c	Wed Feb 25 02:01:54 2004
@@ -405,38 +405,28 @@
 	int value_set = 0;
 	char buffer[256];
 	char *pos;
-	char *args[PROGRAM_MAXARG];
-	int i;
 
-	dbg("executing '%s'", path);
 	retval = pipe(fds);
 	if (retval != 0) {
 		dbg("pipe failed");
 		return -1;
 	}
+
 	pid = fork();
 	switch(pid) {
 	case 0:
 		/* child */
 		close(STDOUT_FILENO);
 		dup(fds[1]);	/* dup write side of pipe to STDOUT */
+
 		if (strchr(path, ' ')) {
-			/* exec with arguments */
-			pos = path;
-			for (i=0; i < PROGRAM_MAXARG-1; i++) {
-				args[i] = strsep(&pos, " ");
-				if (args[i] == NULL)
-					break;
-			}
-			if (args[i]) {
-				dbg("too many args - %d", i);
-				args[i] = NULL;
-			}
-			retval = execv(args[0], args);
+			dbg("execute program in subshell '%s'", path);
+			retval = execl("/bin/sh", "sh", "-c", path, NULL);
 		} else {
+			dbg("execute program '%s'", path);
 			retval = execv(path, main_argv);
 		}
-		dbg("child execve failed");
+		info(FIELD_PROGRAM " execution of '%s' failed", path);
 		exit(1);
 	case -1:
 		dbg("fork failed");
@@ -478,6 +468,7 @@
 			retval = -1;
 		}
 	}
+
 	return retval;
 }
 
===== namedev.h 1.26 vs edited =====
--- 1.26/namedev.h	Thu Feb 19 15:56:44 2004
+++ edited/namedev.h	Wed Feb 25 02:13:54 2004
@@ -48,7 +48,6 @@
 #define ATTR_PARTITIONS		"all_partitions"
 #define PARTITIONS_COUNT	15
 
-#define PROGRAM_MAXARG		10
 #define MAX_SYSFS_PAIRS		5
 
 #define RULEFILE_EXT		".rules"
===== udev.8 1.42 vs edited =====
--- 1.42/udev.8	Wed Feb 25 01:38:29 2004
+++ edited/udev.8	Wed Feb 25 02:04:29 2004
@@ -182,8 +182,7 @@
 .TP
 .B PROGRAM
 Call external program. This key is valid if the program returns successful.
-A few command line options may specified, but shell characters like pipe,
-diversion or similiar options are not available. The environment variables of
+The environment variables of
 .B udev
 are also available for the program.
 .br

Re: [PATCH] udev - simpler PROGRAM execution with arguments

[Patrick Mansfield]

On Wed, Feb 25, 2004 at 02:19:44AM +0100, Kay Sievers wrote:
> While putting the limitations of our PROGRAM execution with arguments in
> the man page, I asked myself why we don't simply use a subshell if a space
> is found in the PROGRAM key.
> It would be possible to use all shell functions like pipe or diversion
> and we get rid of the hardcoded argument limit.

But now you depend on /bin/sh, is that acceptable?

> Pat, is this fine with you? scsi_id without arguments is still called
> directly. As promised, I've added a info() for a failing PROGRAM execution.

Yes, especially since scsi_id should be called with no arguments.

-- Patrick Mansfield


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] udev - simpler PROGRAM execution with arguments

[Kay Sievers]

On Wed, 2004-02-25 at 02:34, Patrick Mansfield wrote:
> On Wed, Feb 25, 2004 at 02:19:44AM +0100, Kay Sievers wrote:
> > While putting the limitations of our PROGRAM execution with arguments in
> > the man page, I asked myself why we don't simply use a subshell if a space
> > is found in the PROGRAM key.
> > It would be possible to use all shell functions like pipe or diversion
> > and we get rid of the hardcoded argument limit.
> 
> But now you depend on /bin/sh, is that acceptable?

If it's not there, only the PROGRAM call with arguments will fail.
I think there are only few systems without /bin/sh but udev :)


> > Pat, is this fine with you? scsi_id without arguments is still called
> > directly. As promised, I've added a info() for a failing PROGRAM execution.
> 
> Yes, especially since scsi_id should be called with no arguments.

Fine.


thanks,
Kay



-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: [PATCH] udev - simpler PROGRAM execution with arguments

[Greg KH]

On Wed, Feb 25, 2004 at 02:42:09AM +0100, Kay Sievers wrote:
> On Wed, 2004-02-25 at 02:34, Patrick Mansfield wrote:
> > On Wed, Feb 25, 2004 at 02:19:44AM +0100, Kay Sievers wrote:
> > > While putting the limitations of our PROGRAM execution with arguments in
> > > the man page, I asked myself why we don't simply use a subshell if a space
> > > is found in the PROGRAM key.
> > > It would be possible to use all shell functions like pipe or diversion
> > > and we get rid of the hardcoded argument limit.
> > 
> > But now you depend on /bin/sh, is that acceptable?
> 
> If it's not there, only the PROGRAM call with arguments will fail.
> I think there are only few systems without /bin/sh but udev :)

Hm, but some initramfs systems will probably not have a /bin/sh.

So I've made the following change instead.  If we go over our internal
limit of arguments then we will call /bin/sh, otherwise we handle it
ourselves.  This should make the initramfs people happy, and everyone
else too.

Any complaints?

thanks,

greg k-h


# remove limit of the number of args passed to PROGRAM
#
# If we go over our internal limit of 7, then we call out to /bin/sh
# otherwise we handle it ourself without relying on a shell.

diff -Nru a/namedev.c b/namedev.c
--- a/namedev.c	Thu Feb 26 13:30:02 2004
+++ b/namedev.c	Thu Feb 26 13:30:02 2004
@@ -421,7 +421,13 @@
 	case 0:
 		/* child */
 		close(STDOUT_FILENO);
-		dup(fds[1]);	/* dup write side of pipe to STDOUT */
+
+		/* dup write side of pipe to STDOUT */
+		dup(fds[1]);
+
+		/* copy off our path to use incase we have too many args */
+		strnfieldcpy(buffer, path, sizeof(buffer));
+
 		if (strchr(path, ' ')) {
 			/* exec with arguments */
 			pos = path;
@@ -431,14 +437,16 @@
 					break;
 			}
 			if (args[i]) {
-				dbg("too many args - %d", i);
-				args[i] = NULL;
+				dbg("too many args - %d, using subshell instead '%s'", i, buffer);
+				retval = execl("/bin/sh", "sh", "-c", buffer, NULL);
+			} else {
+				dbg("execute program '%s'", path);
+				retval = execv(args[0], args);
 			}
-			retval = execv(args[0], args);
 		} else {
 			retval = execv(path, main_argv);
 		}
-		dbg("child execve failed");
+		info(FIELD_PROGRAM " execution of '%s' failed", path);
 		exit(1);
 	case -1:
 		dbg("fork failed");
diff -Nru a/test/udev-test.pl b/test/udev-test.pl
--- a/test/udev-test.pl	Thu Feb 26 13:30:02 2004
+++ b/test/udev-test.pl	Thu Feb 26 13:30:02 2004
@@ -244,6 +244,15 @@
 EOF
 	},
 	{
+		desc     => "program with lots of arguments",
+		subsys   => "block",
+		devpath  => "block/sda/sda3",
+		expected => "foo9" ,
+		conf     => <<EOF
+BUS="scsi", PROGRAM="/bin/echo -n foo3 foo4 foo5 foo6 foo7 foo8 foo9", KERNEL="sda3", NAME="%c{7}"
+EOF
+	},
+	{
 		desc     => "program result substitution (numbered part of)",
 		subsys   => "block",
 		devpath  => "block/sda/sda3",
diff -Nru a/udev.8 b/udev.8
--- a/udev.8	Thu Feb 26 13:30:02 2004
+++ b/udev.8	Thu Feb 26 13:30:02 2004
@@ -182,8 +182,7 @@
 .TP
 .B PROGRAM
 Call external program. This key is valid if the program returns successful.
-A few command line options may specified, but shell characters like pipe,
-diversion or similiar options are not available. The environment variables of
+The environment variables of
 .B udev
 are also available for the program.
 .br


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel