Re: [PATCH 4/5] Cygwin: pty: Prevent pty from changing code page of parent console.

[Takashi Yano <takashi.yano@nifty.ne.jp>]

Hi Corinna,

On Mon, 18 Jan 2021 11:23:40 +0100
Corinna Vinschen wrote:
> I'm going to push patches 1 - 3.  In terms of patch 4 I have a few
> questions:
> 
> On Jan 15 17:32, Takashi Yano via Cygwin-patches wrote:
> > @@ -2185,7 +2185,7 @@ private:
> >    bool send_winch_maybe ();
> >    void setup ();
> >    bool set_unit ();
> > -  static bool need_invisible ();
> > +  static bool need_invisible (bool force=false);
> 
> Please add spaces, i. e., force = false
> 
> > +static DWORD
> > +get_console_process_id (DWORD pid, bool match)
> > +{
> > +  DWORD tmp;
> > +  DWORD num, num_req;
> > +  num = 1;
> > +  num_req = GetConsoleProcessList (&tmp, num);
> > +  DWORD *list;
> 
> So, assuming num_req is 1 after the call, shouldn't that skip the
> rest of the code?
> 
> > +  while (true)
> > +    {
> > +      list = (DWORD *)
> > +	HeapAlloc (GetProcessHeap (), 0, num_req * sizeof (DWORD));
> > +      num = num_req;
> > +      num_req = GetConsoleProcessList (list, num);
> > +      if (num_req > num)
> > +	HeapFree (GetProcessHeap (), 0, list);
> > +      else
> > +	break;
> > +    }
> > +  num = num_req;
> > +
> > +  tmp = 0;
> > +  for (DWORD i=0; i<num; i++)
> > +    if ((match && list[i] == pid) || (!match && list[i] != pid))
> > +      /* Last one is the oldest. */
> > +      /* https://github.com/microsoft/terminal/issues/95 */
> 
> Given that, wouldn't it make more sense to count backwards, from
> num - 1 to 0, from a performance perspective?

Thanks for the advice. I will submit the revised patch.

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