libspeechd SPDVoice cleanup api

[Trevor Saunders]

On Thu, Oct 23, 2014 at 11:29:28PM -0600, Jeremy Whiting wrote:
> Hey all,
> 
> The current libspeechd has api to get a list of voices for the
> synthesizer, but no api to safely clean up the data. The attached
> patch adds a new method free_spd_voices to do that. It iterates over
> the SPDVoice ** list freeing the strdup'ed strings and the SPDVoice
> structures themselves. I tested this with a modified qtspeech to use
> this new api and it builds and runs fine here. Let me know if I need
> to modify it in any way.

makes sense.

> 
> thanks,
> Jeremy
> 
> P.S. is there some patch review tool available from brailcom or
> something I should be using instead?

not that I'm aware of and personally I tend to prefer reviewing in
email.


> From 999f0280b1a0ac075e15296235b4b38fe55eb613 Mon Sep 17 00:00:00 2001
> From: Jeremy Whiting <jpwhiting at kde.org>
> Date: Thu, 23 Oct 2014 23:24:30 -0600
> Subject: [PATCH] Add free_spd_voices convenience method to free SPDVoice
>  structures.
> 
> Iterates over SPDVoice list and frees strings and SPDVoice structures.
> ---
>  src/api/c/libspeechd.c | 10 ++++++++++
>  src/api/c/libspeechd.h |  1 +
>  2 files changed, 11 insertions(+)
> 
> diff --git a/src/api/c/libspeechd.c b/src/api/c/libspeechd.c
> index 3f693b0..b9b918a 100644
> --- a/src/api/c/libspeechd.c
> +++ b/src/api/c/libspeechd.c
> @@ -1221,6 +1221,16 @@ SPDVoice **spd_list_synthesis_voices(SPDConnection * 
> connection)
>       return svoices;
>  }
>  
> +void free_spd_voices(SPDVoice** voices)

super picky, but wouldn't SPDVoice **voices be more consistant?

> +{
> +     int i = 0;
> +     while (voices != NULL && voices[i] != NULL) {

I'd say this function probably shouldn't accept a null voice list, or at
least the check should be manually hoisted out of the loop.

> +             free(voices[i]->name);
> +             free(voices[i]);
> +             ++i;
> +     }

you could get rid of the extra variable and do

for (; *voices; voices++) {
  free((*voices)->name);
  free(*voices);
}

> +}

and shouldn't you free the array of pointers?

Thanks!

Trev

> +
>  char **spd_execute_command_with_list_reply(SPDConnection * connection,
>                                          char *command)
>  {
> diff --git a/src/api/c/libspeechd.h b/src/api/c/libspeechd.h
> index be0d439..4d5c047 100644
> --- a/src/api/c/libspeechd.h
> +++ b/src/api/c/libspeechd.h
> @@ -221,6 +221,7 @@ int spd_get_message_list_fd(SPDConnection * connection, 
> int target,
>  char **spd_list_modules(SPDConnection * connection);
>  char **spd_list_voices(SPDConnection * connection);
>  SPDVoice **spd_list_synthesis_voices(SPDConnection * connection);
> +void free_spd_voices(SPDVoice** voices);
>  char **spd_execute_command_with_list_reply(SPDConnection * connection,
>                                          char *command);
>  
> -- 
> 2.1.2
> 

> _______________________________________________
> Speechd mailing list
> Speechd at lists.freebsoft.org
> http://lists.freebsoft.org/mailman/listinfo/speechd

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: 
<http://lists.freebsoft.org/pipermail/speechd/attachments/20141024/45b29088/attachment.pgp>