Re: [Evolution-hackers] interesting code ...
- From: Parthasarathi Susarla <sparthasarathi novell com>
- To: Not Zed <notzed ximian com>
- Cc: evolution-hackers lists ximian com
- Subject: Re: [Evolution-hackers] interesting code ...
- Date: Wed, 29 Jun 2005 19:02:52 +0530
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]