udev - simpler PROGRAM execution with arguments

PATCH In review Kay Sievers 3 participants 4 messages Feb 25, 2004 – Feb 26, 2004

Share & tools

Kay Sievers

author

2004-02-25 01:19 UTC #0 lore raw

PATCH 1/1 udev - simpler PROGRAM execution with arguments

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

===== namedev.c 1.112 vs edited =====

namedev.c +7 ~~-16~~

@@ -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 =====

namedev.h +0 -1

@@ -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 =====

udev.8 +1 -2

@@ -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

Patrick Mansfield

2004-02-25 01:34 UTC #1 lore raw

Quoted 5 lines · Kay Sievers

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?

Quoted 2 lines

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_id56&alloc_id438&op=click
_______________________________________________
Linux-hotplug-devel mailing list http://linux-hotplug.sourceforge.net
[email protected]
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Kay Sievers

author

2004-02-25 01:42 UTC #2 lore raw

In reply to Kay Sievers

Quoted 8 lines · Patrick Mansfield

Quoted 5 lines · Kay Sievers
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 :)

Quoted 4 lines

Quoted 2 lines
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

Greg KH

2004-02-26 21:31 UTC #3 lore raw

In reply to Kay Sievers

Quoted 8 lines · Kay Sievers

Quoted 7 lines · Patrick Mansfield
Quoted 5 lines · Kay Sievers
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.

Quoted 4 lines

Quoted 1 line
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.

namedev.c +13 -5

diff -Nru a/namedev.c b/namedev.c
@@ -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");

test/udev-test.pl +9 -0

diff -Nru a/test/udev-test.pl b/test/udev-test.pl
@@ -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",

udev.8 +1 -2

diff -Nru a/udev.8 b/udev.8
@@ -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