[PATCH 2/2] Subject: staging: speakup: refactor to use existing code in vt
Gregory Nowak
greg at gregn.net
Fri Mar 29 20:10:39 EDT 2019
Which kernel versions should this patch against? Also, are the changes
under the hood only, or does something change in how the user selects
and pastes text as well?
Greg
On Fri, Mar 29, 2019 at 11:31:04AM +0000, Okash Khawaja wrote:
> This patch replaces speakup's implementations with calls to functions
> in tty/vt/selection.c. Those functions are:
>
> cancel_selection()
> do_set_selection()
> paste_selection()
>
> Currently setting selection is done in interrupt context. However,
> do_set_selection() can sleep - for instance, it requires console_lock
> which can sleep. Therefore we offload that work to a work_struct thread,
> following the same pattern which was already set for paste_selection().
> When setting selection, we also get a reference to tty and make sure to
> release the reference before returning.
>
> struct speakup_paste_work has been renamed to the more generic
> speakup_selection_work because it is now used for both pasting as well
> as setting selection.
>
> Signed-off-by: Okash Khawaja <okash.khawaja at gmail.com>
> ---
> drivers/staging/speakup/main.c | 1 +
> drivers/staging/speakup/selection.c | 187 ++++++++++++------------------------
> drivers/staging/speakup/speakup.h | 1 +
> 3 files changed, 64 insertions(+), 125 deletions(-)
>
> diff --git a/drivers/staging/speakup/main.c b/drivers/staging/speakup/main.c
> index b6a65b0..488f253 100644
> --- a/drivers/staging/speakup/main.c
> +++ b/drivers/staging/speakup/main.c
> @@ -2319,6 +2319,7 @@ static void __exit speakup_exit(void)
> unregister_keyboard_notifier(&keyboard_notifier_block);
> unregister_vt_notifier(&vt_notifier_block);
> speakup_unregister_devsynth();
> + speakup_cancel_selection();
> speakup_cancel_paste();
> del_timer_sync(&cursor_timer);
> kthread_stop(speakup_task);
> diff --git a/drivers/staging/speakup/selection.c b/drivers/staging/speakup/selection.c
> index 0ed1fef..823c679 100644
> --- a/drivers/staging/speakup/selection.c
> +++ b/drivers/staging/speakup/selection.c
> @@ -9,162 +9,99 @@
> #include <linux/tty.h>
> #include <linux/tty_flip.h>
> #include <linux/atomic.h>
> +#include <linux/console.h>
>
> #include "speakup.h"
>
> -/* ------ cut and paste ----- */
> -/* Don't take this from <ctype.h>: 011-015 on the screen aren't spaces */
> -#define ishardspace(c) ((c) == ' ')
> -
> unsigned short spk_xs, spk_ys, spk_xe, spk_ye; /* our region points */
> -
> -/* Variables for selection control. */
> -/* must not be deallocated */
> struct vc_data *spk_sel_cons;
> -/* cleared by clear_selection */
> -static int sel_start = -1;
> -static int sel_end;
> -static int sel_buffer_lth;
> -static char *sel_buffer;
>
> -static unsigned char sel_pos(int n)
> -{
> - return inverse_translate(spk_sel_cons,
> - screen_glyph(spk_sel_cons, n), 0);
> -}
> +struct speakup_selection_work {
> + struct work_struct work;
> + struct tiocl_selection sel;
> + struct tty_struct *tty;
> +};
>
> void speakup_clear_selection(void)
> {
> - sel_start = -1;
> + console_lock();
> + clear_selection();
> + console_unlock();
> }
>
> -/* does screen address p correspond to character at LH/RH edge of screen? */
> -static int atedge(const int p, int size_row)
> +static void __speakup_set_selection(struct work_struct *work)
> {
> - return !(p % size_row) || !((p + 2) % size_row);
> -}
> + struct speakup_selection_work *ssw =
> + container_of(work, struct speakup_selection_work, work);
>
> -/* constrain v such that v <= u */
> -static unsigned short limit(const unsigned short v, const unsigned short u)
> -{
> - return (v > u) ? u : v;
> -}
> + struct tty_struct *tty;
> + struct tiocl_selection sel;
>
> -int speakup_set_selection(struct tty_struct *tty)
> -{
> - int new_sel_start, new_sel_end;
> - char *bp, *obp;
> - int i, ps, pe;
> - struct vc_data *vc = vc_cons[fg_console].d;
> + sel = ssw->sel;
>
> - spk_xs = limit(spk_xs, vc->vc_cols - 1);
> - spk_ys = limit(spk_ys, vc->vc_rows - 1);
> - spk_xe = limit(spk_xe, vc->vc_cols - 1);
> - spk_ye = limit(spk_ye, vc->vc_rows - 1);
> - ps = spk_ys * vc->vc_size_row + (spk_xs << 1);
> - pe = spk_ye * vc->vc_size_row + (spk_xe << 1);
> + /* this ensures we copy sel before releasing the lock below */
> + mb();
>
> - if (ps > pe) /* make sel_start <= sel_end */
> - swap(ps, pe);
> + /* release the lock by setting tty of the struct to NULL */
> + tty = xchg(&ssw->tty, NULL);
>
> if (spk_sel_cons != vc_cons[fg_console].d) {
> - speakup_clear_selection();
> spk_sel_cons = vc_cons[fg_console].d;
> - dev_warn(tty->dev,
> - "Selection: mark console not the same as cut\n");
> - return -EINVAL;
> + pr_warn("Selection: mark console not the same as cut\n");
> + goto unref;
> }
>
> - new_sel_start = ps;
> - new_sel_end = pe;
> -
> - /* select to end of line if on trailing space */
> - if (new_sel_end > new_sel_start &&
> - !atedge(new_sel_end, vc->vc_size_row) &&
> - ishardspace(sel_pos(new_sel_end))) {
> - for (pe = new_sel_end + 2; ; pe += 2)
> - if (!ishardspace(sel_pos(pe)) ||
> - atedge(pe, vc->vc_size_row))
> - break;
> - if (ishardspace(sel_pos(pe)))
> - new_sel_end = pe;
> - }
> - if ((new_sel_start == sel_start) && (new_sel_end == sel_end))
> - return 0; /* no action required */
> -
> - sel_start = new_sel_start;
> - sel_end = new_sel_end;
> - /* Allocate a new buffer before freeing the old one ... */
> - bp = kmalloc((sel_end - sel_start) / 2 + 1, GFP_ATOMIC);
> - if (!bp) {
> - speakup_clear_selection();
> - return -ENOMEM;
> - }
> - kfree(sel_buffer);
> - sel_buffer = bp;
> -
> - obp = bp;
> - for (i = sel_start; i <= sel_end; i += 2) {
> - *bp = sel_pos(i);
> - if (!ishardspace(*bp++))
> - obp = bp;
> - if (!((i + 2) % vc->vc_size_row)) {
> - /* strip trailing blanks from line and add newline,
> - * unless non-space at end of line.
> - */
> - if (obp != bp) {
> - bp = obp;
> - *bp++ = '\r';
> - }
> - obp = bp;
> - }
> - }
> - sel_buffer_lth = bp - sel_buffer;
> - return 0;
> + console_lock();
> + do_set_selection(&sel, tty);
> + console_unlock();
> +
> +unref:
> + tty_kref_put(tty);
> }
>
> -struct speakup_paste_work {
> - struct work_struct work;
> - struct tty_struct *tty;
> +static struct speakup_selection_work speakup_sel_work = {
> + .work = __WORK_INITIALIZER(speakup_sel_work.work,
> + __speakup_set_selection)
> };
>
> +int speakup_set_selection(struct tty_struct *tty)
> +{
> + if (cmpxchg(&speakup_sel_work.tty, NULL, tty))
> + return -EBUSY;
> + /* now we have the 'lock' by setting tty member of
> + * speakup_selection_work
> + */
> +
> + tty_kref_get(tty);
> +
> + speakup_sel_work.sel.xs = spk_xs + 1;
> + speakup_sel_work.sel.ys = spk_ys + 1;
> + speakup_sel_work.sel.xe = spk_xe + 1;
> + speakup_sel_work.sel.ye = spk_ye + 1;
> + speakup_sel_work.sel.sel_mode = TIOCL_SELCHAR;
> +
> + schedule_work_on(WORK_CPU_UNBOUND, &speakup_sel_work.work);
> +
> + return 0;
> +}
> +
> +void speakup_cancel_selection(void)
> +{
> + cancel_work_sync(&speakup_sel_work.work);
> + tty_kref_put(speakup_sel_work.tty);
> +}
> +
> static void __speakup_paste_selection(struct work_struct *work)
> {
> - struct speakup_paste_work *spw =
> - container_of(work, struct speakup_paste_work, work);
> - struct tty_struct *tty = xchg(&spw->tty, NULL);
> - struct vc_data *vc = (struct vc_data *)tty->driver_data;
> - int pasted = 0, count;
> - struct tty_ldisc *ld;
> - DECLARE_WAITQUEUE(wait, current);
> -
> - ld = tty_ldisc_ref(tty);
> - if (!ld)
> - goto tty_unref;
> - tty_buffer_lock_exclusive(&vc->port);
> -
> - add_wait_queue(&vc->paste_wait, &wait);
> - while (sel_buffer && sel_buffer_lth > pasted) {
> - set_current_state(TASK_INTERRUPTIBLE);
> - if (tty_throttled(tty)) {
> - schedule();
> - continue;
> - }
> - count = sel_buffer_lth - pasted;
> - count = tty_ldisc_receive_buf(ld, sel_buffer + pasted, NULL,
> - count);
> - pasted += count;
> - }
> - remove_wait_queue(&vc->paste_wait, &wait);
> - __set_current_state(TASK_RUNNING);
> + struct speakup_selection_work *ssw =
> + container_of(work, struct speakup_selection_work, work);
> + struct tty_struct *tty = xchg(&ssw->tty, NULL);
>
> - tty_buffer_unlock_exclusive(&vc->port);
> - tty_ldisc_deref(ld);
> -tty_unref:
> + paste_selection(tty);
> tty_kref_put(tty);
> }
>
> -static struct speakup_paste_work speakup_paste_work = {
> +static struct speakup_selection_work speakup_paste_work = {
> .work = __WORK_INITIALIZER(speakup_paste_work.work,
> __speakup_paste_selection)
> };
> diff --git a/drivers/staging/speakup/speakup.h b/drivers/staging/speakup/speakup.h
> index e4f4f00..74fe49c 100644
> --- a/drivers/staging/speakup/speakup.h
> +++ b/drivers/staging/speakup/speakup.h
> @@ -72,6 +72,7 @@
> void synth_buffer_clear(void);
> void speakup_clear_selection(void);
> int speakup_set_selection(struct tty_struct *tty);
> +void speakup_cancel_selection(void);
> int speakup_paste_selection(struct tty_struct *tty);
> void speakup_cancel_paste(void);
> void speakup_register_devsynth(void);
> --
> 1.8.3.1
>
--
web site: http://www.gregn.net
gpg public key: http://www.gregn.net/pubkey.asc
skype: gregn1
(authorization required, add me to your contacts list first)
If we haven't been in touch before, e-mail me before adding me to your contacts.
--
Free domains: http://www.eu.org/ or mail dns-manager at EU.org
More information about the Speakup
mailing list