libspeechd SPDVoice cleanup api

[Trevor Saunders]

On Sat, Oct 25, 2014 at 09:41:24PM -0600, Jeremy Whiting wrote:
> 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.

no worries, $day job makes me deal with a bunch of people who are super
picky, so its kind of a habit now ;-)

Trev

> 
> 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
> 
> _______________________________________________
> 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/20141026/5b86b243/attachment.pgp>