On Saturday 29 September 2007 04:14, Loïc Grenié wrote:
> 
>     Here it is. I've tried to take care of all the remarks.
> 
>        Loïc

 /* How to determine who we are? find 3rd char from the end:
  * kill, killall, killall5
- *  ^i       ^a        ^l  - it's unique
- * (checking from the start is complicated by /bin/kill... case) */
-       const char char3 = argv[0][strlen(argv[0]) - 3];
-#define killall (ENABLE_KILLALL && char3 == 'a')
-#define killall5 (ENABLE_KILLALL5 && char3 == 'l')
+ *   4      7        8     - the number of characters of the name is unique
+ */
+       const char namelen = strlen(applet_name);
+#define killall (ENABLE_KILLALL && namelen == 7)
+#define killall5 (ENABLE_KILLALL5 && namelen == 8)
 #endif

You didn't read kill.c header comment carefully. kill.c is special.
applet_name may not be set correctly for it.

+       int scan_mask = PSSCAN_STAT;

You mean, PSSCAN_COMM - "I want to know COMM field". Which brings a question:
what will happen to processes with 15+ char names (they get truncated comm 
field)? 

+       const int NMATCH = 2;
+       regmatch_t re_regs[NMATCH];

Why do you need to find first TWO matches?

+       if (pkill && list) { // -l
+               if (argc || opt & ~0x01)
+                       bb_perror_nomsg_and_die();
+               /* Print the whole signal list */
+               print_signames_and_exit();
+       }

What perror is supposed to print here? What value is in errno here?
I will simply do:
       if (pkill) {
                if (OPT_LIST) /* -l: print the whole signal list */
                        print_signames_and_exit();
                ....


+               if (p->pid != pid &&
+                       (regexec(&re_buffer, cmd, NMATCH, re_regs, 0) != 
REG_NOMATCH &&
+                               (OPT_ANCHOR ||
+                                (re_regs[0].rm_so == 0 && re_regs[0].rm_eo == 
strlen(cmd))))
+                               ^ OPT_INVERT)
+               {

Readability took a dive here. You fell victim of it too, i think:
bug, should be !OPT_ANCHOR instead of OPT_ANCHOR.


I applied a modified version of it to svn. Thanks.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox

Reply via email to