Zhaoming Luo, le sam. 08 févr. 2025 12:51:04 +0800, a ecrit:
> On Sat, Feb 08, 2025 at 08:55:31AM +0800, Zhaoming Luo wrote:
> > On Fri, Feb 07, 2025 at 01:57:01PM +0100, Samuel Thibault wrote:
> > > Zhaoming Luo, le ven. 07 févr. 2025 18:44:50 +0800, a ecrit:
> > > > This is V2 of figuring out the issue. V1 is abandoned due to my mistake,
> > > > but this time I can confirm the source of issue. This time I ran
> > > > rpctrace directly on vim, and used two printfs to locate the isatty().
> > > > 
> > > > ```
> > > > printf ("Start isatty!\n");
> > > > if (mch_isatty(in))
> > > >     channel->ch_to_be_closed |= (1U << PART_IN);
> > > > printf ("Finish isatty!\n");
> > > > ```
> > > > 
> > > > rpctrace output:
> > > > ```
> > > > ...
> > > > 30<--44(pid920)->io_write_request ("Start isatty!\n" -1) = 0 14
> > > > 56<--24(pid920)->term_getctty () = 0x4000002d (Operation not supported)
> > > > 30<--44(pid920)->io_write_request ("Finish isatty!\n" -1) = 0 15
> > > > ...
> > > > ```
> > > > 
> > > > The error seems to be from [2].
> > > 
> > > Indeed, if it was plugged to another translator you'd get a mig bad id
> > > error.
> > > 
> > > My guess is that it's cred->pi.class != tty_class that fails, indeed:
> > > 
> > >   int ret = openpty(&master, &slave, s, &term, &win);
> > >   printf("%d\n", isatty(master));
> > >   printf("%d\n", isatty(slave));
> > > 
> > > says 0 for master on the Hurd while it says 1 on Linux
> > > 
> > > The pi.class != tty_class condition was previously because of a lookup,
> > > but nowadays with the protected payloads we didn't need a lookup any
> > > more and it was replaced with a check that we find cred in the proper
> > > place. I'd say we should also accept the class being pty_class, it
> > > does not seem it would harm: in the uses of term_getctty that I can see,
> > > it matches the obtained cttyid with the one of the process, or it's an
> > > explicit request.
> > 
> > Ok, I will submit a patch so EOPNOSUPP will be returned when
> > cred->pi.class != pty_class is also true.
> > 
> 
> I got stuck at the patch since I'm not sure whether I should add a
> condition like:
> 
> ```
>    if (!cred
>        || cred->pi.bucket != term_bucket
> -      || cred->pi.class != tty_class)
> +      || (cred->pi.class != tty_class && cred->pi.class != pty_class))
>      return EOPNOTSUPP;
> ```

Yes, that's it.

> Or directly remove cred->pi.class != tty_class.

See the other functions how they do it. They do check the class even if
accepting both.

Samuel

Reply via email to