On Wed, Apr 06, 2022 at 11:54:05AM +0300, Andy Shevchenko wrote:
> On Wed, Apr 06, 2022 at 10:21:12AM +0200, Jiri Slaby wrote:
> > On 05. 04. 22, 18:11, Andy Shevchenko wrote:
> > > On Tue, Apr 05, 2022 at 01:24:37PM +0300, Ilpo Järvinen wrote:
> > > > + if (c == START_CHAR(tty)) {
> > > > + if (!lookahead_done) {
> > > > + start_tty(tty);
> > > > + process_echoes(tty);
> > > > + }
> > > > + return true;
> > > > + }
> > > > + if (c == STOP_CHAR(tty)) {
> > > > + if (!lookahead_done)
> > > > + stop_tty(tty);
> > > > + return true;
> > > > + }
> > > Looking into this I would first make a preparatory patch that splits out
> > > current code into something like
> > >
> > > static bool __n_tty_receive_char_special_no_lookahead(struct tty_struct *tty, unsigned char c)
> > > {
> > > ...current code...
> > > }
> > >
> > > Then in the patch 1 you add
> > >
> > > static bool __n_tty_receive_char_special_lookahead(struct tty_struct *tty, unsigned char c)
> > > {
> > > ...
> > > }
> > >
> > > static bool __n_tty_receive_char_special(struct tty_struct *tty, unsigned char c,
> > > bool lookahead_done)
> >
> > This should be dubbed better. Maybe n_tty_receive_char_flow_control()?
> >
> > And I would place the if (I_IXON(tty)) to the caller. I am a bit lost in
> > this pseudo code, so maybe this doesn't make sense in your proposal. I have
> > something like in my mind:
> >
> > if (I_IXON(tty))
> > return n_tty_receive_char_flow_control();
>
> My point to have three helpers which make each change cleaner:
>
> .-> n_tty_receive_char_flow_control_lah()
> |
> | .-> n_tty_receive_char_flow_control_no_lah()
> | |
> `- + -- n_tty_receive_char_flow_control()
>
> Where no_lah variant can be split as preparatory patch prepending the current
> series.
Another possibility to keep one function, but split it first in a separate
patch and then modify.
> And yes, calling I_IXON at the caller seems better.
>
> > Historically, these n_tty_receive* function names were a big mess. Don't
> > produce more of that by simply prepending only "__".
> >
> > > {
> > > if (!I_IXON(tty))
> > > return false;
> > >
> > > if (lookahead_done)
> > > return _lookahead();
> > >
> > > return _no_lookahead();
> > > }
--
With Best Regards,
Andy Shevchenko
