Re: EMACS_GET_TTY_PGRP vs. emacs_get_tty_pgrp

[Ken Brown <kbrown@cornell.edu>]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

On 10/20/2010 11:14 AM, Andreas Schwab wrote:
> Ken Brown<kbrown@cornell.edu>  writes:
>
>> The macro EMACS_GET_TTY_PGRP defined in src/systty.h uses either
>> tcgetpgrp or TIOCGPGRP to get the PGID of a terminal's foreground
>> process group.  But src/process.c defines and uses its own
>> emacs_get_tty_pgrp that only works on systems that have TIOCGPGRP.  Is
>> there a good reason for this?
>
> Is "historical accident" a good reason? :-)

Sure.

It turned out that emacs_get_tty_pgrp has different arguments than its 
uppercase cousin, so I didn't get rid of it.  But I used the macro 
inside the function instead and got rid of all ifdefs involving 
TIOCGPGRP.  Does the attached patch look OK?

Ken

[-- Attachment #2: tty.patch --]
[-- Type: text/plain, Size: 1782 bytes --]

=== modified file 'src/process.c'
--- src/process.c	2010-09-30 04:28:34 +0000
+++ src/process.c	2010-10-20 17:32:57 +0000
@@ -6009,23 +6009,21 @@
 {
   int gid = -1;
 
-#ifdef TIOCGPGRP
-  if (ioctl (p->infd, TIOCGPGRP, &gid) == -1 && ! NILP (p->tty_name))
+  if (EMACS_GET_TTY_PGRP (p->infd, &gid) <= 0 && ! NILP (p->tty_name))
     {
       int fd;
-      /* Some OS:es (Solaris 8/9) does not allow TIOCGPGRP from the
-	 master side.  Try the slave side.  */
+      /* Some OS:es (Solaris 8/9) do not allow TIOCGPGRP/tcgetpgrp
+	 from the master side.  Try the slave side.  */
       fd = emacs_open (SDATA (p->tty_name), O_RDONLY, 0);
 
       if (fd != -1)
 	{
-	  ioctl (fd, TIOCGPGRP, &gid);
+	  EMACS_GET_TTY_PGRP (fd, &gid);
 	  emacs_close (fd);
 	}
     }
-#endif /* defined (TIOCGPGRP ) */
-
-  return gid;
+  /* On some systems EMACS_GET_TTY_PGRP can return 0 if it fails. */
+  return (gid > 0 ? gid : -1);
 }
 
 DEFUN ("process-running-child-p", Fprocess_running_child_p,
@@ -6206,7 +6204,7 @@
 	 handle the signal.  */
 #endif /* defined (SIGNALS_VIA_CHARACTERS) */
 
-#ifdef TIOCGPGRP
+#ifdef EMACS_HAVE_TTY_PGRP
       /* Get the current pgrp using the tty itself, if we have that.
 	 Otherwise, use the pty to get the pgrp.
 	 On pfa systems, saka@pfu.fujitsu.co.JP writes:
@@ -6227,11 +6225,11 @@
 	 Or perhaps this is vestigial.  */
       if (gid == -1)
 	no_pgrp = 1;
-#else  /* ! defined (TIOCGPGRP ) */
+#else  /* ! defined (EMACS_HAVE_TTY_PGRP) */
       /* Can't select pgrps on this system, so we know that
 	 the child itself heads the pgrp.  */
       gid = p->pid;
-#endif /* ! defined (TIOCGPGRP ) */
+#endif /* ! defined (EMACS_HAVE_TTY_PGRP) */
 
       /* If current_group is lambda, and the shell owns the terminal,
 	 don't send any signal.  */