Re: [Crash-utility] [PATCH] add option -E for foreach

Hello Dave, I have modified my patch. In my opinion, regular expression should not mix with other task-identifying arguments, so I made some more modifications

At 2012-2-29 1:01, Dave Anderson wrote: > > > ----- Original Message ----- >> Hello Dave, >> >> The patch is used to add a new option, "-E regex", to perform the >> command(s) on all commands whose names contains a match to "regex". >> "regex" is interpreted as an POSIX extended regular expression. >> >> The new option is helpful when using to search some commands whose names >> are similar. And I think it is more reliable than "foreach<name> >> <command> | grep xxx", for information comes from command may have a >> match to "xxx". >> >> P.S. >> The patch uses some function defined in regex.h, which is offered by glibc. > > The concept is a pretty good idea. However: > > (1) It doesn't work as you've described it in the help page, and > (2) I don't like the invocation, because option characters are > meant to be passed to the selected command, and not for > consumption by the foreach command itself. > > With respect to (1), your help page example shows this: > > crash> help foreach > ... [ cut ] ... > Display the state of tasks whose name contains a match to > 'event.*': > > crash> foreach -E 'event.*' task -R state > PID: 99 TASK: ffff8804750d5500 CPU: 0 COMMAND: "events/0" > state = 1, > > PID: 100 TASK: ffff8804750d4ac0 CPU: 1 COMMAND: "events/1" > state = 1, > > PID: 101 TASK: ffff8804750d4080 CPU: 2 COMMAND: "events/2" > state = 1, > ... > > But your example doesn't work with the encompassing "'" characters: > > crash> foreach -E 'event.*' task -R state > crash> > > If I remove the encompassing "'" characters, it does work: > > crash> foreach -E event.* task -R state > PID: 67 TASK: ffff88033833d500 CPU: 0 COMMAND: "events/0" > state = 1, > > PID: 68 TASK: ffff88033833cac0 CPU: 1 COMMAND: "events/1" > state = 1, > > PID: 69 TASK: ffff88033833c080 CPU: 2 COMMAND: "events/2" > state = 1, > > PID: 70 TASK: ffff880338367540 CPU: 3 COMMAND: "events/3" > state = 1, > > ... > > So apparently regcomp() does not handle strings encompassed with > the "'" characters. > > With respect to (2), however, since the process name option already > has the "\" invocation option to prevent ambiguity with the foreach > command name: > > crash> help foreach > ... > name perform the command(s) on all commands with this name. If the > command name can be confused with a foreach command name, then > precede the name string with a "\". > ... > > Perhaps there can be a third way of specifying the "name" option, where a > regular expression *must* be encompassed by "'" characters, and therefore: > > (a) the argument can be recognized as a POSIX expression, and > (b) the encompassing "'" characters can be stripped off before passing the > argument string to regcomp(). > > So then you could simply enter: > > crash> foreach 'event.*' task -R state > > And have it described in the help page something like: > > crash> help foreach > ... > name perform the command(s) on all processes with this name. If the > process name can be confused with a foreach command name, then > precede the name string with a "\". If the process name is > enclosed within "'" characters, then the encompassed string > is a POSIX extended regular expression that will be used to > match process names. > ... > > Then you can simplify things by replacing the FOREACH_E_FLAG usage with > a FOREACH_REGEX flag that can be set in cmd_foreach() and checked > in foreach(). > > Dave