stdin pipe rename in 3.2.0

stdin pipe rename in 3.2.0

[Christoph Reiter]

Hey,

I noticed that the stdin pipe was renamed from

"\msys-dd50a72ab4668b33-pty1-from-master" to
"\msys-dd50a72ab4668b33-pty0-to-slave" in
https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=bb42852062073439

This trips up https://github.com/k-takata/ptycheck to detect the
cygpty which is used in various code bases.

Is there a reason it was renamed?

And while grepping I noticed the old name is still checked for in
other places like
https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/tty.cc;h=3c016315cdedb1dcca44cb3f3f96b87fd0b90a97;hb=HEAD#l162
which seems weird.

Thanks!

Re: stdin pipe rename in 3.2.0

[Brian Inglis]

On 2021-03-18 14:28, Christoph Reiter via Cygwin wrote:
> I noticed that the stdin pipe was renamed from
> "\msys-dd50a72ab4668b33-pty1-from-master" to
> "\msys-dd50a72ab4668b33-pty0-to-slave" in
> https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=bb42852062073439
> This trips up https://github.com/k-takata/ptycheck to detect the
> cygpty which is used in various code bases.
> Is there a reason it was renamed?
> And while grepping I noticed the old name is still checked for in
> other places like
> https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/tty.cc;h=3c016315cdedb1dcca44cb3f3f96b87fd0b90a97;hb=HEAD#l162
> which seems weird.

If these are being renamed should they not now use neutral terminology based on 
terms such as primary and secondary, and the primary/default repo branches on a 
term such as main?

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Re: stdin pipe rename in 3.2.0

[Christoph Reiter]

On Fri, Mar 19, 2021 at 4:47 AM Brian Inglis
<Brian.Inglis@systematicsw.ab.ca> wrote:
>
> On 2021-03-18 14:28, Christoph Reiter via Cygwin wrote:
> If these are being renamed should they not now use neutral terminology based on
> terms such as primary and secondary, and the primary/default repo branches on a
> term such as main?

Yes. Though there are ~700 instances of such terms in the cygwin code
so I'd suggest you propose this separately to the list.

Thanks!

Re: stdin pipe rename in 3.2.0

[Brian Inglis]

On 2021-03-19 01:53, Christoph Reiter via Cygwin wrote:
> On Fri, Mar 19, 2021 at 4:47 AM Brian Inglis wrote:
>> On 2021-03-18 14:28, Christoph Reiter via Cygwin wrote:
>> If these are being renamed should they not now use neutral terminology based on
>> terms such as primary and secondary, and the primary/default repo branches on a
>> term such as main?

> Yes. Though there are ~700 instances of such terms in the cygwin code
> so I'd suggest you propose this separately to the list.

I'm not suggesting anyone even attempt to do a mass clean up of such terms.

But as any area needs work where these terms are used, take the opportunity.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

This email may be disturbing to some readers as it contains
too much technical detail. Reader discretion is advised.
[Data in binary units and prefixes, physical quantities in SI.]

Re: stdin pipe rename in 3.2.0

[Takashi Yano]

On Thu, 18 Mar 2021 21:28:40 +0100
Christoph Reiter wrote:
> I noticed that the stdin pipe was renamed from
> 
> "\msys-dd50a72ab4668b33-pty1-from-master" to
> "\msys-dd50a72ab4668b33-pty0-to-slave" in
> https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=bb42852062073439

Actually, this is not renamed but newly introduced only
for non-cygwin (native) apps.

> This trips up https://github.com/k-takata/ptycheck to detect the
> cygpty which is used in various code bases.

If this change is affected, your process seems to be
a non-cygwin process.

> Is there a reason it was renamed?
> 
> And while grepping I noticed the old name is still checked for in
> other places like
> https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/tty.cc;h=3c016315cdedb1dcca44cb3f3f96b87fd0b90a97;hb=HEAD#l162
> which seems weird.

The name "from-master" is still used for cygwin
processes.

However, the naming was not appropriate.
The name of output pipes are:
"ptyNNNN-to-master-cyg" for cygwin process,
and
"ptyNNNN-to-master" for non-cygwin process.

However, the name of input pipes are:
"ptyNNNN-from-master" for cygwin process,
and
"ptyNNNN-to-slave" for non-cygwin process.
This is not only consistent but also very confusing.

I would like to rename these pipes to:
"ptyNNNN-from-master-cyg" for cygwin process,
"ptyNNNN-from-master" for non-cygwin process.

Corinna, is it possble to apply the patch for 3.2.0 release?

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

Re: stdin pipe rename in 3.2.0

[Takashi Yano]

On Fri, 19 Mar 2021 19:08:07 +0900
Takashi Yano wrote:
> This is not only consistent but also very confusing.

I mean "This is not only inconsistent but ..."

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

Re: stdin pipe rename in 3.2.0

[Christoph Reiter]

On Fri, Mar 19, 2021 at 11:09 AM Takashi Yano via Cygwin
<cygwin@cygwin.com> wrote:
>
> On Thu, 18 Mar 2021 21:28:40 +0100
> Christoph Reiter wrote:
> > I noticed that the stdin pipe was renamed from
> >
> > "\msys-dd50a72ab4668b33-pty1-from-master" to
> > "\msys-dd50a72ab4668b33-pty0-to-slave" in
> > https://cygwin.com/git/?p=newlib-cygwin.git;a=commit;h=bb42852062073439
>
> Actually, this is not renamed but newly introduced only
> for non-cygwin (native) apps.

ah, I see.

> > This trips up https://github.com/k-takata/ptycheck to detect the
> > cygpty which is used in various code bases.
>
> If this change is affected, your process seems to be
> a non-cygwin process.

yes, I noticed this with non-cygwin processes.

> > Is there a reason it was renamed?
> >
> > And while grepping I noticed the old name is still checked for in
> > other places like
> > https://cygwin.com/git/?p=newlib-cygwin.git;a=blob;f=winsup/cygwin/tty.cc;h=3c016315cdedb1dcca44cb3f3f96b87fd0b90a97;hb=HEAD#l162
> > which seems weird.
>
> The name "from-master" is still used for cygwin
> processes.

ok.

> However, the naming was not appropriate.
> The name of output pipes are:
> "ptyNNNN-to-master-cyg" for cygwin process,
> and
> "ptyNNNN-to-master" for non-cygwin process.
>
> However, the name of input pipes are:
> "ptyNNNN-from-master" for cygwin process,
> and
> "ptyNNNN-to-slave" for non-cygwin process.
> This is not only consistent but also very confusing.
>
> I would like to rename these pipes to:
> "ptyNNNN-from-master-cyg" for cygwin process,
> "ptyNNNN-from-master" for non-cygwin process.

If the naming was changed to keep https://github.com/k-takata/ptycheck
working then it would help me a lot. But I also don't mind patching
this on our side.

Thanks!

Re: stdin pipe rename in 3.2.0

[Takashi Yano]

On Fri, 19 Mar 2021 19:08:07 +0900
Takashi Yano wrote:
> Corinna, is it possble to apply the patch for 3.2.0 release?

By the way, duaring testing https://github.com/k-takata/ptycheck,
I noticed _get_osfhandle() does not work properly for stdout and
stderr. Shouldn't this

extern "C" long
_get_osfhandle (int fd)
{
  long res;

  cygheap_fdget cfd (fd);
  if (cfd >= 0)
    res = (long) cfd->get_handle ();
  else
    res = -1;

  syscall_printf ("%R = get_osfhandle(%d)", res, fd);
  return res;
}

be

extern "C" long
_get_osfhandle (int fd)
{
  long res;

  cygheap_fdget cfd (fd);
  if (cfd >= 0)
    {
      if (fd == 1 || fd == 2)
        res = (long) cfd->get_output_handle_cyg ();
      else
        res = (long) cfd->get_handle_cyg ();
    }
  else
    res = -1;

  syscall_printf ("%R = get_osfhandle(%d)", res, fd);
  return res;
}

?

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

Re: stdin pipe rename in 3.2.0

[Corinna Vinschen]

On Mar 19 21:05, Takashi Yano via Cygwin wrote:
> On Fri, 19 Mar 2021 19:08:07 +0900
> Takashi Yano wrote:
> > Corinna, is it possble to apply the patch for 3.2.0 release?

That's what release testing is for :)

> By the way, duaring testing https://github.com/k-takata/ptycheck,
> I noticed _get_osfhandle() does not work properly for stdout and
> stderr. Shouldn't this
> 
> extern "C" long
> _get_osfhandle (int fd)
> {
>   long res;
> 
>   cygheap_fdget cfd (fd);
>   if (cfd >= 0)
>     res = (long) cfd->get_handle ();
>   else
>     res = -1;
> 
>   syscall_printf ("%R = get_osfhandle(%d)", res, fd);
>   return res;
> }
> 
> be
> 
> extern "C" long
> _get_osfhandle (int fd)
> {
>   long res;
> 
>   cygheap_fdget cfd (fd);
>   if (cfd >= 0)
>     {
>       if (fd == 1 || fd == 2)
>         res = (long) cfd->get_output_handle_cyg ();
>       else
>         res = (long) cfd->get_handle_cyg ();
>     }
>   else
>     res = -1;
> 
>   syscall_printf ("%R = get_osfhandle(%d)", res, fd);
>   return res;
> }
> 
> ?

Maybe.  You introduced the "_cyg" handles, so you should know ;)

On a more serious note, this is, of course, a compatibility
problem.  While _get_osfhandle is called by a Cygwin application,
nobody knows what dubious actions that application will perform
on this handle.  In all likelyhood, it fetched the handle to call
Windows functions.  And *if* it does, wouldn't it make more sense
if the non-Cygwin handle is returned?


Thanks,
Corinna

Re: stdin pipe rename in 3.2.0

[Takashi Yano]

On Fri, 19 Mar 2021 13:58:40 +0100
Corinna Vinschen wrote:
> > extern "C" long
> > _get_osfhandle (int fd)
> > {
> >   long res;
> > 
> >   cygheap_fdget cfd (fd);
> >   if (cfd >= 0)
> >     {
> >       if (fd == 1 || fd == 2)
> >         res = (long) cfd->get_output_handle_cyg ();
> >       else
> >         res = (long) cfd->get_handle_cyg ();
> >     }
> >   else
> >     res = -1;
> > 
> >   syscall_printf ("%R = get_osfhandle(%d)", res, fd);
> >   return res;
> > }
> > 
> > ?
> 
> Maybe.  You introduced the "_cyg" handles, so you should know ;)
> 
> On a more serious note, this is, of course, a compatibility
> problem.  While _get_osfhandle is called by a Cygwin application,
> nobody knows what dubious actions that application will perform
> on this handle.  In all likelyhood, it fetched the handle to call
> Windows functions.  And *if* it does, wouldn't it make more sense
> if the non-Cygwin handle is returned?

You are right. If get_output_handle_cyg() is returned, OPOST
processing and charset conversion does not work. Thanks!
As for input, get_handle_cyg() should be returned rather than
get_handle() because reading from get_handle() will fail because
master writes input to cygwin handle for cygwin process.

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