[PATCH] update: making "test" an ash built-in

Natanael Copa natanael.copa at gmail.com
Thu Jun 8 11:54:16 UTC 2006


On Wed, 2006-06-07 at 17:04 -0400, Paul Fox wrote:
> hi --
> 
> the help text for CONFIG_TEST currently says that "test" is a
> builtin command in ash.  but it's not, and on slower systems the
> performance hit is noticeable.

Even on my Pentium D the performance hit was so huge that busybox ash
was completely unuseable.

> it seems to work, and running test in a tight shell loop is
> between 75 and 150 times faster (depending on whether the test
> conditions are accessing the filesystem or not).  this is on
> a slow ARM with little cache.  better machines will show less
> improvement, i'm sure.

The difference was big even on faster computers.

> any objections if i commit this?

You need builtins for '[' and '[[' too. Please see the attatched update
patch.

Its agains current svn.

The patch below is just for an overview what differs the old
coreutils/test.c with the new libbb/bb_test.c.

Note that I replaced the rest of the bb_error_msg_and_die with only
bb_error_msg. You would not like the entire shell die just because you
entered wrong number of params to 'test', '[' or '[['.

I would love to see it committed.

--- coreutils/test.c	2006-06-08 13:44:52.687869936 +0200
+++ libbb/bb_test.c	2006-06-08 13:27:28.202655824 +0200
@@ -37,6 +37,7 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
+#include <setjmp.h>
 #include "busybox.h"
 
 /* test(1) accepts the following grammar:
@@ -182,36 +183,49 @@
 static int is_a_group_member(gid_t gid);
 static void initialize_group_array(void);
 
-int test_main(int argc, char **argv)
+static jmp_buf leaving;
+
+int bb_test(const char *closebracket, int argc, char **argv)
 {
 	int res;
 
-	if (strcmp(bb_applet_name, "[") == 0) {
-		if (strcmp(argv[--argc], "]"))
-			bb_error_msg_and_die("missing ]");
-		argv[argc] = NULL;
-	}
-	if (strcmp(bb_applet_name, "[[") == 0) {
-		if (strcmp(argv[--argc], "]]"))
-			bb_error_msg_and_die("missing ]]");
+	if (closebracket) {
+		if (strcmp(argv[--argc], closebracket)) {
+			bb_error_msg("missing %s", closebracket);
+			return 2;
+		}
 		argv[argc] = NULL;
 	}
+	res = setjmp(leaving);
+	if (res)
+		return res;
+
+	/* resetting ngroups is probably unnecessary.  it will
+	 * force a new call to getgroups(), which prevents using
+	 * group data fetched during a previous call.  but the
+	 * only way the group data could be stale is if there's
+	 * been an intervening call to setgroups(), and this
+	 * isn't likely in the case of a shell.  paranoia
+	 * prevails...
+	 */
+        ngroups = 0;
+
 	/* Implement special cases from POSIX.2, section 4.62.4 */
 	switch (argc) {
 	case 1:
-		exit(1);
+		return 1;
 	case 2:
-		exit(*argv[1] == '\0');
+		return *argv[1] == '\0';
 	case 3:
 		if (argv[1][0] == '!' && argv[1][1] == '\0') {
-			exit(!(*argv[2] == '\0'));
+			return *argv[2] != '\0';
 		}
 		break;
 	case 4:
 		if (argv[1][0] != '!' || argv[1][1] != '\0') {
 			if (t_lex(argv[2]), t_wp_op && t_wp_op->op_type == BINOP) {
 				t_wp = &argv[1];
-				exit(binop() == 0);
+				return binop() == 0;
 			}
 		}
 		break;
@@ -219,7 +233,7 @@
 		if (argv[1][0] == '!' && argv[1][1] == '\0') {
 			if (t_lex(argv[3]), t_wp_op && t_wp_op->op_type == BINOP) {
 				t_wp = &argv[2];
-				exit(!(binop() == 0));
+				return binop() != 0;
 			}
 		}
 		break;
@@ -228,10 +242,11 @@
 	t_wp = &argv[1];
 	res = !oexpr(t_lex(*t_wp));
 
-	if (*t_wp != NULL && *++t_wp != NULL)
-		bb_error_msg_and_die("%s: unknown operand", *t_wp);
-
-	return (res);
+	if (*t_wp != NULL && *++t_wp != NULL) {
+		bb_error_msg("%s: unknown operand", *t_wp);
+		longjmp(leaving, 2);
+	}
+	return res;
 }
 
 static arith_t oexpr(enum token n)
@@ -269,18 +284,23 @@
 	arith_t res;
 
 	if (n == EOI) {
-		bb_error_msg_and_die("argument expected");
+		bb_error_msg("argument expected");
+		longjmp(leaving, 2);
 	}
 	if (n == LPAREN) {
 		res = oexpr(t_lex(*++t_wp));
-		if (t_lex(*++t_wp) != RPAREN)
-			bb_error_msg_and_die("closing paren expected");
+		if (t_lex(*++t_wp) != RPAREN) {
+			bb_error_msg("closing paren expected");
+			longjmp(leaving, 2);
+		}
 		return res;
 	}
 	if (t_wp_op && t_wp_op->op_type == UNOP) {
 		/* unary expression */
-		if (*++t_wp == NULL)
-			bb_error_msg_and_die(bb_msg_requires_arg, t_wp_op->op_text);
+		if (*++t_wp == NULL) {
+			bb_error_msg(bb_msg_requires_arg, t_wp_op->op_text);
+			longjmp(leaving, 2);
+		}
 		switch (n) {
 		case STREZ:
 			return strlen(*t_wp) == 0;
@@ -309,8 +329,10 @@
 	(void) t_lex(*++t_wp);
 	op = t_wp_op;
 
-	if ((opnd2 = *++t_wp) == (char *) 0)
-		bb_error_msg_and_die(bb_msg_requires_arg, op->op_text);
+	if ((opnd2 = *++t_wp) == (char *) 0) {
+		bb_error_msg(bb_msg_requires_arg, op->op_text);
+		longjmp(leaving, 2);
+	}
 
 	switch (op->op_num) {
 	case STREQ:
@@ -459,12 +481,16 @@
 	r = strtol(s, &p, 10);
 #endif
 
-	if (errno != 0)
-		bb_error_msg_and_die("%s: out of range", s);
+	if (errno != 0) {
+		bb_error_msg("%s: out of range", s);
+		longjmp(leaving, 2);
+	}
 
 	/*   p = bb_skip_whitespace(p); avoid const warning */
-	if (*(bb_skip_whitespace(p)))
-		bb_error_msg_and_die("%s: bad number", s);
+	if (*(bb_skip_whitespace(p))) {
+		bb_error_msg("%s: bad number", s);
+		longjmp(leaving, 2);
+	}
 
 	return r;
 }


--
Natanael Copa
-------------- next part --------------
A non-text attachment was scrubbed...
Name: bb-ash-test-brackets.patch
Type: text/x-patch
Size: 28122 bytes
Desc: not available
Url : http://lists.busybox.net/pipermail/busybox/attachments/20060608/85f4dc84/attachment.bin 


More information about the busybox mailing list