Linux Archive

Linux Archive (http://www.linux-archive.org/)
-   Crash Utility (http://www.linux-archive.org/crash-utility/)
-   -   add option -E for foreach (http://www.linux-archive.org/crash-utility/638658-add-option-e-foreach.html)

Dave Anderson 02-28-2012 04:01 PM

add option -E for foreach
 
----- 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

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 02-29-2012 02:28 PM

add option -E for foreach
 
----- Original Message -----
> 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

Why not? It would be far more useful if you could maintain
the current capability of providing a mixture of pid/task/name
task-identifiers along with this new regular-expression-matching
scheme.

Dave

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

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 02-29-2012 02:49 PM

add option -E for foreach
 
----- Original Message -----
>
>
> ----- Original Message -----
> > 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
>
> Why not? It would be far more useful if you could maintain
> the current capability of providing a mixture of pid/task/name
> task-identifiers along with this new regular-expression-matching
> scheme.
>
> Dave

In fact, I've just reworked (and simplified) your patch so that a
'regex' task identifier may be used in conjunction with the other
task identifier types, make the capability much more useful.

In fact, it could be modified even further to allow multiple
regular expression arguments to be allowed, by maintaining
an regex_t array the same way as it's done for the other
supported task identifiers. I'll try that...

Dave

--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility

Dave Anderson 02-29-2012 03:15 PM

add option -E for foreach
 
----- Original Message -----
>
>
> ----- Original Message -----
> >
> >
> > ----- Original Message -----
> > > 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
> >
> > Why not? It would be far more useful if you could maintain
> > the current capability of providing a mixture of pid/task/name
> > task-identifiers along with this new regular-expression-matching
> > scheme.
> >
> > Dave
>
> In fact, I've just reworked (and simplified) your patch so that a
> 'regex' task identifier may be used in conjunction with the other
> task identifier types, make the capability much more useful.
>
> In fact, it could be modified even further to allow multiple
> regular expression arguments to be allowed, by maintaining
> an regex_t array the same way as it's done for the other
> supported task identifiers. I'll try that...
>
> Dave

Done! With that capability, this a pretty powerful task-identifier.
Thanks for getting the ball rolling -- queued for crash-6.0.5.

Thanks,
Dave


--
Crash-utility mailing list
Crash-utility@redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility


All times are GMT. The time now is 04:16 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.