Ping.

> Hi tech@,
> 
> Both test.1 and ksh.1 (under the non-POSIX compatibility flag) state
> that `test -t` will default to test whether fd 1 is a TTY if the
> argument is omitted. This isn't the case, and both treat `-t` as the
> equivalent of `test -n -t`, ie, test if `-t` is a non-empty string,
> trivially true.
> 
> It's easy to see it in `test`: it exits right away in switch(argc).
> `ksh` is a bit more difficult to follow: builtins eventually call c_test
> in c_test.c. For `test -t`, c_test will be called with wp being "test",
> "-t". It'll eventually call test_parse, with the test environment set
> with
> 
>       te.pos.wp = wp + 1;
>       te.wp_end = wp + argc;
> 
> For this particular case, argc is 2. Bearing that in mind, test_parse
> will make a call stack of test_aexpr -> test_oexpr -> test_nexpr ->
> test_primary. The first 2 ifs are skipped (asuming no TEF_ERROR is set)
> and the next if, which would handle `test -t` gracefully, isn't entered:
> one of the && operands, &te->pos.wp[1] < te->wp_end, is false.
> Afterwards, `-t` is evaled as TO_SNTZE, ie if it's a non-empty string.
> 
> The following diff amends the manpages, removing the "file descriptor
> is optional" parts, and removes the unused machinery of bin/ksh/c_test.c
> to deal with an optional argument in `test -t`, together with a small
> rewrite in test_eval TO_FILTT case, as it was a bit too difficult for me
> to read: it seems that, for legacy reason (haven't looked at the code
> history), opnd1 could be NULL for TO_FILTT, the only test with such
> behaviour. My understanding is that ptest_getopnd avoids that by
> returning "1". So, opnd1 can't be NULL. bi_getn writes to res, but took
> me a while to understand that looking only at the conditional call of
> bi_getn in the if condition. Finally, for some reason, is opnd1 managed
> to be NULL, isatty(0) is called. I believe that the intention was to do
> 
>       res = opnd1 ? isatty(res) : 0;
> 
> instead. I think the proposed rewrite is easier to follow.
> 
> Regress is happy, although nothing in there tests `test -t` or `test -t
> n`. Existing scripts shouldn't break with this change: `test -t` will
> keep being true, whether fd 1 is a TTY or not. The diff can be further
> split on man changes and code changes if needed.
> 
> btw, the easy test for `test -t` being wrong, whether under POSIX compat
> or not, is
> 
>       $ test -t 1; echo $? # 1 is TTY in an interactive session
>       0
>       $ test -t 1 >&-; echo $? # 1 isn't a TTY because it's closed
>       1
>       $ test -t >&-; echo $?
>       0


diff refs/heads/master refs/heads/ksh-t-fd
commit - 55055d619d36cc45f8c6891404c51cd405214e86
commit + 00fa44f8caccc78154e093b5fa719e392716b81e
blob - 7038a52bfa432d515d9683187930407c4d6bd6d5
blob + db16b71a9b64afb7047803c65ec620f03e18e42d
--- bin/ksh/c_test.c
+++ bin/ksh/c_test.c
@@ -156,12 +156,6 @@ c_test(char **wp)
                        }
                        if (argc == 1) {
                                opnd1 = (*te.getopnd)(&te, TO_NONOP, 1);
-                               /* Historically, -t by itself test if fd 1
-                                * is a file descriptor, but POSIX says its
-                                * a string test...
-                                */
-                               if (!Flag(FPOSIX) && strcmp(opnd1, "-t") == 0)
-                                   break;
                                res = (*te.eval)(&te, TO_STNZE, opnd1,
                                    NULL, 1);
                                if (invert & 1)
@@ -271,14 +265,18 @@ test_eval(Test_env *te, Test_op op, const char *opnd1,
        case TO_FILGZ: /* -s */
                return stat(opnd1, &b1) == 0 && b1.st_size > 0L;
        case TO_FILTT: /* -t */
-               if (opnd1 && !bi_getn(opnd1, &res)) {
-                       te->flags |= TEF_ERROR;
-                       res = 0;
-               } else {
-                       /* generate error if in FPOSIX mode? */
-                       res = isatty(opnd1 ? res : 0);
+               {
+                       int s, v;
+
+                       s = bi_getn(opnd1, &v);
+                       if (!s) {
+                               te->flags |= TEF_ERROR;
+                               res = 0;
+                       } else {
+                               res = isatty(v);
+                       }
+                       return res;
                }
-               return res;
        case TO_FILUID: /* -O */
                return stat(opnd1, &b1) == 0 && b1.st_uid == ksheuid;
        case TO_FILGID: /* -G */
@@ -527,7 +525,7 @@ ptest_getopnd(Test_env *te, Test_op op, int do_eval)
 ptest_getopnd(Test_env *te, Test_op op, int do_eval)
 {
        if (te->pos.wp >= te->wp_end)
-               return op == TO_FILTT ? "1" : NULL;
+               return NULL;
        return *te->pos.wp++;
 }
 
blob - cd3bfc3ebf41217b46c69eaff634cd8e9771c425
blob + c8406305e706cff12711bbcb934f13d67f49ebdf
--- bin/ksh/ksh.1
+++ bin/ksh/ksh.1
@@ -2569,20 +2569,6 @@ a i in 1 2; do echo i=$i j=$j; done
 alias a='for ' i='j'
 a i in 1 2; do echo i=$i j=$j; done
 .Ed
-.It
-.Ic test .
-In POSIX mode, the expression
-.Sq Fl t
-(preceded by some number of
-.Sq \&!
-arguments) is always true as it is a non-zero length string;
-in non-POSIX mode, it tests if file descriptor 1 is a
-.Xr tty 4
-(i.e. the
-.Ar fd
-argument to the
-.Fl t
-test may be left out and defaults to 1).
 .El
 .Ss Strict Bourne shell mode
 When the
@@ -3857,18 +3843,12 @@ is not empty.
 .It Fl s Ar file
 .Ar file
 is not empty.
-.It Fl t Op Ar fd
+.It Fl t Ar fd
 File descriptor
 .Ar fd
 is a
 .Xr tty 4
 device.
-If the
-.Ic posix
-option is not set,
-.Ar fd
-may be left out, in which case it is taken to be 1 (the behaviour differs due
-to the special POSIX rules described above).
 .It Fl u Ar file
 .Ar file Ns 's
 mode has the setuid bit set.
blob - 6af8fcb71493c1802546069662c36ae7e97b458f
blob + 46c42936556ee9d6c4ab08b7dfc2bd80af9a0e29
--- bin/test/test.1
+++ bin/test/test.1
@@ -153,7 +153,7 @@ is
 True if the file whose file descriptor number
 is
 .Ar file_descriptor
-(default 1) is open and is associated with a terminal.
+is open and is associated with a terminal.
 .It Fl u Ar file
 True if
 .Ar file

Reply via email to