[Toybox] microcom.c discarding data due to TCSAFLUSH

[enh]

On Mon, May 20, 2024 at 10:43 AM Yi-Yo Chiang <yochiang at google.com> wrote:
>
> Found this out today when comparing captured logs. Every time I connect to a pty the first few hundreds of bytes seem to always be missing.
> I then traced to this line https://github.com/landley/toybox/blob/master/toys/net/microcom.c#L103 and it's the tcsetattr(TCSAFLUSH) that is discarding buffered input data. Something like this is happening:
>
> 1. (process 1) opens and write "xxxxyyyy" to the ptmx end. The data is not immediately read by another process, so it's buffered somewhere. (in the pty driver?)
> 2. (process 2 / microcom) opens and flushes (TCSAFLUSH) the pts end, and then start polling data. Receives "yyyy". Part of the buffered input data were flushed and discarded.
>
> Man page says (https://man7.org/linux/man-pages/man3/termios.3.html):
>
>        TCSANOW
>               the change occurs immediately.
>
>        TCSADRAIN
>               the change occurs after all output written to fd has been
>               transmitted.  This option should be used when changing
>               parameters that affect output.
>
>        TCSAFLUSH
>               the change occurs after all output written to the object
>               referred by fd has been transmitted, and all input that
>               has been received but not read will be discarded before
>               the change is made.
>
> Is there any particular reason to use TCSAFLUSH here?
> If not, can we change to TCSADRAIN or TCSANOW. I don't think there is good reason to _discard received data_ just to set the terminal mode...? Is there really a real world case that the device termios is so dirty that all data, from before setting raw mode, must be discarded?

not that i know of, but iirc that was my thinking here --- "i don't
know what state this is in; let's just get rid of it". now you have a
motivating example for not doing that :-)

> I also tried to modify the microcom code to skip tcsetattr() if the device termios is already equal to the mode we are setting it.
> `if (old_termios != new_termios) tcsetattr(new_termios, TCSAFLUSH)`
> However this doesn't work because microcom always tries to set the device baud. For example a pty device might be configured to use buad 38400, but microcom would want it to be 115200, thus flushing it's data. but pty doesn't really care about the baud most of the time AFAIK, so flushing data in this case just seems disruptive to the user experience.