[patch 6/7] staging: speakup: add TTY-based input functionality

Samuel Thibault samuel.thibault at ens-lyon.org
Mon Apr 10 17:01:04 EDT 2017


Okash Khawaja, on lun. 10 avril 2017 21:17:17 +0100, wrote:
> On Mon, Apr 10, 2017 at 12:53:37AM +0200, Samuel Thibault wrote:
> > We need to add comments about the synchronization, added inline:
> > 
> > Okash Khawaja, on lun. 03 avril 2017 21:21:29 +0100, wrote:
> > > +static int spk_ttyio_receive_buf2(struct tty_struct *tty,
> > > +		const unsigned char *cp, char *fp, int count)
> > > +{
> > > +	struct spk_ldisc_data *ldisc_data = (struct spk_ldisc_data *)tty->disc_data;
> > > +
> > > +	if (spk_ttyio_synth->read_buff_add) {
> > > +		int i;
> > > +		for (i = 0; i < count; i++)
> > > +			spk_ttyio_synth->read_buff_add(cp[i]);
> > > +
> > > +		return count;
> > > +	}
> > > +
> > > +	if (ldisc_data->buf_free == 0)
> >                 /* ttyio_in will tty_schedule_flip */
> > > +		return 0;
> > > +
> >         /* Make sure the consumer has read buf before we have seen
> >          * buf_free == 1 and overwrite buf */
> > > +	mb();
> > > +
> > > +	ldisc_data->buf = cp[0];
> > > +	ldisc_data->buf_free--;
> > 
> > Don't decrement, set to 0. Decrementing would make the reader think
> > that there is potential for multi-character support, but with such a
> > lightweight synchronization there can't be. The fact that buf_free is 1
> > allows the tty part to set it to 0, but not more.
> How about changing the type to bool and switching between true and false
> only?

Why not indeed.

Samuel


More information about the Speakup mailing list