Re: [PATCH] add option -E for foreach

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

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




--
--
Regards
Qiao Nuohan
diff --git a/defs.h b/defs.h
index a942dbb..fd07f66 100755
--- a/defs.h
+++ b/defs.h
@@ -47,6 +47,7 @@
 #include <sys/wait.h>
 #include <sys/time.h>
 #include <execinfo.h> /* backtrace() */
+#include <regex.h>
 
 #ifndef ATTRIBUTE_UNUSED
 #define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
@@ -977,6 +978,7 @@ extern struct machdep_table *machdep;
 #define FOREACH_F_FLAG  (0x400000)
 #define FOREACH_x_FLAG  (0x800000)
 #define FOREACH_d_FLAG (0x1000000)
+#define FOREACH_REGEX  (0x2000000)
 
 struct foreach_data {
 	ulong flags;
@@ -985,6 +987,7 @@ struct foreach_data {
         char *comm_array[MAX_FOREACH_COMMS];
         ulong pid_array[MAX_FOREACH_PIDS];
 	ulong arg_array[MAX_FOREACH_ARGS];
+	regex_t regex;
 	char *reference;
 	int keys;
 	int pids;
diff --git a/help.c b/help.c
index f752283..d33b318 100755
--- a/help.c
+++ b/help.c
@@ -734,7 +734,10 @@ char *help_foreach[] = {
 "           task_struct pointer.",
 "     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 \"\\\".",
+"           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",
+"           thread names. Notice that 'name' can only be specified individually.",
 "     user  perform the command(s) on all user (non-kernel) threads.",
 "   kernel  perform the command(s) on all kernel threads.",
 "   active  perform the command(s) on the active thread on each CPU.\n",
@@ -794,6 +797,17 @@ char *help_foreach[] = {
 "  Display the open files for all tasks:\n",
 "    %s> foreach files",
 "    ...\n",
+"  Display the state of tasks whose name contains a match to \"event.*\":\n",
+"    %s> foreach '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,",
+"    ...\n",
 NULL
 };
 
diff --git a/task.c b/task.c
index fde86eb..bf0baa5 100755
--- a/task.c
+++ b/task.c
@@ -5021,6 +5021,9 @@ cmd_foreach(void)
 			switch (str_to_context(args[optind], &value, &tc))
 			{
 			case STR_PID:
+                                if (fd->flags & FOREACH_REGEX)
+                                        error(FATAL,
+                                            "'name' can only be specified individually!\n");
                                 if (p == MAX_FOREACH_PIDS)
                                         error(INFO,
                                             "too many pids specified!\n");
@@ -5033,6 +5036,9 @@ cmd_foreach(void)
 				break;
 
 			case STR_TASK:
+                                if (fd->flags & FOREACH_REGEX)
+                                        error(FATAL,
+                                            "'name' can only be specified individually!\n");
                                 if (t == MAX_FOREACH_TASKS)
                                         error(INFO,
                                             "too many tasks specified!\n");
@@ -5053,6 +5059,9 @@ cmd_foreach(void)
 		 *  Select all kernel threads.
 		 */
 		if (STREQ(args[optind], "kernel")) {
+                        if (fd->flags & FOREACH_REGEX)
+                                error(FATAL,
+                                   "'name' can only be specified individually!\n");
 			if (fd->flags & FOREACH_USER)
 				error(FATAL, 
 				   "user and kernel are mutually exclusive!\n");
@@ -5065,6 +5074,9 @@ cmd_foreach(void)
 		 *  Select only user threads.
 		 */
                 if (STREQ(args[optind], "user")) {
+                        if (fd->flags & FOREACH_REGEX)
+                                error(FATAL,
+                                   "'name' can only be specified individually!\n");
                         if (fd->flags & FOREACH_KERNEL)
                                 error(FATAL, 
                                    "user and kernel are mutually exclusive!\n");
@@ -5077,6 +5089,9 @@ cmd_foreach(void)
 		 *  Select only active tasks (dumpfile only)
 	  	 */
                 if (STREQ(args[optind], "active")) {
+                        if (fd->flags & FOREACH_REGEX)
+                                error(FATAL,
+                                 "'name' can only be specified individually!\n");
 			if (!DUMPFILE())
 				error(FATAL, 
 				 "active option not allowed on live systems\n");
@@ -5086,12 +5101,34 @@ cmd_foreach(void)
                 }
 
 		/*
+		 *  Regular expression is exclosed within "'" character.
+		 */
+		if (args[optind][0] == '\'' &&
+				args[optind][strlen(args[optind])-1] == '\'') {
+			if (fd->flags & FOREACH_REGEX || fd->flags & FOREACH_SPECIFIED ||
+				fd->flags & FOREACH_KERNEL || fd->flags & FOREACH_USER ||
+				fd->flags & FOREACH_ACTIVE)
+                                error(FATAL, "'name' can only be specified individually!\n");
+			
+			args[optind][strlen(args[optind])-1] = '\0';
+			p1 = args[optind] + 1;
+			
+			if (regcomp(&fd->regex, p1, REG_EXTENDED|REG_NOSUB))
+				error(FATAL, "invalid regular expression!\n");
+			fd->flags |= FOREACH_REGEX;
+			optind++;
+			continue;
+		}
+
+		/*
 	         *  If it's a command name, prefixed or otherwise, take it.
 		 */
 		p1 = (args[optind][0] == '\\') ? 
 			&args[optind][1] : args[optind];
 
 		if (comm_exists(p1)) {
+                        if (fd->flags & FOREACH_REGEX)
+                                error(FATAL, "'name' can only be specified individually!\n");
 			if (c == MAX_FOREACH_COMMS)
 				error(INFO, "too many commands specified!\n");
 			else {
@@ -5335,6 +5372,10 @@ foreach(struct foreach_data *fd)
 				if (STREQ(fd->comm_array[j], tc->comm))
 					doit = TRUE;
 		}
+		else if (fd->flags & FOREACH_REGEX) {
+			if (regexec(&fd->regex, tc->comm, 0, NULL, 0) == 0)
+				doit = TRUE;
+		}
 		else 
 			doit = TRUE;
 
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility

[Index of Archives]     [Fedora Development]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]

 

Powered by Linux