* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
@ 2003-11-20 2:59 ` Arnd Bergmann
2003-11-20 3:21 ` Kay Sievers
` (7 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2003-11-20 2:59 UTC (permalink / raw)
To: linux-hotplug
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="windows-1252", Size: 1379 bytes --]
On Thursday 20 November 2003 17:07, Kay Sievers wrote:> On Thu, Nov 20, 2003 at 01:47:36AM +0100, Arnd Bergmann wrote:> > This still doesn't look correct: args[i] will be out of bounds> > when the loop has finished on CALLOUT_MAXARG, and the args> > array is not zero terminated when calling execve.>> Good catch, but arg is not NULL if MAXARG is reached - so args is still> not terminated :)
> + char *args[CALLOUT_MAXARG];> + int i;...> + for (i=0; i < CALLOUT_MAXARG; i++) {> + args[i] = strsep(&arg, " ");> + if (args[i] = NULL)> + break;> + }> + if (args[i]) {> + dbg("to many args");> + args[i] = NULL;> + }
Ok, it's terminated now, but again out of bounds. It should bechar *args[CALLOUT_MAXARG+1];in the beginning or only loop to (CALLOUT_MAXARG - 1).
Arnd <><ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÓ†+\x12\x17ù޵隊X¬²š'²ŠÞu¼ÿH_çzÑ¢½æÚrCë¢Ú›ðèzĨº·\x1e\x16Šà{ùÞ¶\x17¥§*.mé¨êk¡Ûœ¶+Þü:\x1e²+azZr¢ç+y«^mëmz·(uïÒ\x1c\x04DLq\v9QÿjwazZn²\x17¥¥ƒ”ü)brAÞïá¶Úÿÿû(º·\x1e~Šà{ùÞ·÷h«^ÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿÿ.)îÇøh¶™nƒ÷^½éfj)bž b²Øm¶ŸÿþX§»\x1fá¢Úeº\x0fì¢êÜyú+ïçzÒâžìÿ†‹i–èÿuëÞ—ùb²Ûÿ²‹«qçè®\aÿëa¶ÚlÿÿåŠËlþÊ.ÇŸ¢¸\x1eþwþX¬¶ÏåŠËbú?–)îÇøh¶™nƒ÷^
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
2003-11-20 2:59 ` Arnd Bergmann
@ 2003-11-20 3:21 ` Kay Sievers
2003-11-20 12:31 ` Kay Sievers
` (6 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Kay Sievers @ 2003-11-20 3:21 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 774 bytes --]
Sorry, please ignore the previous version it breaks scsi_id by eating
main_argv. Now, with no arguments are given, behavior isn't changed.
here is argument support for CALLOUT exec:
CALLOUT, PROGRAM="/bin/echo -n xxx", BUS="usb", ID="xxx", NAME="webcam%n"
results in:
Nov 20 02:35:20 pim udev[30422]: get_major_minor: found major = 81, minor = 0
Nov 20 02:35:20 pim udev[30422]: exec_callout: callout to /bin/echo -n xxx
Nov 20 02:35:20 pim udev[30422]: exec_callout: callout returned 'xxx'
Nov 20 02:35:20 pim udev[30422]: get_attr: kernel number appended: 0
The feature is really nice, but the maximum argument count is hard coded to 8.
What do you think?
thanks,
Kay
09-namedev.c-callout-argument-support.diff
namdev.c - support for argument in CALLOUT exec
[-- Attachment #2: 09-namedev.c-callout-argument-support.diff --]
[-- Type: text/plain, Size: 1065 bytes --]
--- ../udev/namedev.c 2003-11-19 12:56:50.000000000 +0100
+++ namedev.c 2003-11-20 04:14:54.000000000 +0100
@@ -480,6 +480,9 @@
pid_t pid;
int value_set = 0;
char buffer[256];
+ char *prog;
+ char *args[8];
+ int i;
dbg("callout to %s\n", dev->exec_program);
retval = pipe(fds);
@@ -499,7 +502,19 @@
*/
close(STDOUT_FILENO);
dup(fds[1]); /* dup write side of pipe to STDOUT */
- retval = execve(dev->exec_program, main_argv, main_envp);
+ if (strchr(dev->exec_program, ' ')) {
+ /* callout with arguments */
+ strcpy(buffer, dev->exec_program);
+ prog = strtok(buffer, " ");
+ for(i=1; i<=8; i++) {
+ args[i] = strtok(NULL, " ");
+ if (args[i] == NULL)
+ break;
+ }
+ retval = execve(prog, args, main_envp);
+ } else {
+ retval = execve(dev->exec_program, main_argv, main_envp);
+ }
if (retval != 0) {
dbg("child execve failed");
exit(1);
@@ -528,6 +543,7 @@
strncpy(value, buffer, len);
}
}
+ dbg("callout returned '%s'", value);
close(fds[0]);
res = wait(&status);
if (res < 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
2003-11-20 2:59 ` Arnd Bergmann
2003-11-20 3:21 ` Kay Sievers
@ 2003-11-20 12:31 ` Kay Sievers
2003-11-20 16:07 ` Kay Sievers
` (5 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Kay Sievers @ 2003-11-20 12:31 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 704 bytes --]
Third round :)
Improved version, using strsep() instead of strtok().
here is argument support for CALLOUT exec:
CALLOUT, PROGRAM="/bin/echo -n xxx", BUS="usb", ID="xxx", NAME="webcam%n"
results in:
Nov 20 02:35:20 pim udev[30422]: get_major_minor: found major = 81, minor = 0
Nov 20 02:35:20 pim udev[30422]: exec_callout: callout to /bin/echo -n xxx
Nov 20 02:35:20 pim udev[30422]: exec_callout: callout returned 'xxx'
Nov 20 02:35:20 pim udev[30422]: get_attr: kernel number appended: 0
The feature is really nice, but the maximum argument count is hard coded to 8.
What do you think?
thanks,
Kay
09-namedev.c-callout-argument-support.diff
namdev.c - support for argument in CALLOUT exec
[-- Attachment #2: 09-namedev.c-callout-argument-support.diff --]
[-- Type: text/plain, Size: 1283 bytes --]
--- ../udev/namedev.c 2003-11-19 12:56:50.000000000 +0100
+++ namedev.c 2003-11-20 13:26:11.000000000 +0100
@@ -45,6 +45,7 @@
#define TYPE_TOPOLOGY "TOPOLOGY"
#define TYPE_REPLACE "REPLACE"
#define TYPE_CALLOUT "CALLOUT"
+#define CALLOUT_MAXARG 8
static LIST_HEAD(config_device_list);
@@ -480,6 +481,9 @@
pid_t pid;
int value_set = 0;
char buffer[256];
+ char *arg;
+ char *args[CALLOUT_MAXARG];
+ int i;
dbg("callout to %s\n", dev->exec_program);
retval = pipe(fds);
@@ -499,7 +503,21 @@
*/
close(STDOUT_FILENO);
dup(fds[1]); /* dup write side of pipe to STDOUT */
- retval = execve(dev->exec_program, main_argv, main_envp);
+ if (strchr(dev->exec_program, ' ')) {
+ /* callout with arguments */
+ arg = dev->exec_program;
+ for (i=0; i < CALLOUT_MAXARG; i++) {
+ args[i] = strsep(&arg, " ");
+ if (args[i] == NULL)
+ break;
+ }
+ if (args[i])
+ dbg("to many args");
+
+ retval = execve(args[0], args, main_envp);
+ } else {
+ retval = execve(dev->exec_program, main_argv, main_envp);
+ }
if (retval != 0) {
dbg("child execve failed");
exit(1);
@@ -528,6 +546,7 @@
strncpy(value, buffer, len);
}
}
+ dbg("callout returned '%s'", value);
close(fds[0]);
res = wait(&status);
if (res < 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (2 preceding siblings ...)
2003-11-20 12:31 ` Kay Sievers
@ 2003-11-20 16:07 ` Kay Sievers
2003-11-20 17:31 ` Kay Sievers
` (4 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Kay Sievers @ 2003-11-20 16:07 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On Thu, Nov 20, 2003 at 01:47:36AM +0100, Arnd Bergmann wrote:
> On Thursday 20 November 2003 13:31, Kay Sievers wrote:
> > + for (i=0; i < CALLOUT_MAXARG; i++) {
> > + args[i] = strsep(&arg, " ");
> > + if (args[i] == NULL)
> > + break;
> > + }
> > + if (args[i])
> > + dbg("to many args");
>
> This still doesn't look correct: args[i] will be out of bounds
> when the loop has finished on CALLOUT_MAXARG, and the args
> array is not zero terminated when calling execve.
>
> I haven't tried it yet, but I think this would make more sense:
>
> + for (i=0; i < CALLOUT_MAXARG-1; i++) {
> + args[i] = strsep(&arg, " ");
> + if (args[i] == NULL)
> + break;
> + }
> + args[i] = arg;
> + if (args[i])
> + dbg("to many args");
Good catch, but arg is not NULL if MAXARG is reached - so args is still
not terminated :)
Corrected patch attached.
Grüße
Kay
[-- Attachment #2: 09-namedev.c-callout-argument-support.diff --]
[-- Type: text/plain, Size: 1310 bytes --]
--- ../udev/namedev.c 2003-11-19 12:56:50.000000000 +0100
+++ namedev.c 2003-11-20 16:41:49.000000000 +0100
@@ -45,6 +45,7 @@
#define TYPE_TOPOLOGY "TOPOLOGY"
#define TYPE_REPLACE "REPLACE"
#define TYPE_CALLOUT "CALLOUT"
+#define CALLOUT_MAXARG 8
static LIST_HEAD(config_device_list);
@@ -480,6 +481,9 @@
pid_t pid;
int value_set = 0;
char buffer[256];
+ char *arg;
+ char *args[CALLOUT_MAXARG];
+ int i;
dbg("callout to %s\n", dev->exec_program);
retval = pipe(fds);
@@ -499,7 +503,22 @@
*/
close(STDOUT_FILENO);
dup(fds[1]); /* dup write side of pipe to STDOUT */
- retval = execve(dev->exec_program, main_argv, main_envp);
+ if (strchr(dev->exec_program, ' ')) {
+ /* callout with arguments */
+ arg = dev->exec_program;
+ for (i=0; i < CALLOUT_MAXARG; i++) {
+ args[i] = strsep(&arg, " ");
+ if (args[i] == NULL)
+ break;
+ }
+ if (args[i]) {
+ dbg("to many args");
+ args[i] = NULL;
+ }
+ retval = execve(args[0], args, main_envp);
+ } else {
+ retval = execve(dev->exec_program, main_argv, main_envp);
+ }
if (retval != 0) {
dbg("child execve failed");
exit(1);
@@ -528,6 +547,7 @@
strncpy(value, buffer, len);
}
}
+ dbg("callout returned '%s'", value);
close(fds[0]);
res = wait(&status);
if (res < 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (3 preceding siblings ...)
2003-11-20 16:07 ` Kay Sievers
@ 2003-11-20 17:31 ` Kay Sievers
2003-11-20 20:14 ` Arnd Bergmann
` (3 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Kay Sievers @ 2003-11-20 17:31 UTC (permalink / raw)
To: linux-hotplug
[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]
On Thu, Nov 20, 2003 at 03:59:31AM +0100, Arnd Bergmann wrote:
> On Thursday 20 November 2003 17:07, Kay Sievers wrote:
> > On Thu, Nov 20, 2003 at 01:47:36AM +0100, Arnd Bergmann wrote:
> > > This still doesn't look correct: args[i] will be out of bounds
> > > when the loop has finished on CALLOUT_MAXARG, and the args
> > > array is not zero terminated when calling execve.
> >
> > Good catch, but arg is not NULL if MAXARG is reached - so args is still
> > not terminated :)
>
> > + char *args[CALLOUT_MAXARG];
> > + int i;
> ...
> > + for (i=0; i < CALLOUT_MAXARG; i++) {
> > + args[i] = strsep(&arg, " ");
> > + if (args[i] == NULL)
> > + break;
> > + }
> > + if (args[i]) {
> > + dbg("to many args");
> > + args[i] = NULL;
> > + }
>
> Ok, it's terminated now, but again out of bounds. It should be
> char *args[CALLOUT_MAXARG+1];
> in the beginning or only loop to (CALLOUT_MAXARG - 1).
Oh, oh, oh...
I hope, I don't send more patches than the number of lines added to the code.
Here is #5 :)
thanks,
Kay
[-- Attachment #2: 09-namedev.c-callout-argument-support.diff --]
[-- Type: text/plain, Size: 1320 bytes --]
--- ../udev/namedev.c 2003-11-19 12:56:50.000000000 +0100
+++ namedev.c 2003-11-20 18:29:42.000000000 +0100
@@ -45,6 +45,7 @@
#define TYPE_TOPOLOGY "TOPOLOGY"
#define TYPE_REPLACE "REPLACE"
#define TYPE_CALLOUT "CALLOUT"
+#define CALLOUT_MAXARG 8
static LIST_HEAD(config_device_list);
@@ -480,6 +481,9 @@
pid_t pid;
int value_set = 0;
char buffer[256];
+ char *arg;
+ char *args[CALLOUT_MAXARG];
+ int i;
dbg("callout to %s\n", dev->exec_program);
retval = pipe(fds);
@@ -499,7 +503,22 @@
*/
close(STDOUT_FILENO);
dup(fds[1]); /* dup write side of pipe to STDOUT */
- retval = execve(dev->exec_program, main_argv, main_envp);
+ if (strchr(dev->exec_program, ' ')) {
+ /* callout with arguments */
+ arg = dev->exec_program;
+ for (i=0; i < CALLOUT_MAXARG-1; i++) {
+ args[i] = strsep(&arg, " ");
+ if (args[i] == NULL)
+ break;
+ }
+ if (args[i]) {
+ dbg("to many args - %d", i);
+ args[i] = NULL;
+ }
+ retval = execve(args[0], args, main_envp);
+ } else {
+ retval = execve(dev->exec_program, main_argv, main_envp);
+ }
if (retval != 0) {
dbg("child execve failed");
exit(1);
@@ -528,6 +547,7 @@
strncpy(value, buffer, len);
}
}
+ dbg("callout returned '%s'", value);
close(fds[0]);
res = wait(&status);
if (res < 0) {
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (4 preceding siblings ...)
2003-11-20 17:31 ` Kay Sievers
@ 2003-11-20 20:14 ` Arnd Bergmann
2003-11-21 6:50 ` Greg KH
` (2 subsequent siblings)
8 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2003-11-20 20:14 UTC (permalink / raw)
To: linux-hotplug
On Friday 21 November 2003 07:53, Greg KH wrote:
> On Thu, Nov 20, 2003 at 01:47:36AM +0100, Arnd Bergmann wrote:
> > CALLOUT, PROGRAM="sh -c 'echo ${DEVPATH} | tr a-z A-Z'", BUS="usb",
> > ID="XXX", NAME="webcam%n"
>
> Oh come on, why not just make that into a simple script, we're trying to
> keep udev simple :)
Ok, that makes sense.
> > To take that even further, do you think it might be a good idea to
> > preprocess the string in the same way as the NAME= argument, allowing
> > e.g. PROGRAM="/bin/devname %b %M:%m" ?
>
> Hm, good idea.
>
> Someone else also proposed adding a number modifier to the %b field to
> pick out a specific character from a bus id. Anyone have any thoughts
> about this? Personally, I think it will be a bit hard to handle bus ids
> that are not fixed widths all the time, but might be handy to try to
> implement a devfs-like udev config file.
When we allow %b in PROGRAM= and find a good way to use the output of
the program as the device name, there won't be any need for such a hack,
as it can better be done in an external script. Getting it right in
udev would be too complicated IMHO (both usage and implementation).
Arnd <><
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (5 preceding siblings ...)
2003-11-20 20:14 ` Arnd Bergmann
@ 2003-11-21 6:50 ` Greg KH
2003-11-21 6:53 ` Greg KH
2003-11-22 18:14 ` Greg KH
8 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2003-11-21 6:50 UTC (permalink / raw)
To: linux-hotplug
On Thu, Nov 20, 2003 at 06:31:42PM +0100, Kay Sievers wrote:
> Oh, oh, oh...
> I hope, I don't send more patches than the number of lines added to the code.
> Here is #5 :)
Heh, this looks good, I've applied it.
thanks,
greg k-h
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (6 preceding siblings ...)
2003-11-21 6:50 ` Greg KH
@ 2003-11-21 6:53 ` Greg KH
2003-11-22 18:14 ` Greg KH
8 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2003-11-21 6:53 UTC (permalink / raw)
To: linux-hotplug
On Thu, Nov 20, 2003 at 01:47:36AM +0100, Arnd Bergmann wrote:
>
> Aside from that, it looks good. Maybe there should be a way to escape
> spaces in the argument in case someone wants to do something like that:
>
> CALLOUT, PROGRAM="sh -c 'echo ${DEVPATH} | tr a-z A-Z'", BUS="usb", ID="XXX", NAME="webcam%n"
Oh come on, why not just make that into a simple script, we're trying to
keep udev simple :)
> To take that even further, do you think it might be a good idea to
> preprocess the string in the same way as the NAME= argument, allowing e.g.
> PROGRAM="/bin/devname %b %M:%m" ?
Hm, good idea.
Someone else also proposed adding a number modifier to the %b field to
pick out a specific character from a bus id. Anyone have any thoughts
about this? Personally, I think it will be a bit hard to handle bus ids
that are not fixed widths all the time, but might be handy to try to
implement a devfs-like udev config file.
thanks,
greg k-h
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [udev] support arguments in callout exec
2003-11-20 0:47 Arnd Bergmann
` (7 preceding siblings ...)
2003-11-21 6:53 ` Greg KH
@ 2003-11-22 18:14 ` Greg KH
8 siblings, 0 replies; 11+ messages in thread
From: Greg KH @ 2003-11-22 18:14 UTC (permalink / raw)
To: linux-hotplug
On Thu, Nov 20, 2003 at 09:14:43PM +0100, Arnd Bergmann wrote:
> > > To take that even further, do you think it might be a good idea to
> > > preprocess the string in the same way as the NAME= argument, allowing
> > > e.g. PROGRAM="/bin/devname %b %M:%m" ?
> >
> > Hm, good idea.
> >
> > Someone else also proposed adding a number modifier to the %b field to
> > pick out a specific character from a bus id. Anyone have any thoughts
> > about this? Personally, I think it will be a bit hard to handle bus ids
> > that are not fixed widths all the time, but might be handy to try to
> > implement a devfs-like udev config file.
>
> When we allow %b in PROGRAM= and find a good way to use the output of
> the program as the device name, there won't be any need for such a hack,
> as it can better be done in an external script. Getting it right in
> udev would be too complicated IMHO (both usage and implementation).
Good point, I'll try to work on adding that this weekend if I get a
break from changing diapers...
greg k-h
-------------------------------------------------------
This SF.net email is sponsored by: SF.net Giveback Program.
Does SourceForge.net help you be more productive? Does it
help you create better code? SHARE THE LOVE, and help us help
YOU! Click Here: http://sourceforge.net/donate/
_______________________________________________
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
^ permalink raw reply [flat|nested] 11+ messages in thread