Skip to contentSkip to conversation

staging: speakup: safely unregister ldisc

PATCH New Okash Khawaja 1 participant 3 messages Jul 16, 2017 Staging

lore
Share & tools
lore
#0 lore raw

cover staging: speakup: safely unregister ldisc

Hi,

These patches make sure that N_SPEAKUP is correctly unregistered when all speakup related modules are unloaded, making sure the refcount correctly represents the number of users.

Patch 1: simply adds functions to register and unregister ldisc, without
changing existing behaviour
Patch 2: uses those functions to register and unregister ldisc correctly

Thanks,
Okash

#1 lore raw

PATCH 1/2 staging: speakup: add functions to register and unregister ldisc

This patch adds the above two functions and makes them available to main.c where they will be called during init and exit functions of main speakup module. Following patch will make use of them.

---
 drivers/staging/speakup/spk_priv.h  |    2 ++
 drivers/staging/speakup/spk_ttyio.c |   12 ++++++++++++
 2 files changed, 14 insertions(+)
drivers/staging/speakup/spk_priv.h +2 -0
@@ -48,6 +48,8 @@ void spk_stop_serial_interrupt(void);
 int spk_wait_for_xmitr(struct spk_synth *in_synth);
 void spk_serial_release(void);
 void spk_ttyio_release(void);
+void spk_ttyio_register_ldisc(void);
+void spk_ttyio_unregister_ldisc(void);
 
 void synth_buffer_skip_nonlatin1(void);
 u16 synth_buffer_getc(void);
drivers/staging/speakup/spk_ttyio.c +12 -0
@@ -200,6 +200,18 @@ static int spk_ttyio_initialise_ldisc(st
 	return ret;
 }
 
+void spk_ttyio_register_ldisc(void)
+{
+	if (tty_register_ldisc(N_SPEAKUP, &spk_ttyio_ldisc_ops))
+		pr_warn("speakup: Error registering line discipline. Most synths won't work.\n");
+}
+
+void spk_ttyio_unregister_ldisc(void)
+{
+	if (tty_unregister_ldisc(N_SPEAKUP))
+		pr_warn("speakup: Couldn't unregister ldisc\n");
+}
+
 static int spk_ttyio_out(struct spk_synth *in_synth, const char ch)
 {
 	if (in_synth->alive && speakup_tty && speakup_tty->ops->write) {
#2 lore raw

PATCH 2/2 staging: speakup: safely register and unregister ldisc

In reply to Okash Khawaja

This patch makes use of functions added in the previous patch. It registers ldisc during init of main speakup module and unregisters it during exit. It also removes the code to register ldisc every time a synth module is loaded. This way we only register the ldisc once when main speakup module is loaded. Since main speakup module is required by all synth modules, it is only unloaded when all synths have been unloaded. Therefore we unregister the ldisc once, when all speakup related references to the ldisc have returned. In unlikely scenario of something outside speakup using the ldisc, the ldisc refcount check in tty_unregister_ldisc will ensure that it is not unregistered while in use.

The function to register ldisc doesn't cause speakup init function to fail. That is different from current behaviour where failure to register ldisc results in failure to load the specific synth module. This is because speakup module is also required by those synths which don't use tty and ldisc. We don't want to prevent those modules from loading when ldisc fails to register. The synth modules will correctly fail when trying to set N_SPEAKUP to tty, if ldisc registrationi had failed.

---
 drivers/staging/speakup/main.c      |    2 ++
 drivers/staging/speakup/spk_ttyio.c |    8 ++------
 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/staging/speakup/main.c +2 -0
@@ -2315,6 +2315,7 @@ static void __exit speakup_exit(void)
 	mutex_lock(&spk_mutex);
 	synth_release();
 	mutex_unlock(&spk_mutex);
+	spk_ttyio_unregister_ldisc();
 
 	speakup_kobj_exit();
 
@@ -2377,6 +2378,7 @@ static int __init speakup_init(void)
 	if (err)
 		goto error_kobjects;
 
+	spk_ttyio_register_ldisc();
 	synth_init(synth_name);
 	speakup_register_devsynth();
 	/*
drivers/staging/speakup/spk_ttyio.c +2 -6
@@ -154,12 +154,6 @@ static int spk_ttyio_initialise_ldisc(st
 	struct ktermios tmp_termios;
 	dev_t dev;
 
-	ret = tty_register_ldisc(N_SPEAKUP, &spk_ttyio_ldisc_ops);
-	if (ret) {
-		pr_err("Error registering line discipline.\n");
-		return ret;
-	}
-
 	ret = get_dev_to_use(synth, &dev);
 	if (ret)
 		return ret;
@@ -196,6 +190,8 @@ static int spk_ttyio_initialise_ldisc(st
 	tty_unlock(tty);
 
 	ret = tty_set_ldisc(tty, N_SPEAKUP);
+	if (ret)
+		pr_err("speakup: Failed to set N_SPEAKUP on tty\n");
 
 	return ret;
 }