bd28f6bf7

Denis Vlasenko

2008-07-19

FAILED Duration: 22s

Subject

test: fix parser to prefer binop over unop, as coreutils does. remove bogus workaround in main(). rename atrocious variables/functions. much expand testsuite. libbb: fix --help to not affect "test --help"

Binary

No binary (build failed)

Architecture & Compiler Flags

Architecture: x86_64-linux-gnu Compiler: gcc (Ubuntu 11.4.0-1ubuntu1~22.04.3) 11.4.0

—

Build Log

(log yok)

Commit Diff GitHub →

coreutils/test.c +193 -60 libbb/appletlib.c +13 -4 testsuite/test.tests +50 -7

coreutils/test.c+193 -60

@@ -47,6 +47,8 @@
	operand ::= <any legal UNIX file name>
	*/

+	#define TEST_DEBUG 0
+
	enum token {
	EOI,
	FILRD,
@@ -95,6 +97,79 @@ enum token {
	#define is_file_access(a) (((unsigned char)((a) - FILRD)) <= 2)
	#define is_file_type(a) (((unsigned char)((a) - FILREG)) <= 5)
	#define is_file_bit(a) (((unsigned char)((a) - FILSUID)) <= 2)
+
+	#if TEST_DEBUG
+	int depth;
+	#define nest_msg(...) do { \
+	depth++; \
+	fprintf(stderr, "%s", depth2, ""); \
+	fprintf(stderr, __VA_ARGS__); \
+	} while (0)
+	#define unnest_msg(...) do { \
+	fprintf(stderr, "%s", depth2, ""); \
+	fprintf(stderr, __VA_ARGS__); \
+	depth--; \
+	} while (0)
+	#define dbg_msg(...) do { \
+	fprintf(stderr, "%s", depth2, ""); \
+	fprintf(stderr, __VA_ARGS__); \
+	} while (0)
+	#define unnest_msg_and_return(expr, ...) do { \
+	number_t __res = (expr); \
+	fprintf(stderr, "%s", depth2, ""); \
+	fprintf(stderr, __VA_ARGS__, res); \
+	depth--; \
+	return __res; \
+	} while (0)
+	static const char *const TOKSTR[] = {
+	"EOI",
+	"FILRD",
+	"FILWR",
+	"FILEX",
+	"FILEXIST",
+	"FILREG",
+	"FILDIR",
+	"FILCDEV",
+	"FILBDEV",
+	"FILFIFO",
+	"FILSOCK",
+	"FILSYM",
+	"FILGZ",
+	"FILTT",
+	"FILSUID",
+	"FILSGID",
+	"FILSTCK",
+	"FILNT",
+	"FILOT",
+	"FILEQ",
+	"FILUID",
+	"FILGID",
+	"STREZ",
+	"STRNZ",
+	"STREQ",
+	"STRNE",
+	"STRLT",
+	"STRGT",
+	"INTEQ",
+	"INTNE",
+	"INTGE",
+	"INTGT",
+	"INTLE",
+	"INTLT",
+	"UNOT",
+	"BAND",
+	"BOR",
+	"LPAREN",
+	"RPAREN",
+	"OPERAND"
+	};
+	#else
+	#define nest_msg(...) ((void)0)
+	#define unnest_msg(...) ((void)0)
+	#define dbg_msg(...) ((void)0)
+	#define unnest_msg_and_return(expr, ...) return expr
+	#endif
+
	enum token_types {
	UNOP,
	BINOP,
@@ -103,10 +178,12 @@ enum token_types {
	PAREN
	};

-	static const struct t_op {
+	struct operator_t {
	char op_text[4];
	unsigned char op_num, op_type;
-	} ops[] = {
+	};
+
+	static const struct operator_t ops[] = {
	{ "-r", FILRD , UNOP },
	{ "-w", FILWR , UNOP },
	{ "-x", FILEX , UNOP },
@@ -152,16 +229,18 @@ static const struct t_op {


	#if ENABLE_FEATURE_TEST_64
-	typedef int64_t arith_t;
+	typedef int64_t number_t;
	#else
-	typedef int arith_t;
+	typedef int number_t;
	#endif


	/* We try to minimize both static and stack usage. */
	struct test_statics {
-	char **t_wp;
-	const struct t_op *t_wp_op;
+	char **args;
+	/* set only by check_operator(), either to bogus struct
+	* or points to matching operator_t struct. Never NULL. */
+	const struct operator_t *last_operator;
	gid_t *group_array;
	int ngroups;
	jmp_buf leaving;
@@ -171,8 +250,8 @@ struct test_statics {
	extern struct test_statics *const test_ptr_to_statics;

	#define S (*test_ptr_to_statics)
-	#define t_wp (S.t_wp )
-	#define t_wp_op (S.t_wp_op )
+	#define args (S.args )
+	#define last_operator (S.last_operator)
	#define group_array (S.group_array )
	#define ngroups (S.ngroups )
	#define leaving (S.leaving )
@@ -185,7 +264,7 @@ extern struct test_statics *const test_ptr_to_statics;
	free(test_ptr_to_statics); \
	} while (0)

-	static arith_t primary(enum token n);
+	static number_t primary(enum token n);

	static void syntax(const char op, const char msg) NORETURN;
	static void syntax(const char op, const char msg)
@@ -200,7 +279,7 @@ static void syntax(const char op, const char msg)

	/* atoi with error detection */
	//XXX: FIXME: duplicate of existing libbb function?
-	static arith_t getn(const char *s)
+	static number_t getn(const char *s)
	{
	char *p;
	#if ENABLE_FEATURE_TEST_64
@@ -253,11 +332,15 @@ static int equalf(const char f1, const char f2)
	*/


-	static enum token t_lex(char *s)
+	static enum token check_operator(char *s)
	{
-	const struct t_op *op;
+	static const struct operator_t no_op = {
+	.op_num = -1,
+	.op_type = -1
+	};
+	const struct operator_t *op;

-	t_wp_op = NULL;
+	last_operator = &no_op;
	if (s == NULL) {
	return EOI;
	}
@@ -265,7 +348,7 @@ static enum token t_lex(char *s)
	op = ops;
	do {
	if (strcmp(s, op->op_text) == 0) {
-	t_wp_op = op;
+	last_operator = op;
	return op->op_num;
	}
	op++;
@@ -278,14 +361,14 @@ static enum token t_lex(char *s)
	static int binop(void)
	{
	const char opnd1, opnd2;
-	struct t_op const *op;
-	arith_t val1, val2;
+	const struct operator_t *op;
+	number_t val1, val2;

-	opnd1 = *t_wp;
-	(void) t_lex(*++t_wp);
-	op = t_wp_op;
+	opnd1 = *args;
+	check_operator(*++args);
+	op = last_operator;

-	opnd2 = *++t_wp;
+	opnd2 = *++args;
	if (opnd2 == NULL)
	syntax(op->op_text, "argument expected");

@@ -482,72 +565,117 @@ static int filstat(char *nm, enum token mode)
	}


-	static arith_t nexpr(enum token n)
+	static number_t nexpr(enum token n)
	{
-	if (n == UNOT)
-	return !nexpr(t_lex(*++t_wp));
-	return primary(n);
+	number_t res;
+
+	nest_msg(">nexpr(%s)\n", TOKSTR[n]);
+	if (n == UNOT) {
+	res = !nexpr(check_operator(*++args));
+	unnest_msg("<nexpr:%lld\n", res);
+	return res;
+	}
+	res = primary(n);
+	unnest_msg("<nexpr:%lld\n", res);
+	return res;
	}


-	static arith_t aexpr(enum token n)
+	static number_t aexpr(enum token n)
	{
-	arith_t res;
+	number_t res;

+	nest_msg(">aexpr(%s)\n", TOKSTR[n]);
	res = nexpr(n);
-	if (t_lex(*++t_wp) == BAND)
-	return aexpr(t_lex(*++t_wp)) && res;
-	t_wp--;
+	dbg_msg("aexpr: nexpr:%lld, next args:%s\n", res, args[1]);
+	if (check_operator(*++args) == BAND) {
+	dbg_msg("aexpr: arg is AND, next args:%s\n", args[1]);
+	res = aexpr(check_operator(*++args)) && res;
+	unnest_msg("<aexpr:%lld\n", res);
+	return res;
+	}
+	args--;
+	unnest_msg("<aexpr:%lld, args:%s\n", res, args[0]);
	return res;
	}


-	static arith_t oexpr(enum token n)
+	static number_t oexpr(enum token n)
	{
-	arith_t res;
+	number_t res;

+	nest_msg(">oexpr(%s)\n", TOKSTR[n]);
	res = aexpr(n);
-	if (t_lex(*++t_wp) == BOR) {
-	return oexpr(t_lex(*++t_wp)) \|\| res;
+	dbg_msg("oexpr: aexpr:%lld, next args:%s\n", res, args[1]);
+	if (check_operator(*++args) == BOR) {
+	dbg_msg("oexpr: next arg is OR, next args:%s\n", args[1]);
+	res = oexpr(check_operator(*++args)) \|\| res;
+	unnest_msg("<oexpr:%lld\n", res);
+	return res;
	}
-	t_wp--;
+	args--;
+	unnest_msg("<oexpr:%lld, args:%s\n", res, args[0]);
	return res;
	}


-
-	static arith_t primary(enum token n)
+	static number_t primary(enum token n)
	{
-	arith_t res;
+	#if TEST_DEBUG
+	number_t res = res; /* for compiler */
+	#else
+	number_t res;
+	#endif
+	const struct operator_t *args0_op;

+	nest_msg(">primary(%s)\n", TOKSTR[n]);
	if (n == EOI) {
	syntax(NULL, "argument expected");
	}
	if (n == LPAREN) {
-	res = oexpr(t_lex(*++t_wp));
-	if (t_lex(*++t_wp) != RPAREN)
+	res = oexpr(check_operator(*++args));
+	if (check_operator(*++args) != RPAREN)
	syntax(NULL, "closing paren expected");
+	unnest_msg("<primary:%lld\n", res);
	return res;
	}
-	if (t_wp_op && t_wp_op->op_type == UNOP) {
+
+	/* coreutils 6.9 checks "is args[1] binop and args[2] exist?" first,
+	* do the same */
+	args0_op = last_operator;
+	/* last_operator = operator at args[1] */
+	if (check_operator(args[1]) != EOI) { /* if args[1] != NULL */
+	if (args[2]) {
+	// coreutils also does this:
+	// if (args[3] && args[0]="-l" && args[2] is BINOP)
+	// return binop(1 /* prepended by -l */);
+	if (last_operator->op_type == BINOP)
+	unnest_msg_and_return(binop(), "<primary: binop:%lld\n");
+	}
+	}
+	/* check "is args[0] unop?" second */
+	if (args0_op->op_type == UNOP) {
	/* unary expression */
-	if (*++t_wp == NULL)
-	syntax(t_wp_op->op_text, "argument expected");
+	if (args[1] == NULL)
+	// syntax(args0_op->op_text, "argument expected");
+	goto check_emptiness;
+	args++;
	if (n == STREZ)
-	return t_wp[0][0] == '\0';
+	unnest_msg_and_return(args[0][0] == '\0', "<primary:%lld\n");
	if (n == STRNZ)
-	return t_wp[0][0] != '\0';
+	unnest_msg_and_return(args[0][0] != '\0', "<primary:%lld\n");
	if (n == FILTT)
-	return isatty(getn(*t_wp));
-	return filstat(*t_wp, n);
+	unnest_msg_and_return(isatty(getn(args)), "<primary: isatty(%s)%lld\n", args);
+	unnest_msg_and_return(filstat(args, n), "<primary: filstat(%s):%lld\n", args);
	}

-	t_lex(t_wp[1]);
-	if (t_wp_op && t_wp_op->op_type == BINOP) {
-	return binop();
+	/check_operator(args[1]); - already done /
+	if (last_operator->op_type == BINOP) {
+	/* args[2] is known to be NULL, isn't it bound to fail? */
+	unnest_msg_and_return(binop(), "<primary:%lld\n");
	}
-
-	return t_wp[0][0] != '\0';
+	check_emptiness:
+	unnest_msg_and_return(args[0][0] != '\0', "<primary:%lld\n");
	}


@@ -555,7 +683,7 @@ int test_main(int argc, char **argv)
	{
	int res;
	const char *arg0;
-	bool negate = 0;
+	// bool negate = 0;

	arg0 = bb_basename(argv[0]);
	if (arg0[0] == '[') {
@@ -599,6 +727,8 @@ int test_main(int argc, char **argv)
	res = 1;
	goto ret;
	}
+	#if 0
+	// Now it's fixed in the parser and should not be needed
	if (LONE_CHAR(argv[0], '!') && argv[1]) {
	negate = 1;
	//argc--;
@@ -609,10 +739,10 @@ int test_main(int argc, char **argv)
	goto ret;
	}
	if (argv[2] && !argv[3]) {
-	t_lex(argv[1]);
-	if (t_wp_op && t_wp_op->op_type == BINOP) {
+	check_operator(argv[1]);
+	if (last_operator->op_type == BINOP) {
	/* "test [!] arg1 <binary_op> arg2" */
-	t_wp = &argv[0];
+	args = &argv[0];
	res = (binop() == 0);
	goto ret;
	}
@@ -624,14 +754,17 @@ int test_main(int argc, char **argv)
	//argc++;
	argv--;
	}
-	t_wp = &argv[0];
-	res = !oexpr(t_lex(*t_wp));
+	#endif
+	args = &argv[0];
+	res = !oexpr(check_operator(*args));

-	if (t_wp != NULL && ++t_wp != NULL) {
-	bb_error_msg("%s: unknown operand", *t_wp);
+	if (args != NULL && ++args != NULL) {
+	/* TODO: example when this happens? */
+	bb_error_msg("%s: unknown operand", *args);
	res = 2;
	}
	ret:
	DEINIT_S();
-	return negate ? !res : res;
+	// return negate ? !res : res;
+	return res;
	}

libbb/appletlib.c+13 -4

@@ -196,8 +196,12 @@ void lbb_prepare(const char *applet
	#if ENABLE_FEATURE_INDIVIDUAL
	/* Redundant for busybox (run_applet_and_exit covers that case)
	* but needed for "individual applet" mode */
-	if (argv[1] && strcmp(argv[1], "--help") == 0)
-	bb_show_usage();
+	if (argv[1] && !argv[2] && strcmp(argv[1], "--help") == 0) {
+	/* Special case. POSIX says "test --help"
+	* should be no different from e.g. "test --foo". */
+	if (!ENABLE_TEST \|\| strcmp(applet_name, "test") != 0)
+	bb_show_usage();
+	}
	#endif
	}

@@ -715,8 +719,13 @@ void FAST_FUNC run_applet_no_and_exit(int applet_no, char **argv)
	xfunc_error_retval = EXIT_FAILURE;

	applet_name = APPLET_NAME(applet_no);
-	if (argc == 2 && !strcmp(argv[1], "--help"))
-	bb_show_usage();
+	if (argc == 2 && strcmp(argv[1], "--help") == 0) {
+	/* Special case. POSIX says "test --help"
+	* should be no different from e.g. "test --foo". */
+	//TODO: just compare applet_no with APPLET_NO_test
+	if (!ENABLE_TEST \|\| strcmp(applet_name, "test") != 0)
+	bb_show_usage();
+	}
	if (ENABLE_FEATURE_SUID)
	check_suid(applet_no);
	exit(applet_main[applet_no](argc, argv));

testsuite/test.tests+50 -7

@@ -5,22 +5,65 @@

	. testing.sh

-	# testing "test name" "options" "expected result" "file input" "stdin"
+	# testing "test name" "command" "expected result" "file input" "stdin"
	# file input will be file called "input"
	# test can create a file "actual" instead of writing to stdout

	# Need to call 'busybox test', otherwise shell builtin is used

-	testing "test ! a = b -a ! c = c: should be false" \
+	testing "test: should be false (1)" \
+	"busybox test; echo \$?" \
+	"1\n" \
+	"" ""
+
+	testing "test '': should be false (1)" \
+	"busybox test ''; echo \$?" \
+	"1\n" \
+	"" ""
+
+	testing "test a: should be true (0)" \
+	"busybox test a; echo \$?" \
+	"0\n" \
+	"" ""
+
+	testing "test --help: should be true (0)" \
+	"busybox test --help; echo \$?" \
+	"0\n" \
+	"" ""
+
+	testing "test -f: should be true (0)" \
+	"busybox test -f; echo \$?" \
+	"0\n" \
+	"" ""
+
+	testing "test ! -f: should be false (1)" \
+	"busybox test ! -f; echo \$?" \
+	"1\n" \
+	"" ""
+
+	testing "test a = a: should be true (0)" \
+	"busybox test a = a; echo \$?" \
+	"0\n" \
+	"" ""
+
+	testing "test -lt = -gt: should be false (1)" \
+	"busybox test -lt = -gt; echo \$?" \
+	"1\n" \
+	"" ""
+
+	testing "test -f = a -o b: should be true (0)" \
+	"busybox test -f = a -o b; echo \$?" \
+	"0\n" \
+	"" ""
+
+	testing "test ! a = b -a ! c = c: should be false (1)" \
	"busybox test ! a = b -a ! c = c; echo \$?" \
	"1\n" \
-	"" \
-	"" \
+	"" ""

-	testing "test ! a = b -a ! c = d: should be true" \
+	testing "test ! a = b -a ! c = d: should be true (0)" \
	"busybox test ! a = b -a ! c = d; echo \$?" \
	"0\n" \
-	"" \
-	"" \
+	"" ""

	exit $FAILCOUNT