Testing new getpt(3) and posix_openpt(3) implementation

[Guillem Jover]

Hi!

I just got annoyed enough with the kFreeBSD message on the console:

  "pid %d (%s) is using legacy pty devices%s\n"

that I started digging around. This is due to applications using the
old BSD-style pseudo-terminal master device.

I've switched the eglibc getpt(3) and posix_openpt(3) to use the new
posix_openpt(2) syscall introduced in kFreeBSD 8.0. I've done this
instead of using the compatibility ‘/dev/ptmx’ device, because it
should allow us to unload the compatibility pty(4) kernel module.

I've not build or run tested the code, so if someone with an eglibc
build-tree around could test the attached patch (against the
glibc-ports tree) that would be pretty helpful, otherwise I might try
to, once I've got some spare time.

thanks,
guillem
Index: kfreebsd/getpt.c
===
--- kfreebsd/getpt.c	(revision 3428)
+++ kfreebsd/getpt.c	(working copy)
@@ -21,63 +21,27 @@
 #include 
 #include 
 #include 
+#include 
 
+/* The system call does not change the controlling terminal, so we have
+ * to do it ourselves.  */
+extern int __syscall_posix_openpt (int oflag);
+libc_hidden_proto (__syscall_posix_openpt)
 
-/* Prefix for master pseudo terminal nodes.  */
-#define _PATH_PTY "/dev/pty"
+/* Prototype for function that opens BSD-style master pseudo-terminals.  */
+int __bsd_getpt (void);
 
-
-/* Letters indicating a series of pseudo terminals.  */
-#ifndef PTYNAME1
-#define PTYNAME1 "pqrs"
-#endif
-const char __libc_ptyname1[] attribute_hidden = PTYNAME1;
-
-/* Letters indicating the position within a series.  */
-#ifndef PTYNAME2
-#define PTYNAME2 "0123456789abcdefghijklmnopqrstuv";
-#endif
-const char __libc_ptyname2[] attribute_hidden = PTYNAME2;
-
-
 /* Open a master pseudo terminal and return its file descriptor.  */
 int
 __posix_openpt (int oflag)
 {
-  char buf[sizeof (_PATH_PTY) + 2];
-  const char *p, *q;
-  char *s;
-
-  s = __mempcpy (buf, _PATH_PTY, sizeof (_PATH_PTY) - 1);
-  /* s[0] and s[1] will be filled in the loop.  */
-  s[2] = '\0';
-
-  for (p = __libc_ptyname1; *p != '\0'; ++p)
+  int fd = INLINE_SYSCALL (posix_openpt, 1, oflag);
+  if (fd >= 0)
 {
-  s[0] = *p;
-
-  for (q = __libc_ptyname2; *q != '\0'; ++q)
-	{
-	  int fd;
-
-	  s[1] = *q;
-
-	  fd = __open (buf, oflag);
-	  if (fd >= 0)
-	{
-	  if (!(oflag & O_NOCTTY))
-		__ioctl (fd, TIOCSCTTY, NULL);
-
-	  return fd;
-	}
-
-	  if (errno == ENOENT)
-	return -1;
-	}
+  if (!(oflag & O_NOCTTY))
+__ioctl (fd, TIOCSCTTY, NULL);
 }
-
-  __set_errno (ENOENT);
-  return -1;
+  return fd;
 }
 
 weak_alias (__posix_openpt, posix_openpt)
@@ -86,7 +50,25 @@
 int
 __getpt (void)
 {
-  return __posix_openpt (O_RDWR);
+  int fd = __posix_openpt (O_RDWR);
+  if (fd == -1)
+{
+  fd = __bsd_getpt ();
+  if (fd >= 0)
+{
+  if (!(oflag & O_NOCTTY))
+__ioctl (fd, TIOCSCTTY, NULL);
+}
+}
+  return fd;
 }
 
-weak_alias (__getpt, getpt)
+
+/* Letters indicating a series of pseudo terminals.  */
+#define PTYNAME1 "pqrs";
+/* Letters indicating the position within a series.  */
+#define PTYNAME2 "0123456789abcdefghijklmnopqrstuv";
+
+#define __getpt __bsd_getpt
+#define HAVE_POSIX_OPENPT
+#include 
Index: kfreebsd/syscalls.list
===
--- kfreebsd/syscalls.list	(revision 3428)
+++ kfreebsd/syscalls.list	(working copy)
@@ -98,6 +98,7 @@
 ntp_adjtime		-	ntp_adjtime		i:p		ntp_adjtime
 obreak			-	obreak			i:a		__syscall_obreak
 sys_open		-	open			i:siv		__syscall_open
+posix_openpt		getpt	posix_openpt		i:i		__syscall_posix_openpt
 poll			-	poll			Ci:pii		__poll poll
 sys_pread		-	pread			i:ibni		__syscall_pread
 sys_freebsd6_pread	-	freebsd6_pread		i:ibnii		__syscall_freebsd6_pread
Index: kfreebsd/Versions
===
--- kfreebsd/Versions	(revision 3428)
+++ kfreebsd/Versions	(working copy)
@@ -112,6 +112,7 @@
 # misc fixes for FreeBSD:
 __syscall_freebsd6_lseek; __syscall_freebsd6_pread; __syscall_freebsd6_pwrite;
 __syscall_lseek; __syscall_pread; __syscall_pwrite;
+__syscall_posix_openpt;
 __syscall_connect; __syscall_sendto;
 __syscall_cpuset_getaffinity ; __syscall_cpuset_setaffinity;
  # global variable used in brk()

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Petr Salinger]

Hi!


I've not build


true ;-)


or run tested the code, so if someone with an eglibc
build-tree around could test the attached patch (against the
glibc-ports tree) that would be pretty helpful


With slight modification in progress.

Petr


--
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/alpine.lrh.2.02.1106091030180.22...@sci.felk.cvut.cz

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Petr Salinger]

or run tested the code, so if someone with an eglibc
build-tree around could test the attached patch (against the
glibc-ports tree) that would be pretty helpful


With slight modification in progress.


There are failures in testsuite, 
at least ptsname.c have to be updated too.


Other related functions are:
grantpt(), ptsname(), unlockpt()


Petr


--
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/alpine.lrh.2.02.1106091054590.22...@sci.felk.cvut.cz

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Guillem Jover]

On Thu, 2011-06-09 at 11:05:28 +0200, Petr Salinger wrote:
> >>or run tested the code, so if someone with an eglibc
> >>build-tree around could test the attached patch (against the
> >>glibc-ports tree) that would be pretty helpful
> >
> >With slight modification in progress.

Thanks for the building and testing!

> There are failures in testsuite, at least ptsname.c have to be
> updated too.

Ah right, had a small change for that one, but as there seemed to be
more changed needed there I didn't include it, was not suspecting it
would cause test suite failures. :) Attached now.

Although the line with:

  p[0] = 't';

seems a bit suspicious, but then I've not analyzed the function in
depth.

thanks,
guillem
Index: kfreebsd/ptsname.c
===
--- kfreebsd/ptsname.c	(revision 3434)
+++ kfreebsd/ptsname.c	(working copy)
@@ -26,8 +26,11 @@
 #include 
 
 
+/* Directory where we can find the slave pty nodes.  */
+#define _PATH_DEVPTS "/dev/pts/"
+
 /* Static buffer for `ptsname'.  */
-static char buffer[sizeof (_PATH_TTY) + 2];
+static char buffer[sizeof (_PATH_DEVPTS) + 20];
 
 
 /* Return the pathname of the pseudo terminal slave associated with

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Petr Salinger]

There are failures in testsuite, at least ptsname.c have to be
updated too.


Ah right, had a small change for that one, but as there seemed to be
more changed needed there I didn't include it, was not suspecting it
would cause test suite failures. :) Attached now.

Although the line with:

 p[0] = 't';

seems a bit suspicious, but then I've not analyzed the function in
depth.


The code gets via kern.devname name of master device "/dev/ptyXX"
and alters into name of slave device "/dev/ttyXX".

I am not sure what we should do with /dev/pts/xxx names.
The 'p' -> 't' alteration might not be the right one.

Petr


--
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/alpine.lrh.2.02.1106091140480.22...@sci.felk.cvut.cz

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Guillem Jover]

On Thu, 2011-06-09 at 11:05:28 +0200, Petr Salinger wrote:
> There are failures in testsuite, at least ptsname.c have to be
> updated too.
> 
> Other related functions are:
> grantpt(), ptsname(), unlockpt()

Ok, I guess the obvious answer to this is:

  


thanks,
guillem


-- 
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: http://lists.debian.org/20110609100315.ga18...@gaara.hadrons.org

Re: Testing new getpt(3) and posix_openpt(3) implementation

[Petr Salinger]

There are failures in testsuite, at least ptsname.c have to be
updated too.

Other related functions are:
grantpt(), ptsname(), unlockpt()


The set of changes is in our glibc-bsd SVN.
It passes the testsuite, seems to work inside chroot.

I have not committed it into pkg-glibc SVN due to tie with multiarch.

Petr


--
To UNSUBSCRIBE, email to debian-bsd-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Archive: 
http://lists.debian.org/alpine.lrh.2.02.1106091907040.23...@sci.felk.cvut.cz