Re: [PATCH v3 1/2] Cygwin: pty: Introduce a helper function get_handle_from_process()

Mark Geisert <[email protected]> Sun, 21 Jun 2026 00:16:21 -0700
Newsgroups gmane.os.cygwin.patches
Message-ID <[email protected]>
Hi Takashi,

On 6/13/2026 7:06 AM, Takashi Yano wrote:
> The current pty code performs the sequence:
>    OpenProcess() -> DuplicateHandle()
> in various places. This helper function encapsulates that sequence
> to improve readability and maintainability.
> 
> Signed-off-by: Takashi Yano <[email protected]>
> Reviewed-by:
> ---
>   winsup/cygwin/fhandler/pty.cc | 66 +++++++++++++++++------------------
>   1 file changed, 33 insertions(+), 33 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
> index 2558fa799..e60e30230 100644
> --- a/winsup/cygwin/fhandler/pty.cc
> +++ b/winsup/cygwin/fhandler/pty.cc
> @@ -2213,6 +2213,23 @@ fhandler_pty_common::close (int flag)
>     return 0;
>   }
>   
> +static inline HANDLE
> +get_handle_from_process (DWORD pid, HANDLE h, bool inh = false)
> +{
> +  HANDLE ret = NULL;
> +  HANDLE owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE, pid);
> +  if (owner)
> +    {
> +      if (!DuplicateHandle (owner, h, GetCurrentProcess (), &ret, 0, inh,
> +			    DUPLICATE_SAME_ACCESS))
> +	termios_printf ("DuplicateHandle() %p from process %d (%E)", h, pid);
> +      CloseHandle (owner);
> +    }
> +  else
> +    termios_printf ("OpenProcess (%d) failed (%E).", pid);
> +  return ret;
> +}
> +
>   void
>   fhandler_pty_common::resize_pseudo_console (struct winsize *ws)
>   {
> @@ -2220,15 +2237,14 @@ fhandler_pty_common::resize_pseudo_console (struct winsize *ws)
>     size.X = ws->ws_col;
>     size.Y = ws->ws_row;
>     HPCON_INTERNAL hpcon_local;
> -  HANDLE pcon_owner =
> -    OpenProcess (PROCESS_DUP_HANDLE, FALSE, get_ttyp ()->nat_pipe_owner_pid);
> -  DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_write_pipe,
> -		   GetCurrentProcess (), &hpcon_local.hWritePipe,
> -		   0, FALSE, DUPLICATE_SAME_ACCESS);
> +  hpcon_local.hWritePipe =
> +    get_handle_from_process (get_ttyp ()->nat_pipe_owner_pid,
> +			     get_ttyp ()->h_pcon_write_pipe);
> +  if (hpcon_local.hWritePipe == NULL)
> +    return;
>     acquire_attach_mutex (mutex_timeout);
>     ResizePseudoConsole ((HPCON) &hpcon_local, size);
>     release_attach_mutex ();
> -  CloseHandle (pcon_owner);
>     CloseHandle (hpcon_local.hWritePipe);
>   }
>   
> @@ -2490,18 +2506,13 @@ fhandler_pty_master::write (const void *ptr, size_t len)
>   	    {
>   	      if (h_pcon_in_dupped)
>   		ForceCloseHandle (h_pcon_in_dupped);
> -	      h_pcon_in_dupped = NULL;
> -	      nat_pipe_owner_pid_dupped = 0;
> -	      HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> -					       get_ttyp ()->nat_pipe_owner_pid);
> -	      if (pcon_owner)
> -		{
> -		  DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
> -				   GetCurrentProcess (), &h_pcon_in_dupped,
> -				   0, FALSE, DUPLICATE_SAME_ACCESS);
> -		  nat_pipe_owner_pid_dupped = get_ttyp ()->nat_pipe_owner_pid;
> -		  CloseHandle (pcon_owner);
> -		}
> +	      h_pcon_in_dupped =
> +		get_handle_from_process (get_ttyp ()->nat_pipe_owner_pid,
> +					 get_ttyp ()->h_pcon_in);
> +	      if (h_pcon_in_dupped)
> +		nat_pipe_owner_pid_dupped = get_ttyp ()->nat_pipe_owner_pid;
> +	      else
> +		nat_pipe_owner_pid_dupped = 0;
>   	    }
>   	  else
>   	    {
> @@ -4265,16 +4276,9 @@ fhandler_pty_slave::transfer_input (tty::xfer_dir dir, HANDLE from, tty *ttyp,
>       to = ttyp->to_slave ();
>   
>     pinfo p (ttyp->master_pid);
> -  HANDLE pty_owner = NULL;
>     if (p)
> -    pty_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE, p->dwProcessId);
> -  if (pty_owner)
> -    {
> -      DuplicateHandle (pty_owner, to, GetCurrentProcess (), &to,
> -		       0, TRUE, DUPLICATE_SAME_ACCESS);
> -      CloseHandle (pty_owner);
> -    }
> -  else
> +    to = get_handle_from_process (p->dwProcessId, to, true);
> +  if (to == NULL)
>       {
>         char pipe[MAX_PATH];
>         __small_sprintf (pipe,
> @@ -4571,12 +4575,8 @@ fhandler_pty_slave::setpgid_aux (pid_t pid)
>         if (get_ttyp ()->pcon_activated && get_ttyp ()->nat_pipe_owner_pid
>   	  && !get_console_process_id (get_ttyp ()->nat_pipe_owner_pid, true))
>   	{
> -	  HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
> -					   get_ttyp ()->nat_pipe_owner_pid);
> -	  DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
> -			   GetCurrentProcess (), &from,
> -			   0, TRUE, DUPLICATE_SAME_ACCESS);
> -	  CloseHandle (pcon_owner);
> +	  from = get_handle_from_process (get_ttyp ()->nat_pipe_owner_pid,
> +					  get_ttyp ()->h_pcon_in, true);
>   	  DWORD target_pid = get_ttyp ()->nat_pipe_owner_pid;
>   	  resume_pid = attach_console_temporarily (target_pid);
>   	  attach_restore = true;

LGTM.  OK to push.

..mark