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

[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 <okash.khawaja@gmail.com>
Reviewed-by: Samuel Thibault <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;
 }

[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 <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;
 }

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

[Samuel Thibault]

Okash Khawaja, on mar. 20 juin 2017 08:31:20 +0100, wrote:
> 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 <okash.khawaja@gmail.com>

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

(I guess you have tested it)

> ---
>  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
> C'est vrai. FreeBSD pourrait être bien s'il avait une licence
> correcte.
Oh, mon troll-ô-meter vient d'exploser...
-+- OR in Guide du linuxien pervers - "Bien regler son troll-ô-meter"

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

[Okash Khawaja]

> On 20 Jun 2017, at 09:16, Samuel Thibault <samuel.thibault@ens-lyon.org> wrote:
> 
> Okash Khawaja, on mar. 20 juin 2017 08:31:20 +0100, wrote:
>> 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 <okash.khawaja@gmail.com>
> 
> Reviewed-by: Samuel Thibault <samuel.thibault@ens-lyon.org>
> 
> (I guess you have tested it)

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

Thanks!