Re: [PATCH 1/2] tty: Add lookahead param to receive_buf

[Jiri Slaby]

On 05. 04. 22, 18:03, Andy Shevchenko wrote:
> On Tue, Apr 05, 2022 at 01:24:36PM +0300, Ilpo Järvinen wrote:
> > After lookahead for XON/XOFF characters is added by the next
> > patch, the receive side needs to ensure the flow-control
> > actions are not retaken later on when those same characters
> > get read by TTY.
> >
> > Thus, pass lookahead count to receive_buf and skip
> > flow-control character actions if already taken for the
> > character in question. Lookahead count will become live after
> > the next patch.
>
> ...
>
> > -static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c)
> > +static void n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
> > +				       bool lookahead_done)
> >   {
> >   	struct n_tty_data *ldata = tty->disc_data;
> >   
> >   	if (I_IXON(tty)) {
> >   		if (c == START_CHAR(tty)) {
> > -			start_tty(tty);
> > -			process_echoes(tty);
> > +			if (!lookahead_done) {
> > +				start_tty(tty);
> > +				process_echoes(tty);
> > +			}
> >   			return;
> >   		}
> >   		if (c == STOP_CHAR(tty)) {
> > -			stop_tty(tty);
> > +			if (!lookahead_done)
> > +				stop_tty(tty);
> >   			return;
> >   		}
>
> Wouldn't be cleaner to inside out the conditionals?

Seconded.

> 	if (I_IXON(tty)) {
> 		if (lookahead_done) {
> 			// Can be joined, but I think this is better

I would join them, IMO it'd be still easy to read and to follow too.

> 			if (c == START_CHAR(tty))
> 				return;
> 			if (c == STOP_CHAR(tty))
> 				return;
> 		} else {
> 			if (c == START_CHAR(tty)) {
> 				start_tty(tty);
> 				process_echoes(tty);
> 				return;
> 			}
> 			if (c == STOP_CHAR(tty)) {
> 				stop_tty(tty);
> 				return;
> 			}
> 		}
> 	}

thanks,
--
js
suse labs