* [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).