Hi,
I'd like to have your opinion for this patch before I check it in, since
I'm not sure this is the right way to fix it.
When I debugged the luit/tcsh problem yesterday, I found that the
tcgetpgrp function does not behave as advertised.
Per POSIX, the tcgetpgrp function returns the pgrp ID only if the file
descriptor references the controlling tty of the process. If the
process has no ctty, or if the descriptor references another tty not
being the controlling tty, the function is supposed to set errno to
ENOTTY and return -1.
Cygwin OTOH, always returns the pgid of the tty, even if it's not the
controlling tty of the process. This leads potentially to weird
results, as you can see in the output of the testcase below.
And then there's Linux, with a tiny special case.
If a Linux process opens the master side of a pty and then calls
tcgetpgrp on this file, the tcgetpgrp will return 0. However, as soon
as a child process has called setsid() and opened the slave side of the
pty and made that tty its controlling tty, tcgetprgrp(master) returns
the own pid, not 0. Don't look into the Linux man page, this behaviour
is not documented.
I checked that on Solaris as well, and Solaris behaves POSIX-compliant.
Here the master side returns -1/ENOTTY, unless you open the master side
without the O_NOCTTY flag and the opening process has no controlling tty
at that time.
My patch makes Cygwin behave like Linux. It's just an additional test
in tcgetpgrp. I'm not sure if that's TRTTD for two reasons.
- Since Linux behaves obviously not POSIX-compliant with respect to the
master pty, I'm not sure we should really reflect Linux literally.
Maybe it's better to behave POSIX-compliant and return -1/ENOTTY for a
non-controlling master all the time, just as on Solaris?
- My first approach changed the actual values in tc->pgid, rather than
just making a test for the ctty in tcgetprgrp. I'm still wondering if
the code should better try to maintain the correct values in the
datastructures, or if it's better to just have this check in tcgetpgrp.
Ok, I used the following test application to verify the behaviour on
Linux, Solaris, and Cygwin:
==== SNIP ====
#define _XOPEN_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <errno.h>
#include <sys/fcntl.h>
#include <sys/wait.h>
#include <unistd.h>
#include <termios.h>
#include <sys/ioctl.h>
int main ()
{
int master, slave, status;
char pts[256];
printf ("parent pid: %d\n", getpid ());
if ((master = open ("/dev/ptmx", O_RDWR | O_NOCTTY)) >= 0)
{
int ret;
grantpt (master);
unlockpt (master);
printf ("parent tcgetpgrp master: %d\n", tcgetpgrp (master));
strcpy (pts, ptsname (master));
switch (fork ())
{
case -1:
break;
case 0: // child
ret = setsid ();
printf ("child pid: %d (setsid: %d)\n", getpid (), ret);
printf ("child tcgetpgrp master before open: %d\n", tcgetpgrp
(master));
if ((slave = open (pts, O_RDWR)) >= 0)
{
printf ("child tcgetpgrp master after open: %d\n", tcgetpgrp
(master));
printf ("child tcgetpgrp slave: %d\n", tcgetpgrp (slave));
close (slave);
}
break;
default:// parent
wait (&status);
printf ("parent tcgetpgrp master: %d\n", tcgetpgrp (master));
break;
}
close (master);
return 0;
}
return 1;
}
==== SNAP ====
Output on Linux:
parent pid: 14670
parent tcgetpgrp master: 0
child pid: 14671 (setsid: 14671)
child tcgetpgrp master before open: 0
child tcgetpgrp master after open: 14671
child tcgetpgrp slave: 14671
parent tcgetpgrp master: 0
Output on Solaris 10:
parent pid: 20372
parent tcgetpgrp master: -1
child pid: 20374 (setsid: 20374)
child tcgetpgrp master before open: -1
child tcgetpgrp master after open: -1
child tcgetpgrp slave: 20374
parent tcgetpgrp master: -1
Output on Cygwin without my patch:
parent pid: 3704
parent tcgetpgrp master: 3704
child pid: 1056 (setsid: 1056)
child tcgetpgrp master before open: 3704
child tcgetpgrp master after open: 1056
child tcgetpgrp slave: 1056
parent tcgetpgrp master: 1056
Output on Cygwin with my patch:
parent pid: 4016
parent tcgetpgrp master: 0
child pid: 872 (setsid: 872)
child tcgetpgrp master before open: 0
child tcgetpgrp master after open: 872
child tcgetpgrp slave: 872
parent tcgetpgrp master: 0
So my patch results in a behaviour just like on Linux for now.
Patch attached.
Corinna
* fhandler.h (fhandler_pty_master::tcgetpgrp): Declare.
* fhandler_termios.cc (fhandler_termios::tcgetpgrp): Only return
valid pgid if tty is controlling tty. Set errno to ENOTTY and
return -1 otherwise.
(fhandler_pty_master::tcgetpgrp): New function. Return 0 for
master side of pty if it's not the controlling tty of the process.
Index: fhandler.h
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler.h,v
retrieving revision 1.381
diff -u -p -r1.381 fhandler.h
--- fhandler.h 28 Sep 2009 12:10:32 -0000 1.381
+++ fhandler.h 6 Oct 2009 08:58:34 -0000
@@ -1109,6 +1109,7 @@ public:
int dup (fhandler_base *);
void fixup_after_fork (HANDLE parent);
void fixup_after_exec ();
+ int tcgetpgrp ();
};
class fhandler_tty_master: public fhandler_pty_master
Index: fhandler_termios.cc
===================================================================
RCS file: /cvs/src/src/winsup/cygwin/fhandler_termios.cc,v
retrieving revision 1.74
diff -u -p -r1.74 fhandler_termios.cc
--- fhandler_termios.cc 3 Jan 2009 05:12:20 -0000 1.74
+++ fhandler_termios.cc 6 Oct 2009 08:58:34 -0000
@@ -99,7 +99,16 @@ fhandler_termios::tcsetpgrp (const pid_t
int
fhandler_termios::tcgetpgrp ()
{
- return tc->pgid;
+ if (myself->ctty != -1 && myself->ctty == tc->ntty)
+ return tc->pgid;
+ set_errno (ENOTTY);
+ return -1;
+}
+
+int
+fhandler_pty_master::tcgetpgrp ()
+{
+ return myself->ctty != -1 && myself->ctty == tc->ntty ? tc->pgid : 0;
}
void
--
Corinna Vinschen Please, send mails regarding Cygwin to
Cygwin Project Co-Leader cygwin AT cygwin DOT com
Red Hat
