Re: [Evolution-hackers] interesting code ...



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




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]