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

Okash Khawaja okash.khawaja at gmail.com
Mon Apr 10 16:17:17 EDT 2017


On Mon, Apr 10, 2017 at 12:53:37AM +0200, Samuel Thibault wrote:
> Hello,
> 
> 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?



More information about the Speakup mailing list