Hi michael, Thanks for the 'review' of the groupwise code, and ofcourse for pointing out the _very_ obvious mistakes in the code.
On Sat, 2005-06-18 at 03:52 +0800, Not Zed wrote: > Or at least ... interesting choice of algorithms. I sure hope this > isn't called very often? > > 2 N^2 loops ... AND it accesses data AFTER it is all freed (the uid's > are no longer reffed after you free the objects which hold them, it just > happens that because of other reasons this will usually work). > I gotta say the whole gw code looks mighty odd, whomever wrote it > obviously loved using singly linked lists - about the most inefficient > choice imaginable in this case, particularly given the data in question > is already in an array in all the cases i've seen (these two below are > more than enough for now, thank-you-very-much). For now i will continue using GSList, since switching to GList would require a huge amount of code change, both in groupwise backend and the provider itself. I also should add that am a little stuck here. Having 2 pointer arrays/lists, what would be the best way to check if data in 'ptr-array/list A' is also present in 'ptr-array/list B'? > Oh boy, here's another gem of a fragment. Absolutely brilliant. > > Of particular note is the ingenous inner-loop which iterates over a list > which was created from an array .. but then indexes into the array > anyway, ignoring all of the data in the list in the first place. The > list is never freed either. I see the list is cunningly being used as a > loop counter! It could be worse, I guess you could've implemented the > list consumer recursively ... > > Note also that 'count' initialised in the first line - is never used. > > At least it doesn't try to de-reference previously freed data (but i > wouldn't bet on that!). Although the code following that pasted here > does a wonderful job of leaking not once, but twice for every > messageinfo it looks up and creates. Oh wait, this leaks the > messageinfo's too. > As for this piece of code, i will see to that the leak(s) get fixed. Sorry for the delay in responding. Looking forward to more such reviews and comments. Thanks and cheers, partha _______________________________________________ evolution-hackers maillist - [email protected] http://lists.ximian.com/mailman/listinfo/evolution-hackers
