[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
libspeechd SPDVoice cleanup api
From: |
Jeremy Whiting |
Subject: |
libspeechd SPDVoice cleanup api |
Date: |
Sat, 25 Oct 2014 21:41:24 -0600 |
Luke,
Thanks for applying this.
Trevor,
Oops, I changed the spacing around the ** in the .h but not in the .c
file, my bad. Some global updating the spacing/style would probably be
good I guess, though it doesn't bother me, I've seen so many different
styles between c code here, qt c++ code in qtspeech, some other styles
in kde sources etc. that I generally ignore style issues, though I try
to at least use tabs if the file has a bunch of tabs in it already.
thanks,
Jeremy
On Sat, Oct 25, 2014 at 7:00 PM, Luke Yelavich
<luke.yelavich at canonical.com> wrote:
> On Sat, Oct 25, 2014 at 04:24:12PM EDT, Trevor Saunders wrote:
>> > >> +void free_spd_voices(SPDVoice** voices)
>> > >
>> > > super picky, but wouldn't SPDVoice **voices be more consistant?
>>
>> I see you left that, but I'm not going to hasle you about it given this
>> code seems pretty inconsistant.
>
> Yeah agreed, probably something that is worth cleaning up at some point.
>
>>
>> > >> +{
>> > >> + 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.
>> >
>> > Yep, removed the null check.
>> >
>> > >
>> > >> + 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);
>> > > }
>> >
>> > If I do this I wont be able to free voices afterwards. So I left that as
>> > is.
>>
>> yeah, you caught me ;) lgtm now fwiw.
>
> Also agreed, I'll apply.
>
> Luke
>
> _______________________________________________
> Speechd mailing list
> Speechd at lists.freebsoft.org
> http://lists.freebsoft.org/mailman/listinfo/speechd
libspeechd SPDVoice cleanup api, Jeremy Whiting, 2014/10/24
libspeechd SPDVoice cleanup api, Luke Yelavich, 2014/10/24