Several months bump.
Lucas <lu...@sexy.is> wrote:
> 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
