bitwiseworks / libc

LIBC Next (kLIBC fork)
9 stars 4 forks source link

isatty on /dev/null #105

Closed SilvanScherrer closed 3 years ago

SilvanScherrer commented 3 years ago

libc thinks that /dev/null is a valid tty. Means isatty() on /dev/null is true. But this is not what a nix port expects, as on nix isatty() on /dev/null is always false. we should fix libc in this regard.

I found this while working on pcre and pcre2.

dmik commented 3 years ago

Here is a simple test case btw:

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>

int main()
{
  fprintf (stderr, "isatty(stdout) %d\n", isatty(fileno(stdout)));

  int fd = open ("/dev/null", O_RDWR);
  if (fd < 0)
    perror ("open");
  else
    fprintf (stderr, "isatty(/dev/null) %d\n", isatty(fd));

  return 0;
}

And it's clear where it comes from. On Win32 _isatty behaves exactly the same (and this behavior is documented, see here) because NUL is a character device there as well as on OS/2 and all character devices are expected to return non-zero from _isatty. EMX docs say exactly the same:

Returns a non-zero value if handle refers to a character device. Returns 0 if handle does not refer to a character file (file or pipe). If there is an error, errno is set and -1 is returned.

I fear I can't break this contract. However, there is _isterm (which is an EMX extension) and for which docs explicitly say that:

Returns a non-zero value if handle refers to the standard input (keyboard) or standard output (screen) device. Otherwise, returns 0. If there is an error, errno is set and 0 is returned.

But currently it behaves exactly like isatty (it actually calls it and then checks for F_DEV in flags after isatty already checked for the same F_DEV).

I guess I can fix _isterm to return 0 for /dev/null (and for NUL, obviously) by checking the device type with DosQueryHType directly.

Once this is done, apps that need a specific check for keyboard/screen, will have to call _isterm on OS/2.

dmik commented 3 years ago

In the above comment I did it better by calling __ioctl2 which also knows how to handle non-standard handles.

And with this extended test case variant:

#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <io.h>

int main()
{
  fprintf (stderr, "isatty(stdin) %d\n", isatty(fileno(stdin)));
  fprintf (stderr, "_isterm(stdin) %d\n", _isterm(fileno(stdin)));

  fprintf (stderr, "isatty(stdout) %d\n", isatty(fileno(stdout)));
  fprintf (stderr, "_isterm(stdout) %d\n", _isterm(fileno(stdout)));

  int fd = open ("/dev/null", O_RDWR);
  if (fd < 0)
    perror ("open");
  else
  {
    fprintf (stderr, "isatty(/dev/null) %d\n", isatty(fd));
    fprintf (stderr, "_isterm(/dev/null) %d\n", _isterm(fd));
  }

  return 0;
}

I get this output which looks perfectly valid to me now:

D:>libc-dev test.exe
isatty(stdin) 1
_isterm(stdin) 1
isatty(stdout) 1
_isterm(stdout) 1
isatty(/dev/null) 1
_isterm(/dev/null) 0

D:>libc-dev test.exe >nul
isatty(stdin) 1
_isterm(stdin) 1
isatty(stdout) 1
_isterm(stdout) 0
isatty(/dev/null) 1
_isterm(/dev/null) 0

D:>libc-dev test.exe >test.txt
isatty(stdin) 1
_isterm(stdin) 1
isatty(stdout) 0
_isterm(stdout) 0
isatty(/dev/null) 1
_isterm(/dev/null) 0

D:>libc-dev test.exe >test.txt <nul
isatty(stdin) 1
_isterm(stdin) 0
isatty(stdout) 0
_isterm(stdout) 0
isatty(/dev/null) 1
_isterm(/dev/null) 0

D:>libc-dev test.exe >test.txt <test.txt
isatty(stdin) 0
_isterm(stdin) 0
isatty(stdout) 0
_isterm(stdout) 0
isatty(/dev/null) 1
_isterm(/dev/null) 0

Code that needs strict Unix-like iastty behavior may easily achieve this by simply doing

#define isatty _isterm

somewhere after the headers.

I'm waiting for Silvan to provide an exact case where he needed this (somewhere in configure, he says) before finally closing this ticket as I first want to analyze some real-world case.

dmik commented 3 years ago

Ok, the real world example is in this commit: https://github.com/bitwiseworks/pcre2-os2/commit/670e98b7308c7f9c46e50e22b360a5993a0d0884. Well, obviously, it is easily fixable by changing

#define INTERACTIVE(f) isatty(fileno(f))

to

#define INTERACTIVE(f) _isterm(fileno(f))

on OS/2 while keeping full EMX compatibility.

So I guess we leave things as it is now. Closing this.