sorry for the delay, this is another mail that I meant to take a look
earlier...
On 2023/05/29 16:36:45 +0000, Lucas <lu...@sexy.is> wrote:
> 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).
agreed. The "(default 1)" in test.1 is completely wrong.
> > `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.
Agree on this too.
> > 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`,
for what is worth I agree with you. It's a historic behaviour that
has been broken for some time already and noone seems to have noticed,
I think it could go away.
> > 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.
I think that the rewrite is more complex than needed. I'm attaching a
slightly revised diff that is closer to the current code.
> > 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
bash and ash behave the same out-of-the-box (i.e. without any eventual
posix flag set.)
I'd prefer some more feedback on this before going on.
diff /usr/src
commit - 15eb8637ab039139400e655284e2e2d8ca898a03
path + /usr/src
blob - 7038a52bfa432d515d9683187930407c4d6bd6d5
file + bin/ksh/c_test.c
--- 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,11 @@ 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)) {
+ if (!bi_getn(opnd1, &res)) {
te->flags |= TEF_ERROR;
- res = 0;
- } else {
- /* generate error if in FPOSIX mode? */
- res = isatty(opnd1 ? res : 0);
+ return 0
}
- return res;
+ return isatty(res);
case TO_FILUID: /* -O */
return stat(opnd1, &b1) == 0 && b1.st_uid == ksheuid;
case TO_FILGID: /* -G */
@@ -527,7 +518,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
file + bin/ksh/ksh.1
--- 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
file + bin/test/test.1
--- 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
