From jilles@stack.nl Sat Apr 4 13:50:46 2009 Return-Path: Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3AE3C106566B for ; Sat, 4 Apr 2009 13:50:46 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (unknown [IPv6:2001:610:1108:5000::149]) by mx1.freebsd.org (Postfix) with ESMTP id DDB7C8FC18 for ; Sat, 4 Apr 2009 13:50:45 +0000 (UTC) (envelope-from jilles@stack.nl) Received: by mx1.stack.nl (Postfix, from userid 65534) id 3F5673F7BA; Sat, 4 Apr 2009 15:50:45 +0200 (CEST) Received: from snail.stack.nl (snail.stack.nl [IPv6:2001:610:1108:5010::131]) by mx1.stack.nl (Postfix) with ESMTP id D906A3F5E6 for ; Sat, 4 Apr 2009 15:50:35 +0200 (CEST) Received: by snail.stack.nl (Postfix, from userid 1677) id 3C6E5228A2; Sat, 4 Apr 2009 15:50:23 +0200 (CEST) Message-Id: <20090404135023.3C6E5228A2@snail.stack.nl> Date: Sat, 4 Apr 2009 15:50:23 +0200 (CEST) From: Jilles Tjoelker Reply-To: Jilles Tjoelker To: FreeBSD-gnats-submit@freebsd.org Cc: Subject: test(1) with 3 or 4 arguments X-Send-Pr-Version: 3.113 X-GNATS-Notify: >Number: 133369 >Category: standards >Synopsis: [patch] test(1) with 3 or 4 arguments >Confidential: no >Severity: serious >Priority: medium >Responsible: freebsd-standards >State: closed >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Sat Apr 04 14:00:03 UTC 2009 >Closed-Date: Fri May 29 15:51:45 UTC 2009 >Last-Modified: Sat Apr 10 01:38:39 UTC 2010 >Originator: Jilles Tjoelker >Release: FreeBSD 7.2-PRERELEASE i386 >Organization: MCGV Stack >Environment: >Description: test(1) does not follow the rules in POSIX exactly if there are 3 or 4 arguments, different from what the man page says. In particular, different from what POSIX prescribes, test "$a" = "$b" does not work properly for all values of $a and $b. /bin/test and the test builtin in /bin/sh use the same code. >How-To-Repeat: An example, more tests are included in the patch. Input: /bin/test \( = \); echo $? Expected result: 1 Actual result: 0 According to POSIX, the rule about the second argument being a binary operator has priority above the rule about the first argument being '(' and the third argument being ')'. >Fix: Apply this patch. It includes additional test cases in TEST.sh. --- test-posixfixes.patch begins here --- --- src/bin/test/test.c.orig 2005-01-10 09:39:26.000000000 +0100 +++ src/bin/test/test.c 2009-04-02 02:28:28.000000000 +0200 @@ -163,6 +163,7 @@ struct t_op const *t_wp_op; int nargc; char **t_wp; +int parenlevel; static int aexpr(enum token); static int binop(void); @@ -171,7 +172,9 @@ static int getn(const char *); static intmax_t getq(const char *); static int intcmp(const char *, const char *); -static int isoperand(void); +static int isunopoperand(void); +static int islparenoperand(void); +static int isrparenoperand(void); static int newerf(const char *, const char *); static int nexpr(enum token); static int oexpr(enum token); @@ -205,7 +208,14 @@ #endif nargc = argc; t_wp = &argv[1]; - res = !oexpr(t_lex(*t_wp)); + parenlevel = 0; + if (nargc == 4 && strcmp(*t_wp, "!") == 0) { + /* Things like ! "" -o x do not fit in the normal grammar. */ + --nargc; + ++t_wp; + res = oexpr(t_lex(*t_wp)); + } else + res = !oexpr(t_lex(*t_wp)); if (--nargc > 0) syntax(*t_wp, "unexpected operator"); @@ -268,12 +278,16 @@ if (n == EOI) return 0; /* missing expression */ if (n == LPAREN) { + parenlevel++; if ((nn = t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) == - RPAREN) + RPAREN) { + parenlevel--; return 0; /* missing expression */ + } res = oexpr(nn); if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) != RPAREN) syntax(NULL, "closing paren expected"); + parenlevel--; return res; } if (t_wp_op && t_wp_op->op_type == UNOP) { @@ -410,8 +424,10 @@ } while (op->op_text) { if (strcmp(s, op->op_text) == 0) { - if ((op->op_type == UNOP && isoperand()) || - (op->op_num == LPAREN && nargc == 1)) + if (((op->op_type == UNOP || op->op_type == BUNOP) + && isunopoperand()) || + (op->op_num == LPAREN && islparenoperand()) || + (op->op_num == RPAREN && isrparenoperand())) break; t_wp_op = op; return op->op_num; @@ -423,7 +439,7 @@ } static int -isoperand(void) +isunopoperand(void) { struct t_op const *op = ops; char *s; @@ -431,19 +447,53 @@ if (nargc == 1) return 1; - if (nargc == 2) - return 0; s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; t = *(t_wp + 2); while (op->op_text) { if (strcmp(s, op->op_text) == 0) return op->op_type == BINOP && - (t[0] != ')' || t[1] != '\0'); + (parenlevel == 0 || t[0] != ')' || t[1] != '\0'); + op++; + } + return 0; +} + +static int +islparenoperand(void) +{ + struct t_op const *op = ops; + char *s; + + if (nargc == 1) + return 1; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + if (nargc != 3) + return 0; + while (op->op_text) { + if (strcmp(s, op->op_text) == 0) + return op->op_type == BINOP; op++; } return 0; } +static int +isrparenoperand(void) +{ + char *s; + + if (nargc == 1) + return 0; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + return 0; +} + /* atoi with error detection */ static int getn(const char *s) --- src/bin/test/TEST.sh.orig 2005-01-10 09:39:26.000000000 +0100 +++ src/bin/test/TEST.sh 2009-04-03 22:53:28.000000000 +0200 @@ -133,5 +133,35 @@ t 1 '""' t 0 '! ""' +t 0 '!' +t 0 '\(' +t 0 '\)' + +t 1 '\( = \)' +t 0 '\( != \)' +t 0 '\( ! \)' +t 0 '\( \( \)' +t 0 '\( \) \)' +t 0 '! = !' +t 1 '! != !' +t 1 '-n = \)' +t 0 '! != \)' +t 1 '! = a' +t 0 '! != -n' +t 0 '! -c /etc/passwd' + +t 0 '! \( = \)' +t 1 '! \( != \)' +t 1 '! = = =' +t 0 '! = = \)' +t 0 '! "" -o ""' +t 1 '! "x" -o ""' +t 1 '! "" -o "x"' +t 1 '! "x" -o "x"' +t 0 '\( -f /etc/passwd \)' +t 1 '\( ! = \)' +t 0 '\( ! "" \)' +t 1 '\( ! -e \)' + echo "" echo "Syntax errors: $ERROR Failed: $FAILED" --- test-posixfixes.patch ends here --- >Release-Note: >Audit-Trail: From: dfilter@FreeBSD.org (dfilter service) To: bug-followup@FreeBSD.org Cc: Subject: Re: standards/133369: commit references a PR Date: Tue, 26 May 2009 22:33:20 +0000 (UTC) Author: jilles Date: Tue May 26 22:33:10 2009 New Revision: 192862 URL: http://svn.freebsd.org/changeset/base/192862 Log: Fix various cases with 3 or 4 parameters in test(1) to be POSIX compliant. More precisely, this gives precedence to an interpretation not using the '(', ')', '-a' and '-o' in their special meaning, if possible. For example, it is now safe to write [ "$a" = "$b" ] and assume it compares the two strings. The man page already says that test(1) works this way, so does not need to be changed. Interpretation of input with more parameters tries a bit harder to find a valid parse in some cases. Add various additional test cases to TEST.sh. PR: standards/133369 Approved by: ed (mentor) Modified: head/bin/test/TEST.sh head/bin/test/test.c Modified: head/bin/test/TEST.sh ============================================================================== --- head/bin/test/TEST.sh Tue May 26 22:21:53 2009 (r192861) +++ head/bin/test/TEST.sh Tue May 26 22:33:10 2009 (r192862) @@ -133,5 +133,45 @@ t 0 '"a" -a ! ""' t 1 '""' t 0 '! ""' +t 0 '!' +t 0 '\(' +t 0 '\)' + +t 1 '\( = \)' +t 0 '\( != \)' +t 0 '\( ! \)' +t 0 '\( \( \)' +t 0 '\( \) \)' +t 0 '! = !' +t 1 '! != !' +t 1 '-n = \)' +t 0 '! != \)' +t 1 '! = a' +t 0 '! != -n' +t 0 '! -c /etc/passwd' + +t 0 '! \( = \)' +t 1 '! \( != \)' +t 1 '! = = =' +t 0 '! = = \)' +t 0 '! "" -o ""' +t 1 '! "x" -o ""' +t 1 '! "" -o "x"' +t 1 '! "x" -o "x"' +t 0 '\( -f /etc/passwd \)' +t 1 '\( ! = \)' +t 0 '\( ! "" \)' +t 1 '\( ! -e \)' + +t 0 '0 -eq 0 -a -d /' +t 0 '-s = "" -o "" = ""' +t 0 '"" = "" -o -s = ""' +t 1 '-s = "" -o -s = ""' +t 0 '-z x -o x = "#" -o x = x' +t 1 '-z y -o y = "#" -o y = x' +t 0 '0 -ne 0 -o ! -f /' +t 0 '1 -ne 0 -o ! -f /etc/passwd' +t 1 '0 -ne 0 -o ! -f /etc/passwd' + echo "" echo "Syntax errors: $ERROR Failed: $FAILED" Modified: head/bin/test/test.c ============================================================================== --- head/bin/test/test.c Tue May 26 22:21:53 2009 (r192861) +++ head/bin/test/test.c Tue May 26 22:33:10 2009 (r192862) @@ -163,6 +163,7 @@ struct t_op { struct t_op const *t_wp_op; int nargc; char **t_wp; +int parenlevel; static int aexpr(enum token); static int binop(void); @@ -171,7 +172,9 @@ static int filstat(char *, enum token); static int getn(const char *); static intmax_t getq(const char *); static int intcmp(const char *, const char *); -static int isoperand(void); +static int isunopoperand(void); +static int islparenoperand(void); +static int isrparenoperand(void); static int newerf(const char *, const char *); static int nexpr(enum token); static int oexpr(enum token); @@ -205,7 +208,14 @@ main(int argc, char **argv) #endif nargc = argc; t_wp = &argv[1]; - res = !oexpr(t_lex(*t_wp)); + parenlevel = 0; + if (nargc == 4 && strcmp(*t_wp, "!") == 0) { + /* Things like ! "" -o x do not fit in the normal grammar. */ + --nargc; + ++t_wp; + res = oexpr(t_lex(*t_wp)); + } else + res = !oexpr(t_lex(*t_wp)); if (--nargc > 0) syntax(*t_wp, "unexpected operator"); @@ -268,12 +278,16 @@ primary(enum token n) if (n == EOI) return 0; /* missing expression */ if (n == LPAREN) { + parenlevel++; if ((nn = t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) == - RPAREN) + RPAREN) { + parenlevel--; return 0; /* missing expression */ + } res = oexpr(nn); if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) != RPAREN) syntax(NULL, "closing paren expected"); + parenlevel--; return res; } if (t_wp_op && t_wp_op->op_type == UNOP) { @@ -410,8 +424,10 @@ t_lex(char *s) } while (op->op_text) { if (strcmp(s, op->op_text) == 0) { - if ((op->op_type == UNOP && isoperand()) || - (op->op_num == LPAREN && nargc == 1)) + if (((op->op_type == UNOP || op->op_type == BUNOP) + && isunopoperand()) || + (op->op_num == LPAREN && islparenoperand()) || + (op->op_num == RPAREN && isrparenoperand())) break; t_wp_op = op; return op->op_num; @@ -423,7 +439,7 @@ t_lex(char *s) } static int -isoperand(void) +isunopoperand(void) { struct t_op const *op = ops; char *s; @@ -431,19 +447,53 @@ isoperand(void) if (nargc == 1) return 1; - if (nargc == 2) - return 0; s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; t = *(t_wp + 2); while (op->op_text) { if (strcmp(s, op->op_text) == 0) return op->op_type == BINOP && - (t[0] != ')' || t[1] != '\0'); + (parenlevel == 0 || t[0] != ')' || t[1] != '\0'); + op++; + } + return 0; +} + +static int +islparenoperand(void) +{ + struct t_op const *op = ops; + char *s; + + if (nargc == 1) + return 1; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + if (nargc != 3) + return 0; + while (op->op_text) { + if (strcmp(s, op->op_text) == 0) + return op->op_type == BINOP; op++; } return 0; } +static int +isrparenoperand(void) +{ + char *s; + + if (nargc == 1) + return 0; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + return 0; +} + /* atoi with error detection */ static int getn(const char *s) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" State-Changed-From-To: open->closed State-Changed-By: jilles State-Changed-When: Fri May 29 15:51:44 UTC 2009 State-Changed-Why: Committed, with a few additional test cases. http://www.freebsd.org/cgi/query-pr.cgi?pr=133369 From dfilter@FreeBSD.ORG Fri Apr 9 22:48:36 2010 Date: Fri, 9 Apr 2010 22:48:36 +0000 (UTC) From: dfilter@FreeBSD.ORG (dfilter service) To: bug-followup@FreeBSD.org Subject: Re: standard/133369: commit references a PR Author: emaste Date: Fri Apr 9 22:48:27 2010 New Revision: 206432 URL: http://svn.freebsd.org/changeset/base/206432 Log: MFC r192862: > Description of fields to fill in above: 76 columns --| Fix various cases with 3 or 4 parameters in test(1) to be POSIX compliant. More precisely, this gives precedence to an interpretation not using the '(', ')', '-a' and '-o' in their special meaning, if possible. For example, it is now safe to write [ "$a" = "$b" ] and assume it compares the two strings. The man page already says that test(1) works this way, so does not need to be changed. Interpretation of input with more parameters tries a bit harder to find a valid parse in some cases. Add various additional test cases to TEST.sh. PR: standards/133369 Discussed with: jilles Modified: stable/7/bin/test/TEST.sh stable/7/bin/test/test.c Directory Properties: stable/7/bin/test/ (props changed) Modified: stable/7/bin/test/TEST.sh ============================================================================== --- stable/7/bin/test/TEST.sh Fri Apr 9 21:18:46 2010 (r206431) +++ stable/7/bin/test/TEST.sh Fri Apr 9 22:48:27 2010 (r206432) @@ -133,5 +133,45 @@ t 0 '"a" -a ! ""' t 1 '""' t 0 '! ""' +t 0 '!' +t 0 '\(' +t 0 '\)' + +t 1 '\( = \)' +t 0 '\( != \)' +t 0 '\( ! \)' +t 0 '\( \( \)' +t 0 '\( \) \)' +t 0 '! = !' +t 1 '! != !' +t 1 '-n = \)' +t 0 '! != \)' +t 1 '! = a' +t 0 '! != -n' +t 0 '! -c /etc/passwd' + +t 0 '! \( = \)' +t 1 '! \( != \)' +t 1 '! = = =' +t 0 '! = = \)' +t 0 '! "" -o ""' +t 1 '! "x" -o ""' +t 1 '! "" -o "x"' +t 1 '! "x" -o "x"' +t 0 '\( -f /etc/passwd \)' +t 1 '\( ! = \)' +t 0 '\( ! "" \)' +t 1 '\( ! -e \)' + +t 0 '0 -eq 0 -a -d /' +t 0 '-s = "" -o "" = ""' +t 0 '"" = "" -o -s = ""' +t 1 '-s = "" -o -s = ""' +t 0 '-z x -o x = "#" -o x = x' +t 1 '-z y -o y = "#" -o y = x' +t 0 '0 -ne 0 -o ! -f /' +t 0 '1 -ne 0 -o ! -f /etc/passwd' +t 1 '0 -ne 0 -o ! -f /etc/passwd' + echo "" echo "Syntax errors: $ERROR Failed: $FAILED" Modified: stable/7/bin/test/test.c ============================================================================== --- stable/7/bin/test/test.c Fri Apr 9 21:18:46 2010 (r206431) +++ stable/7/bin/test/test.c Fri Apr 9 22:48:27 2010 (r206432) @@ -163,6 +163,7 @@ struct t_op { struct t_op const *t_wp_op; int nargc; char **t_wp; +int parenlevel; static int aexpr(enum token); static int binop(void); @@ -171,7 +172,9 @@ static int filstat(char *, enum token); static int getn(const char *); static intmax_t getq(const char *); static int intcmp(const char *, const char *); -static int isoperand(void); +static int isunopoperand(void); +static int islparenoperand(void); +static int isrparenoperand(void); static int newerf(const char *, const char *); static int nexpr(enum token); static int oexpr(enum token); @@ -205,7 +208,14 @@ main(int argc, char **argv) #endif nargc = argc; t_wp = &argv[1]; - res = !oexpr(t_lex(*t_wp)); + parenlevel = 0; + if (nargc == 4 && strcmp(*t_wp, "!") == 0) { + /* Things like ! "" -o x do not fit in the normal grammar. */ + --nargc; + ++t_wp; + res = oexpr(t_lex(*t_wp)); + } else + res = !oexpr(t_lex(*t_wp)); if (--nargc > 0) syntax(*t_wp, "unexpected operator"); @@ -268,12 +278,16 @@ primary(enum token n) if (n == EOI) return 0; /* missing expression */ if (n == LPAREN) { + parenlevel++; if ((nn = t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL)) == - RPAREN) + RPAREN) { + parenlevel--; return 0; /* missing expression */ + } res = oexpr(nn); if (t_lex(nargc > 0 ? (--nargc, *++t_wp) : NULL) != RPAREN) syntax(NULL, "closing paren expected"); + parenlevel--; return res; } if (t_wp_op && t_wp_op->op_type == UNOP) { @@ -410,8 +424,10 @@ t_lex(char *s) } while (op->op_text) { if (strcmp(s, op->op_text) == 0) { - if ((op->op_type == UNOP && isoperand()) || - (op->op_num == LPAREN && nargc == 1)) + if (((op->op_type == UNOP || op->op_type == BUNOP) + && isunopoperand()) || + (op->op_num == LPAREN && islparenoperand()) || + (op->op_num == RPAREN && isrparenoperand())) break; t_wp_op = op; return op->op_num; @@ -423,7 +439,7 @@ t_lex(char *s) } static int -isoperand(void) +isunopoperand(void) { struct t_op const *op = ops; char *s; @@ -431,19 +447,53 @@ isoperand(void) if (nargc == 1) return 1; - if (nargc == 2) - return 0; s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; t = *(t_wp + 2); while (op->op_text) { if (strcmp(s, op->op_text) == 0) return op->op_type == BINOP && - (t[0] != ')' || t[1] != '\0'); + (parenlevel == 0 || t[0] != ')' || t[1] != '\0'); + op++; + } + return 0; +} + +static int +islparenoperand(void) +{ + struct t_op const *op = ops; + char *s; + + if (nargc == 1) + return 1; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + if (nargc != 3) + return 0; + while (op->op_text) { + if (strcmp(s, op->op_text) == 0) + return op->op_type == BINOP; op++; } return 0; } +static int +isrparenoperand(void) +{ + char *s; + + if (nargc == 1) + return 0; + s = *(t_wp + 1); + if (nargc == 2) + return parenlevel == 1 && strcmp(s, ")") == 0; + return 0; +} + /* atoi with error detection */ static int getn(const char *s) _______________________________________________ svn-src-all@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscribe@freebsd.org" >Unformatted: