[PATCH] Fix ttyname and ttyname_r

[PATCH] Fix ttyname and ttyname_r

[Jakub Jelinek]

Hi!

ttyname{,_r} cleanup from April this year moved isatty check before
readlink which detected EBADF.  As isatty (-1) == 0 (-1 stands for any
invalid fd), this results in ttyname{,_r} returning ENOTTY (errno resp.
return value) instead of EBADF.
I don't think we can argument that invalid file descriptor does not
refer to a terminal either and therefore it can return ENOTTY.
As isatty function isn't documented to set any particular errno,
I chose to inline isatty, i.e. call tcgetattr which is documented
to set EBADF resp. ENOTTY, exactly what we need for ttyname{,_r}.

2006-12-04  Jakub Jelinek  <jakub@redhat.com>

	* sysdeps/unix/sysv/linux/ttyname.c: Include termios.h.
	(ttyname): Use tcgetattr instead of isatty, don't set errno to ENOTTY.
	* sysdeps/unix/sysv/linux/ttyname_r.c: Include termios.h.
	(__ttyname_r): Use tcgetattr instead of isatty, don't set errno to
	ENOTTY.
	* io/Makefile: Add rules to build and run tst-ttyname_r test.
	* io/tst-ttyname_r.c: New test.

--- libc/sysdeps/unix/sysv/linux/ttyname.c.jj	2006-04-19 09:26:48.000000000 +0200
+++ libc/sysdeps/unix/sysv/linux/ttyname.c	2006-12-04 13:43:24.000000000 +0100
@@ -22,6 +22,7 @@
 #include <dirent.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <termios.h>
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
@@ -118,12 +119,12 @@ ttyname (int fd)
   int dostat = 0;
   char *name;
   int save = errno;
+  struct termios term;
 
-  if (__builtin_expect (!__isatty (fd), 0))
-    {
-      __set_errno (ENOTTY);
-      return NULL;
-    }
+  /* isatty check, tcgetattr is used because it sets the correct
+     errno (EBADF resp. ENOTTY) on error.  */
+  if (__builtin_expect (__tcgetattr (fd, &term) < 0, 0))
+    return NULL;
 
   /* We try using the /proc filesystem.  */
   *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0';
--- libc/sysdeps/unix/sysv/linux/ttyname_r.c.jj	2006-04-19 09:26:48.000000000 +0200
+++ libc/sysdeps/unix/sysv/linux/ttyname_r.c	2006-12-04 13:43:35.000000000 +0100
@@ -22,6 +22,7 @@
 #include <dirent.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <termios.h>
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
@@ -115,11 +116,11 @@ __ttyname_r (int fd, char *buf, size_t b
       return ERANGE;
     }
 
-  if (__builtin_expect (!__isatty (fd), 0))
-    {
-      __set_errno (ENOTTY);
-      return ENOTTY;
-    }
+  /* isatty check, tcgetattr is used because it sets the correct
+     errno (EBADF resp. ENOTTY) on error.  */
+  struct termios term;
+  if (__builtin_expect (__tcgetattr (fd, &term) < 0, 0))
+    return errno;
 
   /* We try using the /proc filesystem.  */
   *_fitoa_word (fd, __stpcpy (procname, "/proc/self/fd/"), 10, 0) = '\0';
--- libc/io/Makefile.jj	2006-10-31 23:05:31.000000000 +0100
+++ libc/io/Makefile	2006-12-04 13:41:42.000000000 +0100
@@ -66,7 +66,7 @@ tests		:= test-utime test-stat test-stat
 		   tst-openat tst-unlinkat tst-fstatat tst-futimesat \
 		   tst-renameat tst-fchownat tst-fchmodat tst-faccessat \
 		   tst-symlinkat tst-linkat tst-readlinkat tst-mkdirat \
-		   tst-mknodat tst-mkfifoat
+		   tst-mknodat tst-mkfifoat tst-ttyname_r
 
 distribute	:= ftwtest-sh
 
--- libc/io/tst-ttyname_r.c.jj	2006-12-04 13:35:15.000000000 +0100
+++ libc/io/tst-ttyname_r.c	2006-12-04 13:41:02.000000000 +0100
@@ -0,0 +1,42 @@
+#include <errno.h>
+#include <error.h>
+#include <fcntl.h>
+#include <stdio.h>
+#include <unistd.h>
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+#include <test-skeleton.c>
+
+static int temp_fd;
+
+static void
+do_prepare (void)
+{
+  char *temp_file;
+  temp_fd = create_temp_file ("tst-ttyname_r.", &temp_file);
+  if (temp_fd == -1)
+    error (1, errno, "cannot create temporary file");
+}
+
+static int
+do_test (void)
+{
+  int ret = 0;
+  char buf[sysconf (_SC_TTY_NAME_MAX) + 1];
+  int res = ttyname_r (-1, buf, sizeof (buf));
+  if (res != EBADF)
+    {
+      printf ("1st ttyname_r returned with res %d\n", res);
+      ret++;
+    }
+  res = ttyname_r (temp_fd, buf, sizeof (buf));
+  if (res != ENOTTY)
+    {
+      printf ("2nd ttyname_r returned with res %d\n", res);
+      ret++;
+    }
+  return ret;
+}

	Jakub

Re: [PATCH] Fix ttyname and ttyname_r

[Ulrich Drepper]

Applied.

-- 
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖