[patch] staging: speakup: fix synth caching when synth init fails

[Okash Khawaja]

synths[] array caches currently loaded synths. synth_add checks
synths[] before adding a new one. It however ignores the result of
do_synth_init. So when do_synth_init fails, the failed synth is still
cached. Since, as a result module loading fails too, synth_remove -
which is responsible for removing the cached synth - is never called.
Next time the failing synth is added again it succeeds because
synth_add finds it cached inside synths[].

This patch fixes this by caching a synth only after do_synth_init
succeeds.

Signed-off-by: Okash Khawaja <***@gmail.com>

---
drivers/staging/speakup/synth.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -445,10 +445,15 @@ int synth_add(struct spk_synth *in_synth
mutex_unlock(&spk_mutex);
return -1;
}
- synths[i++] = in_synth;
- synths[i] = NULL;
+
if (in_synth->startup)
status = do_synth_init(in_synth);
+
+ if (!status) {
+ synths[i++] = in_synth;
+ synths[i] = NULL;
+ }
+
mutex_unlock(&spk_mutex);
return status;
}

[Samuel Thibault]

Post by Okash Khawaja
synths[] array caches currently loaded synths. synth_add checks
synths[] before adding a new one. It however ignores the result of
do_synth_init. So when do_synth_init fails, the failed synth is still
cached. Since, as a result module loading fails too, synth_remove -
which is responsible for removing the cached synth - is never called.
Next time the failing synth is added again it succeeds because
synth_add finds it cached inside synths[].
This patch fixes this by caching a synth only after do_synth_init
succeeds.

Reviewed-by: Samuel Thibault <***@ens-lyon.org>

(I guess you have tested it)

Post by Okash Khawaja
---
drivers/staging/speakup/synth.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -445,10 +445,15 @@ int synth_add(struct spk_synth *in_synth
mutex_unlock(&spk_mutex);
return -1;
}
- synths[i++] = in_synth;
- synths[i] = NULL;
+
if (in_synth->startup)
status = do_synth_init(in_synth);
+
+ if (!status) {
+ synths[i++] = in_synth;
+ synths[i] = NULL;
+ }
+
mutex_unlock(&spk_mutex);
return status;
}

[Okash Khawaja]

Post by Samuel Thibault

Post by Okash Khawaja
synths[] array caches currently loaded synths. synth_add checks
synths[] before adding a new one. It however ignores the result of
do_synth_init. So when do_synth_init fails, the failed synth is still
cached. Since, as a result module loading fails too, synth_remove -
which is responsible for removing the cached synth - is never called.
Next time the failing synth is added again it succeeds because
synth_add finds it cached inside synths[].
This patch fixes this by caching a synth only after do_synth_init
succeeds.

(I guess you have tested it)

Yes tested it working after the patch. No obvious regressions :)

Thanks!

[Okash Khawaja]

synths[] array caches currently loaded synths. synth_add checks
synths[] before adding a new one. It however ignores the result of
do_synth_init. So when do_synth_init fails, the failed synth is still
cached. Since, as a result module loading fails too, synth_remove -
which is responsible for removing the cached synth - is never called.
Next time the failing synth is added again it succeeds because
synth_add finds it cached inside synths[].

This patch fixes this by caching a synth only after do_synth_init
succeeds.

Signed-off-by: Okash Khawaja <***@gmail.com>
Reviewed-by: Samuel Thibault <***@ens-lyon.org>

---
drivers/staging/speakup/synth.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

--- a/drivers/staging/speakup/synth.c
+++ b/drivers/staging/speakup/synth.c
@@ -445,10 +445,15 @@ int synth_add(struct spk_synth *in_synth
mutex_unlock(&spk_mutex);
return -1;
}
- synths[i++] = in_synth;
- synths[i] = NULL;
+
if (in_synth->startup)
status = do_synth_init(in_synth);
+
+ if (!status) {
+ synths[i++] = in_synth;
+ synths[i] = NULL;
+ }
+
mutex_unlock(&spk_mutex);
return status;
}