From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kay Sievers Date: Wed, 10 Mar 2004 23:55:35 +0000 Subject: [PATCH] cleanup callout fork Message-Id: <20040310235535.GA31657@vrfy.org> MIME-Version: 1 Content-Type: multipart/mixed; boundary="M9NhX3UHpAaciwkO" List-Id: To: linux-hotplug@vger.kernel.org --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Here I change the callout fork logic. The current cersion is unable to read a pipe which is not flushed at once, Now we read until it's closed. The maximum argument count is calculated by the strlen now. We have 100 chars for our result buffer so we can't have more than 50 parameters. So it's much more clear what will happen now and not some magic boundary where we use shell behind it. Parameter can be combined to one by using apostrophes. this on works now: BUS="scsi", PROGRAM="/bin/sh -c 'echo foo3 foo4 foo5 foo6 foo7 foo8 foo9 | sed s/foo9/bar9/'", KERNEL="sda3", NAME="%c{7}" Two new test are also added. thanks, Kay --M9NhX3UHpAaciwkO Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="01-callout-cleanup.patch" ===== namedev.c 1.127 vs edited ===== --- 1.127/namedev.c Wed Mar 10 21:04:30 2004 +++ edited/namedev.c Thu Mar 11 00:42:42 2004 @@ -370,22 +370,41 @@ static int execute_program(char *path, char *value, int len) { int retval; - int res; + int count; int status; int fds[2]; pid_t pid; - int value_set = 0; - char buffer[255]; char *pos; - char *args[PROGRAM_MAXARG]; + char arg[PROGRAM_SIZE]; + char *argv[sizeof(arg) / 2]; int i; - dbg("executing '%s'", path); + i = 0; + if (strchr(path, ' ')) { + strfieldcpy(arg, path); + pos = arg; + while (pos != NULL) { + if (pos[0] == '\'') { + /* don't separate if in apostrophes */ + pos++; + argv[i] = strsep(&pos, "\'"); + while (pos[0] == ' ') + pos++; + } else { + argv[i] = strsep(&pos, " "); + } + dbg("arg[%i] '%s'", i, argv[i]); + i++; + } + } + argv[i] = NULL; + retval = pipe(fds); if (retval != 0) { dbg("pipe failed"); return -1; } + pid = fork(); switch(pid) { case 0: @@ -394,28 +413,14 @@ /* dup write side of pipe to STDOUT */ dup(fds[1]); - - /* copy off our path to use incase we have too many args */ - strfieldcpymax(buffer, path, sizeof(buffer)); - - 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, 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); - } + if (argv[0] != NULL) { + dbg("execute '%s' with given arguments", argv[0]); + retval = execv(argv[0], argv); } else { + dbg("execute '%s' with main argument", path); retval = execv(path, main_argv); } + info(FIELD_PROGRAM " execution of '%s' failed", path); exit(1); case -1: @@ -425,34 +430,33 @@ /* parent reads from fds[0] */ close(fds[1]); retval = 0; + i = 0; while (1) { - res = read(fds[0], buffer, sizeof(buffer) - 1); - if (res <= 0) + count = read(fds[0], value + i, len - i-1); + if (count <= 0) break; - buffer[res] = '\0'; - if (res > len) { + + i += count; + if (i >= len-1) { dbg("result len %d too short", len); retval = -1; - } - if (value_set) { - dbg("result value already set"); - retval = -1; - } else { - value_set = 1; - strncpy(value, buffer, len); - pos = value + strlen(value)-1; - if (pos[0] == '\n') - pos[0] = '\0'; - dbg("result is '%s'", value); + break; } } - close(fds[0]); - res = wait(&status); - if (res < 0) { - dbg("wait failed result %d", res); + + if (count < 0) { + dbg("read failed with '%s'", strerror(errno)); retval = -1; } + if (i > 0 && value[i] == '\n') + i--; + value[i] = '\0'; + dbg("result is '%s'", value); + + close(fds[0]); + wait(&status); + if (!WIFEXITED(status) || (WEXITSTATUS(status) != 0)) { dbg("exec program status 0x%x", status); retval = -1; @@ -730,7 +734,7 @@ apply_format(udev, dev->program, sizeof(dev->program), class_dev, sysfs_device); if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) { - dbg(FIELD_PROGRAM " returned nozero"); + dbg(FIELD_PROGRAM " returned nonzero"); goto try_parent; } else { dbg(FIELD_PROGRAM " returned successful"); ===== namedev.h 1.28 vs edited ===== --- 1.28/namedev.h Wed Mar 10 21:04:30 2004 +++ edited/namedev.h Wed Mar 10 22:00:15 2004 @@ -51,7 +51,6 @@ #define ATTR_PARTITIONS "all_partitions" #define PARTITIONS_COUNT 15 -#define PROGRAM_MAXARG 10 #define MAX_SYSFS_PAIRS 5 #define RULEFILE_EXT ".rules" ===== test/udev-test.pl 1.42 vs edited ===== --- 1.42/test/udev-test.pl Thu Mar 4 14:38:53 2004 +++ edited/test/udev-test.pl Thu Mar 11 00:44:53 2004 @@ -262,6 +262,24 @@ EOF }, { + desc => "program with subshell", + subsys => "block", + devpath => "block/sda/sda3", + expected => "bar9" , + conf => < "program arguments combined with apostrophes", + subsys => "block", + devpath => "block/sda/sda3", + expected => "foo7" , + conf => < "characters before the %c{N} substitution", subsys => "block", devpath => "block/sda/sda3", --M9NhX3UHpAaciwkO-- ------------------------------------------------------- This SF.Net email is sponsored by: IBM Linux Tutorials Free Linux tutorial presented by Daniel Robbins, President and CEO of GenToo technologies. Learn everything from fundamentals to system administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&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