speechd-discuss
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]