public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
* [PATCH] Cygwin: ctty: Replace ctty constant with more descriptive macros.
@ 2023-03-07  2:33 Takashi Yano
       [not found] ` <DM8PR09MB7095179671A47284BE556204A5B79@DM8PR09MB7095.namprd09.prod.outlook.com>
  0 siblings, 1 reply; 2+ messages in thread
From: Takashi Yano @ 2023-03-07  2:33 UTC (permalink / raw)
  To: cygwin-patches; +Cc: Takashi Yano, Corinna Vinschen

This patch replaces ctty constants with more descriptive macros
(CTTY_UNINITIALIZED and CTTY_RELEASED) rather than -1 and -2 as
well as checking sign with CTTY_IS_VALID().

Fixes: 3b7df69aaa57 (Cygwin: ctty: Add comments for the special values: -1 and -2.)
Suggested-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 winsup/cygwin/dtable.cc                 |  8 ++++----
 winsup/cygwin/exceptions.cc             |  2 +-
 winsup/cygwin/external.cc               |  4 ++--
 winsup/cygwin/fhandler/console.cc       | 10 ++++++----
 winsup/cygwin/fhandler/process.cc       |  4 ++--
 winsup/cygwin/fhandler/pty.cc           | 12 ++++++------
 winsup/cygwin/fhandler/termios.cc       |  7 ++++---
 winsup/cygwin/local_includes/fhandler.h | 11 +++++++++++
 winsup/cygwin/local_includes/tty.h      |  2 +-
 winsup/cygwin/pinfo.cc                  | 17 +++++++----------
 winsup/cygwin/spawn.cc                  |  2 +-
 winsup/cygwin/syscalls.cc               |  6 ++----
 winsup/cygwin/tty.cc                    |  2 +-
 13 files changed, 48 insertions(+), 39 deletions(-)

diff --git a/winsup/cygwin/dtable.cc b/winsup/cygwin/dtable.cc
index 0ed3ea85d..7a8c12aa5 100644
--- a/winsup/cygwin/dtable.cc
+++ b/winsup/cygwin/dtable.cc
@@ -323,7 +323,7 @@ dtable::init_std_file_from_handle (int fd, HANDLE handle)
 	   || GetNumberOfConsoleInputEvents (handle, (DWORD *) &buf))
     {
       /* Console I/O */
-      if (myself->ctty > 0)
+      if (CTTY_IS_VALID (myself->ctty))
 	dev.parse (myself->ctty);
       else
 	{
@@ -603,10 +603,10 @@ fh_alloc (path_conv& pc)
 	      fhraw = cnew_no_ctor (fhandler_console, -1);
 	      debug_printf ("not called from open for /dev/tty");
 	    }
-	  else if (myself->ctty <= 0 && last_tty_dev
+	  else if (!CTTY_IS_VALID (myself->ctty) && last_tty_dev
 		   && !myself->set_ctty (fh_last_tty_dev, 0))
 	    debug_printf ("no /dev/tty assigned");
-	  else if (myself->ctty > 0)
+	  else if (CTTY_IS_VALID (myself->ctty))
 	    {
 	      debug_printf ("determining /dev/tty assignment for ctty %p", myself->ctty);
 	      if (iscons_dev (myself->ctty))
@@ -682,7 +682,7 @@ build_fh_pc (path_conv& pc)
 
   /* Keep track of the last tty-like thing opened.  We could potentially want
      to open it if /dev/tty is referenced. */
-  if (myself->ctty > 0 || !fh->is_tty () || !pc.isctty_capable ())
+  if (CTTY_IS_VALID (myself->ctty) || !fh->is_tty () || !pc.isctty_capable ())
     last_tty_dev = FH_NADA;
   else
     last_tty_dev = fh->dev ();
diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index c3433ab94..642afb788 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1049,7 +1049,7 @@ ctrl_c_handler (DWORD type)
       return FALSE;
     }
 
-  if (myself->ctty != -1)
+  if (myself->ctty != CTTY_UNINITIALIZED)
     {
       if (type == CTRL_CLOSE_EVENT)
 	{
diff --git a/winsup/cygwin/external.cc b/winsup/cygwin/external.cc
index 582bab84f..97e452874 100644
--- a/winsup/cygwin/external.cc
+++ b/winsup/cygwin/external.cc
@@ -73,12 +73,12 @@ fillout_pinfo (pid_t pid, int winpid)
 	  ep.pid = thispid + MAX_PID;
 	  ep.dwProcessId = thispid;
 	  ep.process_state = PID_IN_USE;
-	  ep.ctty = -1;
+	  ep.ctty = CTTY_UNINITIALIZED;
 	  break;
 	}
       else if (nextpid || p->pid == pid)
 	{
-	  ep.ctty = (p->ctty < 0 || iscons_dev (p->ctty))
+	  ep.ctty = (!CTTY_IS_VALID (p->ctty) || iscons_dev (p->ctty))
 		    ? p->ctty : device::minor (p->ctty);
 	  ep.pid = p->pid;
 	  ep.ppid = p->ppid;
diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 0cbfe4ea4..c67385e99 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -665,12 +665,13 @@ fhandler_console::set_unit ()
     this_unit == FH_CONSOLE || this_unit == FH_CONIN || this_unit == FH_CONOUT;
   if (!generic_console && this_unit != FH_TTY)
     unit = get_minor ();
-  else if (myself->ctty != -1)
+  else if (myself->ctty != CTTY_UNINITIALIZED)
     unit = device::minor (myself->ctty);
 
   if (shared_console_info[unit])
     ; /* Do nothing */
-  else if (generic_console && myself->ctty != -1 && !iscons_dev (myself->ctty))
+  else if (generic_console
+	   && myself->ctty != CTTY_UNINITIALIZED && !iscons_dev (myself->ctty))
     devset = FH_ERROR;
   else
     {
@@ -1731,7 +1732,7 @@ int
 fhandler_console::dup (fhandler_base *child, int flags)
 {
   /* See comments in fhandler_pty_slave::dup */
-  if (myself->ctty != -2)
+  if (myself->ctty != CTTY_RELEASED)
     myself->set_ctty (this, flags);
   return 0;
 }
@@ -1932,7 +1933,8 @@ fhandler_console::close ()
   memset (&con_ra, 0, sizeof (con_ra));
 
   if (!have_execed && !invisible_console
-      && (myself->ctty <= 0 || get_device () == (dev_t) myself->ctty))
+      && (!CTTY_IS_VALID (myself->ctty)
+	  || get_device () == (dev_t) myself->ctty))
     free_console ();
 
   if (shared_console_info[unit])
diff --git a/winsup/cygwin/fhandler/process.cc b/winsup/cygwin/fhandler/process.cc
index 864e2f4d5..1e5e83b4a 100644
--- a/winsup/cygwin/fhandler/process.cc
+++ b/winsup/cygwin/fhandler/process.cc
@@ -468,7 +468,7 @@ static off_t
 format_process_ctty (void *data, char *&destbuf)
 {
   _pinfo *p = (_pinfo *) data;
-  if (p->ctty < 0)
+  if (!CTTY_IS_VALID (p->ctty))
     {
       destbuf = (char *) crealloc_abort (destbuf, 2);
       return __small_sprintf (destbuf, "\n");
@@ -1098,7 +1098,7 @@ format_process_stat (void *data, char *&destbuf)
 /* ctty maj is 31:16, min is 15:0; tty_nr s/b maj 15:8, min 31:20, 7:0;
    maj is 31:16 >> 16 & fff << 8; min is 15:0 >> 8 & ff << 20 | & ff */
   int tty_nr = 0;
-  if (p->ctty > 0)
+  if (CTTY_IS_VALID (p->ctty))
     tty_nr =   (((p->ctty >>  8) & 0xff)  << 20)
 	     | (((p->ctty >> 16) & 0xfff) <<  8)
 	     |   (p->ctty        & 0xff);
diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
index 0dac80a16..664d7dbc6 100644
--- a/winsup/cygwin/fhandler/pty.cc
+++ b/winsup/cygwin/fhandler/pty.cc
@@ -1609,12 +1609,12 @@ fhandler_pty_slave::dup (fhandler_base *child, int flags)
   /* This code was added in Oct 2001 for some undisclosed reason.
      However, setting the controlling tty on a dup causes rxvt to
      hang when the parent does a dup since the controlling pgid changes.
-     Specifically testing for -2 (ctty has been setsid'ed) works around
-     this problem.  However, it's difficult to see scenarios in which you
-     have a dup'able fd, no controlling tty, and not having run setsid.
-     So, we might want to consider getting rid of the set_ctty in tty-like dup
-     methods entirely at some point */
-  if (myself->ctty != -2)
+     Specifically testing for CTTY_RELEASED (ctty has been setsid'ed)
+     works around this problem.  However, it's difficult to see scenarios
+     in which you have a dup'able fd, no controlling tty, and not having
+     run setsid.  So, we might want to consider getting rid of the
+     set_ctty in tty-like dup methods entirely at some point */
+  if (myself->ctty != CTTY_RELEASED)
     myself->set_ctty (this, flags);
   report_tty_counts (child, "duped slave", "");
   return 0;
diff --git a/winsup/cygwin/fhandler/termios.cc b/winsup/cygwin/fhandler/termios.cc
index 5b92cdd31..1a5dfdd1b 100644
--- a/winsup/cygwin/fhandler/termios.cc
+++ b/winsup/cygwin/fhandler/termios.cc
@@ -111,7 +111,7 @@ fhandler_termios::tcsetpgrp (const pid_t pgid)
 int
 fhandler_termios::tcgetpgrp ()
 {
-  if (myself->ctty > 0 && myself->ctty == tc ()->ntty)
+  if (CTTY_IS_VALID (myself->ctty) && myself->ctty == tc ()->ntty)
     return tc ()->pgid;
   set_errno (ENOTTY);
   return -1;
@@ -685,7 +685,7 @@ fhandler_termios::sigflush ()
 pid_t
 fhandler_termios::tcgetsid ()
 {
-  if (myself->ctty > 0 && myself->ctty == tc ()->ntty)
+  if (CTTY_IS_VALID (myself->ctty) && myself->ctty == tc ()->ntty)
     return tc ()->getsid ();
   set_errno (ENOTTY);
   return -1;
@@ -730,7 +730,8 @@ fhandler_termios::ioctl (int cmd, void *varg)
 
   termios_printf ("myself->ctty %d, myself->sid %d, myself->pid %d, arg %d, tc()->getsid () %d\n",
 		  myself->ctty, myself->sid, myself->pid, arg, tc ()->getsid ());
-  if (myself->ctty > 0 || myself->sid != myself->pid || (!arg && tc ()->getsid () > 0))
+  if (CTTY_IS_VALID (myself->ctty) || myself->sid != myself->pid
+      || (!arg && tc ()->getsid () > 0))
     {
       set_errno (EPERM);
       return -1;
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index 085a2a10d..f82f565cf 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -1905,6 +1905,17 @@ class fhandler_serial: public fhandler_base
 #define release_output_mutex() \
   __release_output_mutex (__PRETTY_FUNCTION__, __LINE__)
 
+/*
+ -1: CTTY is not initialized yet. Can associate with the TTY
+     which is associated with the own session.
+ -2: CTTY has been released by setsid(). Can associate with
+     a new TTY as CTTY, but cannot associate with the TTYs
+     already associated with other sessions.
+*/
+#define CTTY_UNINITIALIZED -1
+#define CTTY_RELEASED -2
+#define CTTY_IS_VALID(c) ((c) > 0)
+
 extern DWORD mutex_timeout;
 DWORD acquire_attach_mutex (DWORD t);
 void release_attach_mutex (void);
diff --git a/winsup/cygwin/local_includes/tty.h b/winsup/cygwin/local_includes/tty.h
index cd1d6ecaa..df3bf60bf 100644
--- a/winsup/cygwin/local_includes/tty.h
+++ b/winsup/cygwin/local_includes/tty.h
@@ -13,7 +13,7 @@ details. */
 #define INP_BUFFER_SIZE 256
 #define OUT_BUFFER_SIZE 256
 #define NTTYS		128
-#define real_tty_attached(p)	((p)->ctty > 0 && !iscons_dev ((p)->ctty))
+#define real_tty_attached(p)	(CTTY_IS_VALID ((p)->ctty) && !iscons_dev ((p)->ctty))
 
 /* Input/Output/ioctl events */
 
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 37770b643..bfd338e5b 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -97,7 +97,7 @@ pinfo_init (char **envp, int envc)
 
       myself.thisproc (NULL);
       myself->pgid = myself->sid = myself->pid;
-      myself->ctty = -1; /* -1 means not initialized yet. */
+      myself->ctty = CTTY_UNINITIALIZED;
       myself->uid = ILLEGAL_UID;
       myself->gid = ILLEGAL_GID;
       environ_init (NULL, 0);	/* call after myself has been set up */
@@ -212,7 +212,7 @@ pinfo::exit (DWORD n)
     maybe_set_exit_code_from_windows ();	/* may block */
   exit_state = ES_FINAL;
 
-  if (myself->ctty > 0 && !iscons_dev (myself->ctty))
+  if (CTTY_IS_VALID (myself->ctty) && !iscons_dev (myself->ctty))
     {
       lock_ttys here;
       tty *t = cygwin_shared->tty[device::minor(myself->ctty)];
@@ -514,7 +514,7 @@ pinfo::pinfo (HANDLE parent, pinfo_minimal& from, pid_t pid):
 const char *
 _pinfo::_ctty (char *buf)
 {
-  if (ctty <= 0)
+  if (!CTTY_IS_VALID (ctty))
     strcpy (buf, "no ctty");
   else
     {
@@ -530,12 +530,9 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
 {
   tty_min& tc = *fh->tc ();
   debug_printf ("old %s, ctty device number %y, tc.ntty device number %y flags & O_NOCTTY %y", __ctty (), ctty, tc.ntty, flags & O_NOCTTY);
-  if (fh && (ctty <= 0 || ctty == tc.ntty) && !(flags & O_NOCTTY))
+  if (fh && (!CTTY_IS_VALID (ctty) || ctty == tc.ntty) && !(flags & O_NOCTTY))
     {
-      if (tc.getsid () && tc.getsid () != sid && ctty == -2)
-	/* ctty == -2 means CTTY has been released by setsid().
-	   Can be associated only with a new TTY which is not
-	   associated with any other session. */
+      if (tc.getsid () && tc.getsid () != sid && ctty == CTTY_RELEASED)
 	; /* Do nothing if another session is associated with the TTY. */
       else
 	{
@@ -581,14 +578,14 @@ _pinfo::set_ctty (fhandler_termios *fh, int flags)
 	 an obvious bug surfaces. */
       if (sid == pid && !tc.getsid ())
 	tc.setsid (sid);
-      if (ctty > 0)
+      if (CTTY_IS_VALID (ctty))
 	sid = tc.getsid ();
       /* See above */
       if ((!tc.getpgid () || being_debugged ()) && pgid == pid)
 	tc.setpgid (pgid);
     }
   debug_printf ("cygheap->ctty now %p, archetype %p", cygheap->ctty, fh ? fh->archetype : NULL);
-  return ctty > 0;
+  return CTTY_IS_VALID (ctty);
 }
 
 /* Test to determine if a process really exists and is processing signals.
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 32ba5b377..4d0ca127b 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -623,7 +623,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       si.cb = sizeof (si);
 
       if (!iscygwin ())
-	init_console_handler (myself->ctty > 0);
+	init_console_handler (CTTY_IS_VALID (myself->ctty));
 
     loop:
       /* When ruid != euid we create the new process under the current original
diff --git a/winsup/cygwin/syscalls.cc b/winsup/cygwin/syscalls.cc
index c529192b4..8ae0397fb 100644
--- a/winsup/cygwin/syscalls.cc
+++ b/winsup/cygwin/syscalls.cc
@@ -1176,9 +1176,7 @@ setsid (void)
     syscall_printf ("hmm.  pgid %d pid %d", myself->pgid, myself->pid);
   else
     {
-      myself->ctty = -2; /* -2 means CTTY has been released by setsid().
-			    Can be associated only with a new TTY which
-			    is not associated with any session. */
+      myself->ctty = CTTY_RELEASED;
       myself->sid = myself->pid;
       myself->pgid = myself->pid;
       if (cygheap->ctty)
@@ -2832,7 +2830,7 @@ ctermid (char *str)
 {
   if (str == NULL)
     str = _my_tls.locals.ttybuf;
-  if (myself->ctty < 0)
+  if (!CTTY_IS_VALID (myself->ctty))
     strcpy (str, "no tty");
   else
     {
diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc
index 6ec8927cb..bf7c6010f 100644
--- a/winsup/cygwin/tty.cc
+++ b/winsup/cygwin/tty.cc
@@ -58,7 +58,7 @@ revoke (char *ttyname)
 extern "C" int
 ttyslot (void)
 {
-  if (myself->ctty <= 0 || iscons_dev (myself->ctty))
+  if (!CTTY_IS_VALID (myself->ctty) || iscons_dev (myself->ctty))
     return -1;
   return device::minor (myself->ctty);
 }
-- 
2.39.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [EXTERNAL] [PATCH] Cygwin: ctty: Replace ctty constant with more descriptive macros.
       [not found] ` <DM8PR09MB7095179671A47284BE556204A5B79@DM8PR09MB7095.namprd09.prod.outlook.com>
@ 2023-03-07  3:30   ` Takashi Yano
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Yano @ 2023-03-07  3:30 UTC (permalink / raw)
  To: Lavrentiev, Anton (NIH/NLM/NCBI) [C]; +Cc: cygwin-patches

On Tue, 7 Mar 2023 02:42:49 +0000
"Lavrentiev, Anton (NIH/NLM/NCBI) [C]" wrote:

> Can't help but notice that these two lines are not exactly logically equivalent:
>  format_process_ctty (void *data, char *&destbuf)
>  {
>    _pinfo *p = (_pinfo *) data;
> -  if (p->ctty < 0)
> +  if (!CTTY_IS_VALID (p->ctty))
> 
> And here as well:
> 
> {
>    if (str == NULL)
>      str = _my_tls.locals.ttybuf;
> -  if (myself->ctty < 0)
> +  if (!CTTY_IS_VALID (myself->ctty))

Thanks.
You are right, however, ctty value 0 is never used, so no problem I think.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-03-07  3:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-07  2:33 [PATCH] Cygwin: ctty: Replace ctty constant with more descriptive macros Takashi Yano
     [not found] ` <DM8PR09MB7095179671A47284BE556204A5B79@DM8PR09MB7095.namprd09.prod.outlook.com>
2023-03-07  3:30   ` [EXTERNAL] " Takashi Yano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).