[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [bug-inetutils] Telnetd fails to handle some of TIOCPKT control byte
Mats Erik Andersson
Re: [bug-inetutils] Telnetd fails to handle some of TIOCPKT control bytes.
Sun, 22 Mar 2015 10:37:37 +0100
Dear Takashi Yano!
Den 22 March 2015 klockan 14:32 skrev Takashi Yano detta:
> Thank you for your reply.
> With your patch, I have confirmed the problem has disappeared.
> However, I am a little bit concerned about calling pty_get_char(0)
> twice in the case of TIOCPKT_FLUSHWRITE or TIOCPKT_IOCTL.
> Is it intentional behaviour?
A relevant observation. When acting on TIOCPKT_IOCTL the call sequence
copy_termbuf (); /* Pty buffer is now emtied. */
discards the packet preamble, then copies payload and resets the
buffer. This will make sure that any following pty_get_char(0)
becomes a no-op as no data is left. The remark above was introduced
to the development head to record this observation, but I left it
out in my comment to the list as is has no effect on the program.
The case of TIOCPKT_FLUSHWRITE is different, as only the network
buffer is touched, apart from the single pty_get_char(0). Now it
seems that your worry is justified. According to pty(7D) on Solaris
When a pseudo-terminal is in packet mode, the controller
device will return data preceded by TIOCPKT_DATA (= 0),
or a single byte reflecting control status information.
This means that TIOCPKT_FLUSHWRITE arrives isolated, so also in this
case the pty buffer is empty after the first pty_get_char(0) and the
second becomes a no-op again. However, even though the control byte
was flushed, is is still preserved in the variable 'c', so the
next test is still correct even if (TIOCPKT_FLUSHWRITE | TIOCPKT_STOP)
had been received. Neither of TIOCPKT_NOSTOP and TIOCPKT_STOP touches
the pty buffer, so no harmful side effect is possible. The code would
look better without this first pty_get_char(0), so I might remove it
for better understanding and quality.
I thank you for your interest and care in this matter.
Mats Erik Andersson