[Evolution-hackers] interesting code ...

Parthasarathi Susarla sparthasarathi at novell.com
Wed Jun 29 09:32:52 EDT 2005


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



More information about the evolution-hackers mailing list